3mf: claim the backup lock when the backup's state cannot be established (#16431)

has_restore_data() signals "this backup is in use, leave it alone" by setting
origin to "<lock>". Its caller treats anything else as permission to delete
(Plater.cpp, the EVT_RESTORE_PROJECT handler):

    try {
        if (originfile != "<lock>") // see bbs_3mf.cpp for lock detail
            boost::filesystem::remove_all(last);
    }
    catch (...) {}

So every "return false" that leaves origin empty is a delete instruction, and
three of them fired without anyone having established that the backup was
spent.

lock.txt exists but cannot be read. load_string_file() was outside the try, so
the exception left has_restore_data() altogether. The handler is queued and has
no try around the call, so this took the application down. Fixes #15670.

lock.txt reads but holds no pid. A crash or a full disk mid-write leaves a
zero-length lock.txt, because save_string_file() truncates before it writes.
boost::lexical_cast<int>("") throws into the catch, which returned false with
origin still empty, and the caller deleted the backup. Also #15670.

The .3mf probe could not answer. A path that cannot be probed at all reports
false exactly like a missing one, so an unreachable backup looked spent. The
error_code now separates the two: ENOENT and ENOTDIR mean "not there" and stay
collectable, any other error means the question was never answered.

All three now claim the lock. Declining a restore is recoverable; deleting a
backup nobody could examine is not.

What deliberately did not change: a readable directory with no lock and no
project still comes back with origin empty and stays collectable, as do a lock
held by another process and a directory that does not exist. Claiming the lock
whenever anything looked amiss would strand every spent backup on disk, so
those three cases are tests rather than prose.

Tested in tests/libslic3r/test_3mf.cpp. Against the previous behaviour the new
cases fail as they should -- the two uninterpretable locks come back with an
empty origin, and the unreadable one escapes as

  basic_ios::clear: iostream error

while the three collectable cases pass unchanged, before and after.

src/libslic3r/Format/bbs_3mf.cpp

@@ -51,6 +51,7 @@
 #include <boost/spirit/home/karma.hpp>
 #include <deque>
 #include <boost/filesystem/directory.hpp>
+#include <boost/system/errc.hpp>
 #include <boost/thread/lock_guard.hpp>
 #include <boost/date_time/posix_time/posix_time_duration.hpp>
 #include <boost/date_time/posix_time/posix_time_types.hpp>
@@ -9776,13 +9777,13 @@ bool has_restore_data(std::string & path, std::string& origin)
         origin = "<lock>";
         return false;
     }
-    // ORCA: non-throwing probes; ec is deliberately unread, a backup path that cannot be
-    //       probed is treated as having no restore data rather than aborting the application.
+    // ORCA: the caller removes this directory unless origin is "<lock>", so every way of
+    //       failing to establish the backup's state has to claim the lock instead.
     boost::system::error_code ec;
     if (boost::filesystem::exists(path + "/lock.txt", ec)) {
         std::string pid;
-        load_string_file(path + "/lock.txt", pid);
         try {
+            load_string_file(path + "/lock.txt", pid);
             if (get_process_name(boost::lexical_cast<int>(pid)) ==
                 get_process_name(0)) {
                 origin = "<lock>";
@@ -9790,12 +9791,20 @@ bool has_restore_data(std::string & path, std::string& origin)
             }
         }
         catch (...) {
+            origin = "<lock>";
             return false;
         }
     }
     std::string file3mf = path + "/.3mf";
-    if (!boost::filesystem::exists(file3mf, ec))
+    if (!boost::filesystem::exists(file3mf, ec)) {
+        // ORCA: a .3mf that is simply absent is a spent backup and stays collectable. A probe
+        //       that could not look must not be: ec reports ENOENT/ENOTDIR for "not there" as
+        //       well, so only a different error means the answer is unknown.
+        if (ec && ec != boost::system::errc::no_such_file_or_directory &&
+                  ec != boost::system::errc::not_a_directory)
+            origin = "<lock>";
         return false;
+    }
     try {
         if (boost::filesystem::exists(path + "/origin.txt", ec))
             load_string_file(path + "/origin.txt", origin);

tests/libslic3r/test_3mf.cpp

@@ -2361,5 +2361,96 @@ TEST_CASE("has_restore_data reports no restore data for a path it cannot probe",
     REQUIRE_NOTHROW(has = has_restore_data(unprobeable, origin2));
     CHECK_FALSE(has);
     CHECK(unprobeable == backup.string());   // not rewritten to the .3mf it could not confirm
+    CHECK(origin2 == "<lock>");              // and the caller is told not to remove it
 }
 #endif // _WIN32
+
+// Every "return false" that leaves origin empty is an instruction to the caller to
+// remove_all() the backup directory (Plater.cpp's EVT_RESTORE_PROJECT handler), so the
+// distinction these cases draw is between a backup that is spent and one whose state could
+// not be established. Only the first may be collected.
+TEST_CASE("has_restore_data claims the lock when lock.txt cannot be interpreted", "[3mf]") {
+    ScopedTemporaryDir      outer("orca_lock");
+    boost::filesystem::path backup = outer.path() / "backup";
+    REQUIRE(boost::filesystem::create_directory(backup));
+    std::ofstream((backup / ".3mf").string()) << "stand-in for the backed-up project";
+
+    SECTION("an empty lock.txt holds no pid") {
+        std::ofstream lock((backup / "lock.txt").string());
+        lock.close();   // created, left zero length
+        REQUIRE(boost::filesystem::file_size(backup / "lock.txt") == 0);
+
+        std::string path = backup.string(), origin;
+        bool        has  = true;
+        REQUIRE_NOTHROW(has = has_restore_data(path, origin));
+        CHECK_FALSE(has);
+        CHECK(origin == "<lock>");
+    }
+
+    SECTION("lock.txt holds something that is not a pid") {
+        std::ofstream((backup / "lock.txt").string()) << "not-a-pid";
+
+        std::string path = backup.string(), origin;
+        bool        has  = true;
+        REQUIRE_NOTHROW(has = has_restore_data(path, origin));
+        CHECK_FALSE(has);
+        CHECK(origin == "<lock>");
+    }
+}
+
+#ifndef _WIN32
+TEST_CASE("has_restore_data claims the lock when lock.txt cannot be read", "[3mf]") {
+    if (::geteuid() == 0)
+        SKIP("root ignores the file permissions this test depends on");
+
+    ScopedTemporaryDir      outer("orca_lock");
+    boost::filesystem::path backup = outer.path() / "backup";
+    REQUIRE(boost::filesystem::create_directory(backup));
+    std::ofstream((backup / ".3mf").string()) << "stand-in for the backed-up project";
+    std::ofstream((backup / "lock.txt").string()) << "1";
+    boost::system::error_code mode_ec;   // a filesystem that refuses the mode change must skip below
+    boost::filesystem::permissions(backup / "lock.txt", boost::filesystem::no_perms, mode_ec);
+
+    boost::nowide::ifstream probe((backup / "lock.txt").string());
+    if (probe.good())
+        SKIP("this filesystem does not enforce file read permissions");
+
+    // load_string_file() throws here.
+    std::string path = backup.string(), origin;
+    bool        has  = true;
+    REQUIRE_NOTHROW(has = has_restore_data(path, origin));
+    CHECK_FALSE(has);
+    CHECK(origin == "<lock>");
+}
+#endif // _WIN32
+
+// The negative control for the three cases above: claiming the lock whenever anything is
+// amiss would strand every spent backup on disk forever, so a readable directory that
+// simply has nothing to restore must still come back collectable.
+TEST_CASE("has_restore_data leaves a spent backup collectable", "[3mf]") {
+    ScopedTemporaryDir      outer("orca_spent");
+    boost::filesystem::path backup = outer.path() / "backup";
+    REQUIRE(boost::filesystem::create_directory(backup));
+
+    SECTION("no lock and no project") {
+        std::string path = backup.string(), origin;
+        CHECK_FALSE(has_restore_data(path, origin));
+        CHECK(origin.empty());
+    }
+
+    SECTION("a lock from a process that is not us, and no project") {
+        // get_process_name() compares executable names and answers empty for a pid it cannot
+        // read, so pid 1 never matches this process and the lock must not be claimed.
+        std::ofstream((backup / "lock.txt").string()) << "1";
+
+        std::string path = backup.string(), origin;
+        CHECK_FALSE(has_restore_data(path, origin));
+        CHECK(origin.empty());
+    }
+
+    SECTION("the directory does not exist at all") {
+        std::string path = (outer.path() / "gone").string(), origin;
+        CHECK_FALSE(has_restore_data(path, origin));
+        CHECK(origin.empty());
+    }
+}