fix(inode_manager): handle file mapping errors

include/dwarfs/inode_manager.h

@@ -81,7 +81,7 @@ class inode_manager {
   }
 
   void scan_background(worker_group& wg, os_access const& os,
-                       std::shared_ptr<inode> ino, file const* p) const {
+                       std::shared_ptr<inode> ino, file* p) const {
     impl_->scan_background(wg, os, std::move(ino), p);
   }
 
@@ -103,9 +103,8 @@ class inode_manager {
     virtual void for_each_inode_in_order(
         std::function<void(std::shared_ptr<inode> const&)> const& fn) const = 0;
     virtual fragment_infos fragment_category_info() const = 0;
-    virtual void
+    virtual void scan_background(worker_group& wg, os_access const& os,
-    scan_background(worker_group& wg, os_access const& os,
+                                 std::shared_ptr<inode> ino, file* p) const = 0;
-                    std::shared_ptr<inode> ino, file const* p) const = 0;
     virtual void dump(std::ostream& os) const = 0;
     virtual sortable_inode_span sortable_span() const = 0;
     virtual sortable_inode_span

src/dwarfs/inode_manager.cpp

@@ -25,6 +25,7 @@
 #include <cstdlib>
 #include <cstring>
 #include <deque>
+#include <exception>
 #include <limits>
 #include <numeric>
 #include <ostream>
@@ -38,6 +39,7 @@
 #include <fmt/format.h>
 
 #include <folly/Demangle.h>
+#include <folly/String.h>
 #include <folly/small_vector.h>
 #include <folly/sorted_vector_types.h>
 
@@ -548,9 +550,8 @@ class inode_manager_ final : public inode_manager::impl {
     return rv;
   }
 
-  void
+  void scan_background(worker_group& wg, os_access const& os,
-  scan_background(worker_group& wg, os_access const& os,
+                       std::shared_ptr<inode> ino, file* p) const override;
-                  std::shared_ptr<inode> ino, file const* p) const override;
 
   void dump(std::ostream& os) const override;
 
@@ -592,7 +593,7 @@ template <typename LoggerPolicy>
 void inode_manager_<LoggerPolicy>::scan_background(worker_group& wg,
                                                    os_access const& os,
                                                    std::shared_ptr<inode> ino,
-                                                   file const* p) const {
+                                                   file* p) const {
   // TODO: I think the size check makes everything more complex.
   //       If we don't check the size, we get the code to run
   //       that ensures `fragments_` is updated. Also, there
@@ -603,7 +604,16 @@ void inode_manager_<LoggerPolicy>::scan_background(worker_group& wg,
       auto const size = p->size();
       std::shared_ptr<mmif> mm;
       if (size > 0) {
-        mm = os.map_file(p->fs_path(), size);
+        try {
+          mm = os.map_file(p->fs_path(), size);
+        } catch (...) {
+          LOG_ERROR << "failed to map file \"" << p->path_as_string()
+                    << "\": " << folly::exceptionStr(std::current_exception())
+                    << ", creating empty inode";
+          ++prog_.errors;
+          p->override_size(0);
+          // don't return here, we still need scan() to run
+        }
       }
       ino->scan(mm.get(), opts_, prog_);
       update_prog(ino, p);

test/test_helpers.cpp

@@ -328,6 +328,11 @@ void os_access_mock::set_access_fail(fs::path const& path) {
   access_fail_set_.emplace(path);
 }
 
+void os_access_mock::set_map_file_error(std::filesystem::path const& path,
+                                        std::exception_ptr ep) {
+  map_file_err_[path] = std::move(ep);
+}
+
 size_t os_access_mock::size() const { return root_ ? root_->size() : 0; }
 
 std::vector<std::string> os_access_mock::splitpath(fs::path const& path) {
@@ -426,6 +431,10 @@ std::unique_ptr<mmif>
 os_access_mock::map_file(fs::path const& path, size_t size) const {
   if (auto de = find(path);
       de && de->status.type() == posix_file_type::regular) {
+    if (auto it = map_file_err_.find(path); it != map_file_err_.end()) {
+      std::rethrow_exception(it->second);
+    }
+
     return std::make_unique<mmap_mock>(
         std::visit(overloaded{
                        [this](std::string const& str) { return str; },

test/test_helpers.h

@@ -21,6 +21,7 @@
 
 #pragma once
 
+#include <exception>
 #include <filesystem>
 #include <functional>
 #include <map>
@@ -91,6 +92,8 @@ class os_access_mock : public os_access {
   void add_local_files(std::filesystem::path const& path);
 
   void set_access_fail(std::filesystem::path const& path);
+  void
+  set_map_file_error(std::filesystem::path const& path, std::exception_ptr ep);
 
   void setenv(std::string name, std::string value);
 
@@ -125,6 +128,7 @@ class os_access_mock : public os_access {
   std::unique_ptr<mock_dirent> root_;
   size_t ino_{1000000};
   std::set<std::filesystem::path> access_fail_set_;
+  std::map<std::filesystem::path, std::exception_ptr> map_file_err_;
   std::map<std::string, std::string> env_;
 };
 

test/tool_main_test.cpp

@@ -2101,3 +2101,16 @@ TEST_P(segmenter_repeating_sequence_test, github161) {
 
 INSTANTIATE_TEST_SUITE_P(dwarfs, segmenter_repeating_sequence_test,
                          ::testing::Values('\0', 'G', '\xff'));
+
+TEST(mkdwarfs_test, map_file_error) {
+  mkdwarfs_tester t;
+  t.os->set_map_file_error(
+      "/somedir/ipsum.py",
+      std::make_exception_ptr(std::runtime_error("map_file_error")));
+
+  EXPECT_EQ(2, t.run("-i / -o - --categorize")) << t.err();
+
+  EXPECT_THAT(t.err(),
+              ::testing::HasSubstr("map_file_error, creating empty inode"));
+  EXPECT_THAT(t.err(), ::testing::HasSubstr("filesystem created with 1 error"));
+}