diff --git a/src/cryfs-cli/Cli.cpp b/src/cryfs-cli/Cli.cpp index 88cade03..b0646bec 100644 --- a/src/cryfs-cli/Cli.cpp +++ b/src/cryfs-cli/Cli.cpp @@ -51,8 +51,8 @@ using std::string; using std::endl; using std::shared_ptr; using std::make_shared; +using std::make_unique; using std::function; -using std::make_shared; using boost::optional; using boost::none; using boost::chrono::minutes; @@ -219,7 +219,7 @@ namespace cryfs { cipher, blocksizeBytes, missingBlockIsIntegrityViolation).loadOrCreate(std::move(configFilePath), allowFilesystemUpgrade, allowReplacedFilesystem); } - void Cli::_runFilesystem(const ProgramOptions &options) { + void Cli::_runFilesystem(const ProgramOptions &options, std::function onMounted) { try { LocalStateDir localStateDir(Environment::localStateDir()); auto blockStore = make_unique_ref(options.baseDir()); @@ -229,11 +229,11 @@ namespace cryfs { options.allowIntegrityViolations(), missingBlockIsIntegrityViolation)); _sanityCheckFilesystem(_device->get()); - auto initFilesystem = [this, &options] (fspp::fuse::Fuse *fuse){ + auto initFilesystem = [&] (fspp::fuse::Fuse *fs){ 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(), [fuse] {fuse->stop();}); + _idleUnmounter = _createIdleCallback(options.unmountAfterIdleMinutes(), [fs] {fs->stop();}); if (_idleUnmounter != none) { (*_device)->onFsAction(std::bind(&CallAfterTimeout::resetTimer, _idleUnmounter->get())); } @@ -241,7 +241,7 @@ namespace cryfs { return make_shared(std::move(*_device)); }; - auto fuse = make_unique_ref(initFilesystem, "cryfs", "cryfs@" + options.baseDir().string()); + auto fuse = make_unique(initFilesystem, std::move(onMounted), "cryfs", "cryfs@" + options.baseDir().string()); _initLogfile(options); @@ -369,14 +369,14 @@ namespace cryfs { return false; } - int Cli::main(int argc, const char *argv[], unique_ref httpClient) { + int Cli::main(int argc, const char *argv[], unique_ref httpClient, std::function onMounted) { cpputils::showBacktraceOnCrash(); try { _showVersion(std::move(httpClient)); ProgramOptions options = program_options::Parser(argc, argv).parse(CryCiphers::supportedCipherNames()); _sanityChecks(options); - _runFilesystem(options); + _runFilesystem(options, std::move(onMounted)); } catch (const CryfsException &e) { if (e.errorCode() != ErrorCode::Success) { std::cerr << "Error: " << e.what() << std::endl; diff --git a/src/cryfs-cli/Cli.h b/src/cryfs-cli/Cli.h index b5f70388..0195a967 100644 --- a/src/cryfs-cli/Cli.h +++ b/src/cryfs-cli/Cli.h @@ -18,11 +18,11 @@ namespace cryfs { class Cli final { public: Cli(cpputils::RandomGenerator &keyGenerator, const cpputils::SCryptSettings& scryptSettings, std::shared_ptr console); - int main(int argc, const char *argv[], cpputils::unique_ref httpClient); + int main(int argc, const char *argv[], cpputils::unique_ref httpClient, std::function onMounted); private: void _checkForUpdates(cpputils::unique_ref httpClient); - void _runFilesystem(const program_options::ProgramOptions &options); + void _runFilesystem(const program_options::ProgramOptions &options, std::function onMounted); CryConfigLoader::ConfigLoadResult _loadOrCreateConfig(const program_options::ProgramOptions &options, const LocalStateDir& localStateDir); void _checkConfigIntegrity(const boost::filesystem::path& basedir, const LocalStateDir& localStateDir, const CryConfigFile& config, bool allowReplacedFilesystem); boost::optional _loadOrCreateConfigFile(boost::filesystem::path configFilePath, LocalStateDir localStateDir, const boost::optional &cipher, const boost::optional &blocksizeBytes, bool allowFilesystemUpgrade, const boost::optional &missingBlockIsIntegrityViolation, bool allowReplacedFilesystem); diff --git a/src/cryfs-cli/main.cpp b/src/cryfs-cli/main.cpp index c9f2874c..6dcc1c0d 100644 --- a/src/cryfs-cli/main.cpp +++ b/src/cryfs-cli/main.cpp @@ -26,7 +26,7 @@ int main(int argc, const char *argv[]) { auto httpClient = make_unique_ref(); #endif return Cli(keyGenerator, SCrypt::DefaultSettings, make_shared()) - .main(argc, argv, std::move(httpClient)); + .main(argc, argv, std::move(httpClient), []{}); } catch (const CryfsException &e) { if (e.errorCode() != ErrorCode::Success) { std::cerr << "Error: " << e.what() << std::endl; diff --git a/src/fspp/fuse/Fuse.cpp b/src/fspp/fuse/Fuse.cpp index 769b0351..00528318 100644 --- a/src/fspp/fuse/Fuse.cpp +++ b/src/fspp/fuse/Fuse.cpp @@ -229,8 +229,10 @@ Fuse::~Fuse() { _argv.clear(); } -Fuse::Fuse(std::function (Fuse *fuse)> init, std::string fstype, boost::optional fsname) - :_init(std::move(init)), _fs(make_shared()), _mountdir(), _running(false), _fstype(std::move(fstype)), _fsname(std::move(fsname)) { +Fuse::Fuse(std::function (Fuse *fuse)> init, std::function onMounted, std::string fstype, boost::optional fsname) + :_init(std::move(init)), _onMounted(std::move(onMounted)), _fs(make_shared()), _mountdir(), _running(false), _fstype(std::move(fstype)), _fsname(std::move(fsname)) { + ASSERT(static_cast(_init), "Invalid init given"); + ASSERT(static_cast(_onMounted), "Invalid onMounted given"); } void Fuse::_logException(const std::exception &e) { @@ -866,6 +868,7 @@ void Fuse::init(fuse_conn_info *conn) { LOG(INFO, "Filesystem started."); _running = true; + _onMounted(); #ifdef FSPP_LOG cpputils::logging::setLevel(DEBUG); diff --git a/src/fspp/fuse/Fuse.h b/src/fspp/fuse/Fuse.h index a140d846..7d173654 100644 --- a/src/fspp/fuse/Fuse.h +++ b/src/fspp/fuse/Fuse.h @@ -21,7 +21,7 @@ class Filesystem; class Fuse final { public: - explicit Fuse(std::function (Fuse *fuse)> init, std::string fstype, boost::optional fsname); + explicit Fuse(std::function (Fuse *fuse)> init, std::function onMounted, std::string fstype, boost::optional fsname); ~Fuse(); void run(const boost::filesystem::path &mountdir, const std::vector &fuseOptions); @@ -69,6 +69,7 @@ private: void _add_fuse_option_if_not_exists(std::vector *argv, const std::string &key, const std::string &value); std::function (Fuse *fuse)> _init; + std::function _onMounted; std::shared_ptr _fs; boost::filesystem::path _mountdir; std::vector _argv; diff --git a/test/cryfs-cli/testutils/CliTest.h b/test/cryfs-cli/testutils/CliTest.h index 24df1ade..0214d444 100644 --- a/test/cryfs-cli/testutils/CliTest.h +++ b/test/cryfs-cli/testutils/CliTest.h @@ -16,6 +16,7 @@ #include #include #include +#include #include "../../cryfs/testutils/MockConsole.h" #include "../../cryfs/testutils/TestWithFakeHomeDirectory.h" #include @@ -41,18 +42,18 @@ public: return std::move(httpClient); } - int run(const std::vector& args) { - std::vector _args; - _args.reserve(args.size() + 1); - _args.emplace_back("cryfs"); - for (const std::string& arg : args) { - _args.emplace_back(arg.c_str()); - } + int run(const std::vector& args, std::function onMounted) { + std::vector _args; + _args.reserve(args.size() + 1); + _args.emplace_back("cryfs"); + for (const std::string& arg : args) { + _args.emplace_back(arg.c_str()); + } auto &keyGenerator = cpputils::Random::PseudoRandom(); ON_CALL(*console, askPassword(testing::StrEq("Password: "))).WillByDefault(testing::Return("pass")); ON_CALL(*console, askPassword(testing::StrEq("Confirm Password: "))).WillByDefault(testing::Return("pass")); // Run Cryfs - return cryfs::Cli(keyGenerator, cpputils::SCrypt::TestSettings, console).main(_args.size(), _args.data(), _httpClient()); + return cryfs::Cli(keyGenerator, cpputils::SCrypt::TestSettings, console).main(_args.size(), _args.data(), _httpClient(), std::move(onMounted)); } void EXPECT_EXIT_WITH_HELP_MESSAGE(const std::vector& args, const std::string &message, cryfs::ErrorCode errorCode) { @@ -60,7 +61,7 @@ public: } void EXPECT_RUN_ERROR(const std::vector& args, const std::string& message, cryfs::ErrorCode errorCode) { - FilesystemOutput filesystem_output = _run_filesystem(args, boost::none); + FilesystemOutput filesystem_output = _run_filesystem(args, boost::none, []{}); EXPECT_EQ(exitCode(errorCode), filesystem_output.exit_code); EXPECT_TRUE(std::regex_search(filesystem_output.stderr_, std::regex(message))); @@ -70,10 +71,10 @@ public: //TODO Make this work when run in background ASSERT(std::find(args.begin(), args.end(), string("-f")) != args.end(), "Currently only works if run in foreground"); - FilesystemOutput filesystem_output = _run_filesystem(args, mountDir); + FilesystemOutput filesystem_output = _run_filesystem(args, mountDir, []{}); - EXPECT_EQ(0, filesystem_output.exit_code); - EXPECT_TRUE(std::regex_search(filesystem_output.stdout_, std::regex("Mounting filesystem"))); + EXPECT_EQ(0, filesystem_output.exit_code); + EXPECT_TRUE(std::regex_search(filesystem_output.stdout_, std::regex("Mounting filesystem"))); } struct FilesystemOutput final { @@ -82,60 +83,80 @@ public: std::string stderr_; }; - FilesystemOutput _run_filesystem(const std::vector& args, const boost::optional& mountDirForUnmounting) { - testing::internal::CaptureStdout(); - testing::internal::CaptureStderr(); - std::future exit_code = std::async(std::launch::async, [this, &args] { - return run(args); + static void _unmount(const boost::filesystem::path &mountDir) { + int returncode = -1; +#if defined(__APPLE__) + returncode = cpputils::Subprocess::call(std::string("umount ") + mountDir.string().c_str() + " 2>/dev/null").exitcode; +#elif defined(_MSC_VER) + std::wstring mountDir_ = std::wstring_convert>().from_bytes(mountDir.string()); + BOOL success = DokanRemoveMountPoint(mountDir_.c_str()); + returncode = success ? 0 : -1; +#else + returncode = cpputils::Subprocess::call( + std::string("fusermount -u ") + mountDir.string().c_str() + " 2>/dev/null").exitcode; +#endif + ASSERT(returncode == 0, "Unmount failed"); + } + + FilesystemOutput _run_filesystem(const std::vector& args, boost::optional mountDirForUnmounting, std::function onMounted) { + testing::internal::CaptureStdout(); + testing::internal::CaptureStderr(); + + bool exited = false; + cpputils::ConditionBarrier isMountedOrFailedBarrier; + + std::future exit_code = std::async(std::launch::async, [&] { + int exit_code = run(args, [&] { isMountedOrFailedBarrier.release(); }); + // just in case it fails, we also want to release the barrier. + // if it succeeds, this will release it a second time, which doesn't hurt. + exited = true; + isMountedOrFailedBarrier.release(); + return exit_code; }); - if (mountDirForUnmounting.is_initialized()) { - boost::filesystem::path mountDir = *mountDirForUnmounting; - std::future unmount_success = std::async(std::launch::async, [&mountDir] { - int returncode = -1; - while (returncode != 0) { -#if defined(__APPLE__) - returncode = cpputils::Subprocess::call(std::string("umount ") + mountDir.string().c_str() + " 2>/dev/null").exitcode; -#elif defined(_MSC_VER) - // Somehow this sleeping is needed to not deadlock. Race condition in mounting/unmounting? - std::this_thread::sleep_for(std::chrono::milliseconds(50)); - std::wstring mountDir_ = std::wstring_convert>().from_bytes(mountDir.string()); - BOOL success = DokanRemoveMountPoint(mountDir_.c_str()); - returncode = success ? 0 : -1; -#else - returncode = cpputils::Subprocess::call(std::string("fusermount -u ") + mountDir.string().c_str() + " 2>/dev/null").exitcode; -#endif - } - return true; - }); - - if(std::future_status::ready != unmount_success.wait_for(std::chrono::seconds(10))) { - testing::internal::GetCapturedStdout(); // stop capturing stdout - testing::internal::GetCapturedStderr(); // stop capturing stderr - - std::cerr << "Unmount thread didn't finish"; - // The std::future destructor of a future created with std::async blocks until the future is ready. - // so, instead of causing a deadlock, rather abort - exit(EXIT_FAILURE); + std::future on_mounted_success = std::async(std::launch::async, [&] { + isMountedOrFailedBarrier.wait(); + if (exited) { + // file system already exited on its own, this indicates an error. It should have stayed mounted. + // while the exit_code from run() will signal an error in this case, we didn't encounter another + // error in the onMounted future, so return true here. + return true; } - EXPECT_TRUE(unmount_success.get()); // this also re-throws any potential exceptions + // now we know the filesystem stayed online, so we can call the onMounted callback + onMounted(); + // and unmount it afterwards + if (mountDirForUnmounting.is_initialized()) { + _unmount(*mountDirForUnmounting); + } + return true; + }); + + if(std::future_status::ready != on_mounted_success.wait_for(std::chrono::seconds(10))) { + testing::internal::GetCapturedStdout(); // stop capturing stdout + testing::internal::GetCapturedStderr(); // stop capturing stderr + + std::cerr << "onMounted thread (e.g. used for unmount) didn't finish"; + // The std::future destructor of a future created with std::async blocks until the future is ready. + // so, instead of causing a deadlock, rather abort + exit(EXIT_FAILURE); } + EXPECT_TRUE(on_mounted_success.get()); // this also re-throws any potential exceptions if(std::future_status::ready != exit_code.wait_for(std::chrono::seconds(10))) { - testing::internal::GetCapturedStdout(); // stop capturing stdout - testing::internal::GetCapturedStderr(); // stop capturing stderr - - std::cerr << "Filesystem thread didn't finish"; - // The std::future destructor of a future created with std::async blocks until the future is ready. - // so, instead of causing a deadlock, rather abort - exit(EXIT_FAILURE); + testing::internal::GetCapturedStdout(); // stop capturing stdout + testing::internal::GetCapturedStderr(); // stop capturing stderr + + std::cerr << "Filesystem thread didn't finish"; + // The std::future destructor of a future created with std::async blocks until the future is ready. + // so, instead of causing a deadlock, rather abort + exit(EXIT_FAILURE); } - return { - exit_code.get(), - testing::internal::GetCapturedStdout(), - testing::internal::GetCapturedStderr() - }; + return { + exit_code.get(), + testing::internal::GetCapturedStdout(), + testing::internal::GetCapturedStderr() + }; } }; diff --git a/test/fspp/testutils/FuseTest.cpp b/test/fspp/testutils/FuseTest.cpp index d7cade13..62cfa419 100644 --- a/test/fspp/testutils/FuseTest.cpp +++ b/test/fspp/testutils/FuseTest.cpp @@ -57,7 +57,7 @@ unique_ref FuseTest::TestFS() { FuseTest::TempTestFS::TempTestFS(shared_ptr fsimpl) :_mountDir(), - _fuse([fsimpl] (Fuse*) {return fsimpl;}, "fusetest", boost::none), _fuse_thread(&_fuse) { + _fuse([fsimpl] (Fuse*) {return fsimpl;}, []{}, "fusetest", boost::none), _fuse_thread(&_fuse) { _fuse_thread.start(_mountDir.path(), {"-f"}); }