diff --git a/ChangeLog.txt b/ChangeLog.txt index 855e00a6..25eed6ba 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -3,6 +3,7 @@ Version 0.10.1 (unreleased) Fixed bugs: * If file system migration encounters files or folders with the wrong format in the base directory, it now just ignores them instead of crashing. * When trying to migrate a file system from CryFS 0.9.3 or older, show an error message suggesting to first open it with 0.9.10 because we can't load that anymore. +* The '--unmount-idle' parameter works again Other: * Updated to crypto++ 8.1 diff --git a/src/cpp-utils/thread/ThreadSystem.cpp b/src/cpp-utils/thread/ThreadSystem.cpp index dc5a62a6..13a0b0a6 100644 --- a/src/cpp-utils/thread/ThreadSystem.cpp +++ b/src/cpp-utils/thread/ThreadSystem.cpp @@ -52,15 +52,34 @@ namespace cpputils { void ThreadSystem::_stopAllThreadsForRestart() { _mutex.lock(); // Is unlocked in the after-fork handler. This way, the whole fork() is protected. for (RunningThread &thread : _runningThreads) { + if (boost::this_thread::get_id() == thread.thread.get_id()) { + // This means fork was called from within one of our _runningThreads. + // We cannot wait or ourselves to die. + // Forking from within a thread is usually chaos since the forked process only gets a copy + // of the calling thread as its new main thread. So we (hopefully) never should do this. + // This is, however, a valid pattern when fork() is directly followed by an exec(). + // So let's just ignore this situation and continue as if nothing happened, assuming an exec() + // follows soon. + continue; + } thread.thread.interrupt(); } for (RunningThread &thread : _runningThreads) { + if (boost::this_thread::get_id() == thread.thread.get_id()) { + // This means fork was called from within one of our _runningThreads. See comment above. + continue; + } thread.thread.join(); } } void ThreadSystem::_restartAllThreads() { for (RunningThread &thread : _runningThreads) { + if (thread.thread.joinable()) { + // Because all non-self threads have been terminated in _stopAllThreadsForRestart, + // this means fork was called from within one of our _runningThreads. See comment above. + continue; + } thread.thread = _startThread(thread.loopIteration, thread.threadName); } _mutex.unlock(); // Was locked in the before-fork handler diff --git a/src/cryfs-cli/Cli.cpp b/src/cryfs-cli/Cli.cpp index b4e7a508..261459f9 100644 --- a/src/cryfs-cli/Cli.cpp +++ b/src/cryfs-cli/Cli.cpp @@ -252,7 +252,11 @@ namespace cryfs_cli { ASSERT(_device != none, "File system not ready to be initialized. Was it already initialized before?"); //TODO Test auto unmounting after idle timeout - _idleUnmounter = _createIdleCallback(options.unmountAfterIdleMinutes(), [fs] {fs->stop();}); + const boost::optional idle_minutes = options.unmountAfterIdleMinutes(); + _idleUnmounter = _createIdleCallback(idle_minutes, [fs, idle_minutes] { + LOG(INFO, "Unmounting because file system was idle for {} minutes", *idle_minutes); + fs->stop(); + }); if (_idleUnmounter != none) { (*_device)->onFsAction(std::bind(&CallAfterTimeout::resetTimer, _idleUnmounter->get())); } diff --git a/test/cpp-utils/process/SubprocessTest.cpp b/test/cpp-utils/process/SubprocessTest.cpp index a5b0e93d..9a1c857c 100644 --- a/test/cpp-utils/process/SubprocessTest.cpp +++ b/test/cpp-utils/process/SubprocessTest.cpp @@ -1,6 +1,8 @@ #include #include +#include + using cpputils::Subprocess; using cpputils::SubprocessError; @@ -98,3 +100,25 @@ TEST(SubprocessTest, Call_error5withoutput_output) { TEST(SubprocessTest, Call_error5withoutput_exitcode) { EXPECT_EQ(5, Subprocess::call(exit_with_message_and_status("hello", 5)).exitcode); } + +// TODO Move this test to a test suite for ThreadSystem/LoopThread +#include +TEST(SubprocessTest, CallFromThreadSystemThread) { + cpputils::ConditionBarrier barrier; + + cpputils::LoopThread thread( + [&barrier] () { + auto result = Subprocess::check_call(exit_with_message_and_status("hello", 0)); + EXPECT_EQ(0, result.exitcode); + EXPECT_EQ("hello", result.output); + + barrier.release(); + + return false; // don't run loop again + }, + "child_thread" + ); + thread.start(); + barrier.wait(); + thread.stop(); // just to make sure it's stopped before the test exits. Returning false above should already stop it, but we don't know when exactly. thread.stop() will block until it's actually stopped. +}