diff --git a/docs/core-sdk.md b/docs/core-sdk.md index 367cada..9dca4c8 100644 --- a/docs/core-sdk.md +++ b/docs/core-sdk.md @@ -336,7 +336,11 @@ auto resolver = std::make_shared(); moonbase::licensing licensing(options, store, resolver); ``` -The default store is in-memory. `file_license_store` persists a JSON representation of the validated license. +The default store is in-memory. `file_license_store` persists a JSON representation of the +validated license, replacing the file in one step so a reader never finds half of one. +Instances that share the file take turns in `validate_token_online`, so one may wait for +another's network check. When shutting down, call `cancel_lock_waits()` on the store and a +waiting call throws `storage_error` instead. A custom resolver's id is compared literally, so it does not need to follow the [`mbd2_` stamp format](device-identity.md), and it gives up cross-SDK compatibility by diff --git a/docs/juce-module.md b/docs/juce-module.md index e1002e0..89c0872 100644 --- a/docs/juce-module.md +++ b/docs/juce-module.md @@ -119,7 +119,8 @@ ActivationDialog::show(config, [](bool wasActivated) { /* … */ }); `moonbase::licensing`. Network calls run on a controller-owned thread pool; all state changes and repaints happen on the message thread, gated by a generation counter so a slow request can never clobber a newer state. Destroying the controller cancels any -in-flight request and joins its workers, so nothing keeps running after teardown — you +in-flight request, stops waiting on another instance's license check, and joins its +workers, so nothing keeps running after teardown: you can call `start()` straight from the editor constructor and destroy the editor at any time (plugin scanning, pluginval, rapid open/close) without deferring or guarding it. The screens: diff --git a/include/moonbase/detail/file_lock.hpp b/include/moonbase/detail/file_lock.hpp index a70c292..7285c23 100644 --- a/include/moonbase/detail/file_lock.hpp +++ b/include/moonbase/detail/file_lock.hpp @@ -1,10 +1,14 @@ #pragma once +#include +#include #include +#include #include #include #include #include +#include #include #include "moonbase/errors.hpp" @@ -34,7 +38,7 @@ namespace moonbase::detail { // POSIX: flock(LOCK_EX) on the file's descriptor. Locks are per-open-file // description and cooperative; only callers that go through this class // observe each other. -// Windows: CRT _locking(_LK_LOCK) on a single-byte range. The range is +// Windows: CRT _locking on a single-byte range. The range is // required by the API but otherwise arbitrary; all callers using this class // lock the same range. _locking accepts ranges past EOF, so we deliberately // do not _chsize_s the file — SetEndOfFile on a sibling handle would race @@ -42,10 +46,18 @@ namespace moonbase::detail { // This intentionally avoids in public SDK headers because // store.hpp includes this detail header. // -// Throws moonbase::storage_error on unrecoverable open/lock failures. +// The lock is requested without blocking and retried every few milliseconds +// while someone else holds it, which can be for the length of their network +// check. A blocking request could not be called off, so a plugin closing +// while it waited would hold up the host until that check finished. Setting +// `give_up` ends the wait instead, with storage_error. No time limit +// otherwise: the holder lets go once its own request returns or times out. +// +// Throws moonbase::storage_error on unrecoverable open/lock failures, and +// when it gives up waiting. class file_lock { public: - explicit file_lock(const std::filesystem::path& path) + file_lock(const std::filesystem::path& path, const std::atomic& give_up) { const auto parent = path.parent_path(); if (!parent.empty()) { @@ -70,15 +82,6 @@ class file_lock { + std::generic_category().message(open_error)); } fd_ = fd; - - if (::_locking(fd_, _LK_LOCK, lock_range_size_) != 0) { - const auto err = errno; - ::_close(fd_); - fd_ = -1; - throw storage_error( - "Could not acquire license file lock: " - + std::generic_category().message(err)); - } #else fd_ = ::open(path.c_str(), O_RDWR | O_CREAT | O_CLOEXEC, 0644); if (fd_ < 0) { @@ -87,16 +90,17 @@ class file_lock { "Could not open license lock file: " + std::generic_category().message(err)); } +#endif - if (::flock(fd_, LOCK_EX) != 0) { - const auto err = errno; - ::close(fd_); - fd_ = -1; - throw storage_error( - "Could not acquire license file lock: " - + std::generic_category().message(err)); + auto pause = std::chrono::milliseconds(1); + while (!try_lock()) { + if (give_up.load(std::memory_order_acquire)) { + release(); + throw storage_error("Stopped waiting for the license file lock"); + } + std::this_thread::sleep_for(pause); + pause = std::min(pause * 2, max_pause_); } -#endif } file_lock(const file_lock&) = delete; @@ -121,6 +125,33 @@ class file_lock { ~file_lock() { release(); } private: + // True once the lock is ours, false while another holder has it. Closes the + // file and throws for any other failure. + bool try_lock() + { +#if defined(_WIN32) + if (::_locking(fd_, _LK_NBLCK, lock_range_size_) == 0) { + return true; + } + const auto err = errno; + if (err == EACCES) { + return false; + } +#else + if (::flock(fd_, LOCK_EX | LOCK_NB) == 0) { + return true; + } + const auto err = errno; + if (err == EWOULDBLOCK || err == EINTR) { + return false; + } +#endif + release(); + throw storage_error( + "Could not acquire license file lock: " + + std::generic_category().message(err)); + } + void release() noexcept { #if defined(_WIN32) @@ -142,6 +173,7 @@ class file_lock { #if defined(_WIN32) static constexpr long lock_range_size_ = 1; #endif + static constexpr std::chrono::milliseconds max_pause_{20}; int fd_ = -1; }; diff --git a/include/moonbase/store.hpp b/include/moonbase/store.hpp index 4fc5c36..73f148c 100644 --- a/include/moonbase/store.hpp +++ b/include/moonbase/store.hpp @@ -1,6 +1,9 @@ #pragma once +#include +#include #include +#include #include #include #include @@ -8,6 +11,13 @@ #include #include #include +#include + +#if defined(_WIN32) +#include +#else +#include +#endif #include @@ -143,6 +153,18 @@ class file_license_store : public license_store { } } + const auto contents = nlohmann::json(value).dump(2); + + // Write the new license beside the old one and rename it into place, so + // a reader sees one or the other and never a half-written file, which + // load_local_license would delete as corrupt. That holds for readers + // that don't take the lock, such as an instance loading at startup. + if (replace_with_temp_file(contents)) { + return; + } + + // Where that fails, as in a folder that takes no new files but lets the + // license itself be rewritten, write it in place. errno = 0; std::ofstream file(path_, std::ios::trunc); if (!file) { @@ -150,7 +172,7 @@ class file_license_store : public license_store { throw storage_error("Could not open local license file for writing: " + utf8_path(path_) + errno_suffix(err)); } - file << nlohmann::json(value).dump(2); + file << contents; file.flush(); if (!file) { throw storage_error("Could not write local license file: " + utf8_path(path_)); @@ -178,10 +200,115 @@ class file_license_store : public license_store { // process tree. auto lock_path = path_; lock_path += ".lock"; - return std::make_unique(lock_path); + return std::make_unique(lock_path, *lock_waits_cancelled_); + } + + // For shutting down: from here on lock_for_update() throws storage_error + // instead of waiting for a lock that another instance or process holds, and + // a call already waiting gives up the same way. A free lock is still taken. + // Without this, an instance closing while a sibling re-validates waits for + // that check, which offline can run until its request times out. Callable + // from any thread; it cannot be undone. + void cancel_lock_waits() noexcept + { + lock_waits_cancelled_->store(true, std::memory_order_release); } private: + // Writes contents to a new file beside the license and renames it over the + // license. False, with the new file removed again, if any step fails or the + // rename would change more than the contents. + bool replace_with_temp_file(const std::string& contents) const + { + // Follow a symlinked license path to the file itself, so the link keeps + // pointing at the license instead of being replaced by a copy of it. + auto target = path_; + std::error_code link_error; + if (std::filesystem::is_symlink(path_, link_error)) { + target = std::filesystem::canonical(path_, link_error); + if (link_error) { + return false; // a dangling link: writing in place creates its target + } + } + + // No license yet is fine (status reports that as an error too). One also + // known by another name, a hard link, would keep the old contents there. + std::error_code status_error; + const auto existing = std::filesystem::status(target, status_error); + const bool exists = std::filesystem::exists(existing); + std::error_code links_error; + if (exists && std::filesystem::hard_link_count(target, links_error) != 1) { + return false; + } + + auto temp = target; + temp += temp_suffix(); + { + std::ofstream file(temp, std::ios::trunc); + if (!file) { + return false; + } + // Keep the license's permissions, so one restricted to its owner + // stays that way, and set them before the contents land. Only on + // POSIX: on Windows these are just the read-only flag. + std::error_code mode_error; +#if !defined(_WIN32) + if (exists) { + std::filesystem::permissions(temp, existing.permissions(), mode_error); + } +#endif + if (!mode_error) { + file << contents; + } + file.close(); + if (mode_error || !file) { + std::error_code ignored; + std::filesystem::remove(temp, ignored); + return false; + } + } + + // Windows won't replace a file that another handle has open, as a + // sibling reading the license does for a moment (and a virus scanner + // may for longer), so try a few times there before giving up. +#if defined(_WIN32) + constexpr int attempts = 10; +#else + constexpr int attempts = 1; +#endif + std::error_code ec; + auto pause = std::chrono::milliseconds(1); + for (int attempt = 1;; ++attempt) { + std::filesystem::rename(temp, target, ec); + if (!ec) { + return true; + } + if (attempt == attempts) { + break; + } + std::this_thread::sleep_for(pause); + pause = std::min(pause * 2, std::chrono::milliseconds(100)); + } + std::filesystem::remove(temp, ec); + return false; + } + + // Unique to one write, so writers never share a temporary file, whether in + // different processes, on different threads, or in two plugin binaries + // loaded into the same host. + static std::string temp_suffix() + { +#if defined(_WIN32) + const auto process = static_cast(::_getpid()); +#else + const auto process = static_cast(::getpid()); +#endif + static std::atomic sequence{0}; + const auto ticks = std::chrono::steady_clock::now().time_since_epoch().count(); + return "." + std::to_string(process) + "-" + std::to_string(ticks) + "-" + + std::to_string(sequence.fetch_add(1)) + ".tmp"; + } + // Best-effort OS reason for a failed std::fstream open. Stream failures don't // portably set errno, so callers reset errno to 0 first and we only append a // reason when something was actually recorded. @@ -205,13 +332,18 @@ class file_license_store : public license_store { class file_store_lock : public store_lock_guard { public: - explicit file_store_lock(const std::filesystem::path& path) : lock_(path) {} + file_store_lock(const std::filesystem::path& path, const std::atomic& give_up) + : lock_(path, give_up) + { + } private: detail::file_lock lock_; }; std::filesystem::path path_; + // Shared so the store stays copyable; copies cancel together. + std::shared_ptr> lock_waits_cancelled_ = std::make_shared>(false); }; } // namespace moonbase diff --git a/modules/moonbase_licensing/juce/ActivationController.cpp b/modules/moonbase_licensing/juce/ActivationController.cpp index 8027bf4..268b467 100644 --- a/modules/moonbase_licensing/juce/ActivationController.cpp +++ b/modules/moonbase_licensing/juce/ActivationController.cpp @@ -146,7 +146,7 @@ ActivationController::ActivationController(ActivationConfig config) try { licensing_ = std::make_shared( - config_.toLicensingOptions(), std::move(store), deviceIds, transport); + config_.toLicensingOptions(), store, deviceIds, transport); } catch (const std::exception& ex) { @@ -159,10 +159,14 @@ ActivationController::ActivationController(ActivationConfig config) inventory_.emplace(config_.toLicensingOptions(), inventoryTransport); - cancelInFlight_ = [transport, inventoryTransport] + cancelInFlight_ = [transport, inventoryTransport, store] { transport->cancel(); inventoryTransport->cancel(); + // A worker waiting its turn on the license lock while another instance + // re-validates stops waiting, rather than hold up the host until that + // check is done. + store->cancel_lock_waits(); }; // A custom resolver's device_name() is arbitrary consumer code, and this // constructor runs inside a plugin editor's constructor, where an escaping @@ -218,9 +222,10 @@ ActivationController::~ActivationController() stopTimer(); ++updateGeneration_; // drop any queued update-flow continuations updateDownload_.reset(); // cancels + joins the installer download thread - // Unblock any in-flight request, then wait for the workers to finish, so no - // detached thread keeps running module code after we (and possibly the - // plugin binary) are gone. cancelInFlight_ makes the drain near-instant. + // Unblock any in-flight request or wait for the license lock, then wait for + // the workers to finish, so no detached thread keeps running module code + // after we (and possibly the plugin binary) are gone. cancelInFlight_ makes + // the drain near-instant. if (cancelInFlight_) cancelInFlight_(); threadPool_.stop(); diff --git a/modules/moonbase_licensing/juce/ActivationController.h b/modules/moonbase_licensing/juce/ActivationController.h index 3f3a1fe..e0959b7 100644 --- a/modules/moonbase_licensing/juce/ActivationController.h +++ b/modules/moonbase_licensing/juce/ActivationController.h @@ -86,8 +86,9 @@ class ActivationController : private juce::Timer, // with fake store / transport / fingerprint. The primary constructor builds // the real dependencies from the config; this one takes them ready-made. // cancelInFlight (optional) is invoked on teardown to interrupt a blocking - // request so workers can't outlive the controller (the real path wires it to - // the HTTP transport's cancel()). + // request or lock wait so workers can't outlive the controller (the real path + // wires it to the HTTP transports' cancel() and the store's + // cancel_lock_waits()). ActivationController(ActivationConfig config, std::shared_ptr licensing, juce::String deviceName = "Test Device", diff --git a/modules/moonbase_licensing/juce/WorkerPool.h b/modules/moonbase_licensing/juce/WorkerPool.h index 1ef1be9..e639cd7 100644 --- a/modules/moonbase_licensing/juce/WorkerPool.h +++ b/modules/moonbase_licensing/juce/WorkerPool.h @@ -72,11 +72,10 @@ class WorkerPool // Drops the jobs that haven't started, then waits for the running ones and // joins the workers. Callers cancel what they can first (the controller - // cancels its HTTP requests), so this is normally instant. It never gives up - // on a worker, because one left running could still be executing module code - // after the plugin binary is unloaded. The longest it waits is a job blocked - // on the license lock while another process holds it for its own network - // check, which that process's request timeouts bound. Safe to call twice. + // cancels its HTTP requests and any wait for the license lock), so this is + // normally instant. It never gives up on a worker, because one left running + // could still be executing module code after the plugin binary is unloaded. + // Safe to call twice. void stop() { if (threads_.empty()) diff --git a/modules/moonbase_licensing/moonbase/detail/file_lock.hpp b/modules/moonbase_licensing/moonbase/detail/file_lock.hpp index a70c292..7285c23 100644 --- a/modules/moonbase_licensing/moonbase/detail/file_lock.hpp +++ b/modules/moonbase_licensing/moonbase/detail/file_lock.hpp @@ -1,10 +1,14 @@ #pragma once +#include +#include #include +#include #include #include #include #include +#include #include #include "moonbase/errors.hpp" @@ -34,7 +38,7 @@ namespace moonbase::detail { // POSIX: flock(LOCK_EX) on the file's descriptor. Locks are per-open-file // description and cooperative; only callers that go through this class // observe each other. -// Windows: CRT _locking(_LK_LOCK) on a single-byte range. The range is +// Windows: CRT _locking on a single-byte range. The range is // required by the API but otherwise arbitrary; all callers using this class // lock the same range. _locking accepts ranges past EOF, so we deliberately // do not _chsize_s the file — SetEndOfFile on a sibling handle would race @@ -42,10 +46,18 @@ namespace moonbase::detail { // This intentionally avoids in public SDK headers because // store.hpp includes this detail header. // -// Throws moonbase::storage_error on unrecoverable open/lock failures. +// The lock is requested without blocking and retried every few milliseconds +// while someone else holds it, which can be for the length of their network +// check. A blocking request could not be called off, so a plugin closing +// while it waited would hold up the host until that check finished. Setting +// `give_up` ends the wait instead, with storage_error. No time limit +// otherwise: the holder lets go once its own request returns or times out. +// +// Throws moonbase::storage_error on unrecoverable open/lock failures, and +// when it gives up waiting. class file_lock { public: - explicit file_lock(const std::filesystem::path& path) + file_lock(const std::filesystem::path& path, const std::atomic& give_up) { const auto parent = path.parent_path(); if (!parent.empty()) { @@ -70,15 +82,6 @@ class file_lock { + std::generic_category().message(open_error)); } fd_ = fd; - - if (::_locking(fd_, _LK_LOCK, lock_range_size_) != 0) { - const auto err = errno; - ::_close(fd_); - fd_ = -1; - throw storage_error( - "Could not acquire license file lock: " - + std::generic_category().message(err)); - } #else fd_ = ::open(path.c_str(), O_RDWR | O_CREAT | O_CLOEXEC, 0644); if (fd_ < 0) { @@ -87,16 +90,17 @@ class file_lock { "Could not open license lock file: " + std::generic_category().message(err)); } +#endif - if (::flock(fd_, LOCK_EX) != 0) { - const auto err = errno; - ::close(fd_); - fd_ = -1; - throw storage_error( - "Could not acquire license file lock: " - + std::generic_category().message(err)); + auto pause = std::chrono::milliseconds(1); + while (!try_lock()) { + if (give_up.load(std::memory_order_acquire)) { + release(); + throw storage_error("Stopped waiting for the license file lock"); + } + std::this_thread::sleep_for(pause); + pause = std::min(pause * 2, max_pause_); } -#endif } file_lock(const file_lock&) = delete; @@ -121,6 +125,33 @@ class file_lock { ~file_lock() { release(); } private: + // True once the lock is ours, false while another holder has it. Closes the + // file and throws for any other failure. + bool try_lock() + { +#if defined(_WIN32) + if (::_locking(fd_, _LK_NBLCK, lock_range_size_) == 0) { + return true; + } + const auto err = errno; + if (err == EACCES) { + return false; + } +#else + if (::flock(fd_, LOCK_EX | LOCK_NB) == 0) { + return true; + } + const auto err = errno; + if (err == EWOULDBLOCK || err == EINTR) { + return false; + } +#endif + release(); + throw storage_error( + "Could not acquire license file lock: " + + std::generic_category().message(err)); + } + void release() noexcept { #if defined(_WIN32) @@ -142,6 +173,7 @@ class file_lock { #if defined(_WIN32) static constexpr long lock_range_size_ = 1; #endif + static constexpr std::chrono::milliseconds max_pause_{20}; int fd_ = -1; }; diff --git a/modules/moonbase_licensing/moonbase/store.hpp b/modules/moonbase_licensing/moonbase/store.hpp index 4fc5c36..73f148c 100644 --- a/modules/moonbase_licensing/moonbase/store.hpp +++ b/modules/moonbase_licensing/moonbase/store.hpp @@ -1,6 +1,9 @@ #pragma once +#include +#include #include +#include #include #include #include @@ -8,6 +11,13 @@ #include #include #include +#include + +#if defined(_WIN32) +#include +#else +#include +#endif #include @@ -143,6 +153,18 @@ class file_license_store : public license_store { } } + const auto contents = nlohmann::json(value).dump(2); + + // Write the new license beside the old one and rename it into place, so + // a reader sees one or the other and never a half-written file, which + // load_local_license would delete as corrupt. That holds for readers + // that don't take the lock, such as an instance loading at startup. + if (replace_with_temp_file(contents)) { + return; + } + + // Where that fails, as in a folder that takes no new files but lets the + // license itself be rewritten, write it in place. errno = 0; std::ofstream file(path_, std::ios::trunc); if (!file) { @@ -150,7 +172,7 @@ class file_license_store : public license_store { throw storage_error("Could not open local license file for writing: " + utf8_path(path_) + errno_suffix(err)); } - file << nlohmann::json(value).dump(2); + file << contents; file.flush(); if (!file) { throw storage_error("Could not write local license file: " + utf8_path(path_)); @@ -178,10 +200,115 @@ class file_license_store : public license_store { // process tree. auto lock_path = path_; lock_path += ".lock"; - return std::make_unique(lock_path); + return std::make_unique(lock_path, *lock_waits_cancelled_); + } + + // For shutting down: from here on lock_for_update() throws storage_error + // instead of waiting for a lock that another instance or process holds, and + // a call already waiting gives up the same way. A free lock is still taken. + // Without this, an instance closing while a sibling re-validates waits for + // that check, which offline can run until its request times out. Callable + // from any thread; it cannot be undone. + void cancel_lock_waits() noexcept + { + lock_waits_cancelled_->store(true, std::memory_order_release); } private: + // Writes contents to a new file beside the license and renames it over the + // license. False, with the new file removed again, if any step fails or the + // rename would change more than the contents. + bool replace_with_temp_file(const std::string& contents) const + { + // Follow a symlinked license path to the file itself, so the link keeps + // pointing at the license instead of being replaced by a copy of it. + auto target = path_; + std::error_code link_error; + if (std::filesystem::is_symlink(path_, link_error)) { + target = std::filesystem::canonical(path_, link_error); + if (link_error) { + return false; // a dangling link: writing in place creates its target + } + } + + // No license yet is fine (status reports that as an error too). One also + // known by another name, a hard link, would keep the old contents there. + std::error_code status_error; + const auto existing = std::filesystem::status(target, status_error); + const bool exists = std::filesystem::exists(existing); + std::error_code links_error; + if (exists && std::filesystem::hard_link_count(target, links_error) != 1) { + return false; + } + + auto temp = target; + temp += temp_suffix(); + { + std::ofstream file(temp, std::ios::trunc); + if (!file) { + return false; + } + // Keep the license's permissions, so one restricted to its owner + // stays that way, and set them before the contents land. Only on + // POSIX: on Windows these are just the read-only flag. + std::error_code mode_error; +#if !defined(_WIN32) + if (exists) { + std::filesystem::permissions(temp, existing.permissions(), mode_error); + } +#endif + if (!mode_error) { + file << contents; + } + file.close(); + if (mode_error || !file) { + std::error_code ignored; + std::filesystem::remove(temp, ignored); + return false; + } + } + + // Windows won't replace a file that another handle has open, as a + // sibling reading the license does for a moment (and a virus scanner + // may for longer), so try a few times there before giving up. +#if defined(_WIN32) + constexpr int attempts = 10; +#else + constexpr int attempts = 1; +#endif + std::error_code ec; + auto pause = std::chrono::milliseconds(1); + for (int attempt = 1;; ++attempt) { + std::filesystem::rename(temp, target, ec); + if (!ec) { + return true; + } + if (attempt == attempts) { + break; + } + std::this_thread::sleep_for(pause); + pause = std::min(pause * 2, std::chrono::milliseconds(100)); + } + std::filesystem::remove(temp, ec); + return false; + } + + // Unique to one write, so writers never share a temporary file, whether in + // different processes, on different threads, or in two plugin binaries + // loaded into the same host. + static std::string temp_suffix() + { +#if defined(_WIN32) + const auto process = static_cast(::_getpid()); +#else + const auto process = static_cast(::getpid()); +#endif + static std::atomic sequence{0}; + const auto ticks = std::chrono::steady_clock::now().time_since_epoch().count(); + return "." + std::to_string(process) + "-" + std::to_string(ticks) + "-" + + std::to_string(sequence.fetch_add(1)) + ".tmp"; + } + // Best-effort OS reason for a failed std::fstream open. Stream failures don't // portably set errno, so callers reset errno to 0 first and we only append a // reason when something was actually recorded. @@ -205,13 +332,18 @@ class file_license_store : public license_store { class file_store_lock : public store_lock_guard { public: - explicit file_store_lock(const std::filesystem::path& path) : lock_(path) {} + file_store_lock(const std::filesystem::path& path, const std::atomic& give_up) + : lock_(path, give_up) + { + } private: detail::file_lock lock_; }; std::filesystem::path path_; + // Shared so the store stays copyable; copies cancel together. + std::shared_ptr> lock_waits_cancelled_ = std::make_shared>(false); }; } // namespace moonbase diff --git a/tests/juce/controller_tests.cpp b/tests/juce/controller_tests.cpp index 56a1ffa..9ba1e23 100644 --- a/tests/juce/controller_tests.cpp +++ b/tests/juce/controller_tests.cpp @@ -2268,6 +2268,40 @@ TEST_CASE("destroying the controller mid-request cancels and joins without hangi CHECK(blocking->entered.load()); } +TEST_CASE("destroying the controller while another instance holds the license lock does not wait for it") +{ + controller_fixture fx; + auto claims = default_claims(); + claims["validated"] = now_seconds() - 3600; // past the throttle -> start() checks online, under the lock + fx.seedStored(fx.token(claims)); + fx.config.deviceIdResolver = fx.fingerprint; + + // Another instance holds the lock for its own online check, which offline + // can run until its request times out. Released after 3 s at the latest, so + // a controller that waits for it fails this test instead of hanging it. + moonbase::file_license_store sibling(compat::toFilesystemPath(fx.licenseFile)); + auto held = sibling.lock_for_update(); + juce::WaitableEvent tornDown; + std::thread releaser([&held, &tornDown] + { + tornDown.wait(3000); + held.reset(); + }); + + // The primary constructor, so the teardown runs exactly as in a plugin. + auto controller = std::make_unique(fx.config); + controller->start(); + pumpFor(300); // the worker is waiting for the lock by now + + const auto before = std::chrono::steady_clock::now(); + controller.reset(); + const auto teardown = std::chrono::steady_clock::now() - before; + tornDown.signal(); + releaser.join(); + + CHECK(teardown < std::chrono::milliseconds(1500)); +} + TEST_CASE("the worker pool runs every job it is given") { std::atomic ran{0}; diff --git a/tests/store_tests.cpp b/tests/store_tests.cpp index 1929877..528c80a 100644 --- a/tests/store_tests.cpp +++ b/tests/store_tests.cpp @@ -143,6 +143,86 @@ TEST_CASE("file_license_store removes an unparseable license file and reports no std::filesystem::remove(path, ec); } +TEST_CASE("file_license_store readers never catch the license half-written") +{ + // Readers don't take the lock (an instance loading its license at startup + // doesn't), and load_local_license deletes a file it can't parse. So a reader + // that caught a write half done would delete the license, and the customer + // would have to activate again. + const auto dir = std::filesystem::temp_directory_path() / ("moonbase-cpp-torn-" + unique_suffix()); + std::filesystem::create_directories(dir); + const auto path = dir / "license.mb"; + + auto small = sample_license(); + auto large = sample_license(); + large.token = std::string(64 * 1024, 'x'); // takes several writes, so a wide window + + file_license_store writer(path); + writer.store_local_license(small); + + std::atomic done{false}; + std::atomic reads{0}; + std::atomic misses{0}; + std::thread reading([&] { + file_license_store reader(path); + while (!done.load()) { + try { + if (!reader.load_local_license()) { + ++misses; + } + } catch (const storage_error&) { + ++misses; + } + ++reads; + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + }); + + for (int i = 0; reads.load() < 100 && i < 100000; ++i) { + writer.store_local_license(i % 2 == 0 ? large : small); + } + done = true; + reading.join(); + + CHECK(reads.load() > 0); + CHECK(misses.load() == 0); + CHECK(std::filesystem::exists(path)); + + // Nothing left beside it but the license itself. + int entries = 0; + for (const auto& entry : std::filesystem::directory_iterator(dir)) { + (void) entry; + ++entries; + } + CHECK(entries == 1); + + std::error_code ec; + std::filesystem::remove_all(dir, ec); +} + +TEST_CASE("file_license_store keeps a hard-linked license shared") +{ + // Writing a new file and renaming it into place would replace only the name + // written through, leaving the other name on the old license. + const auto dir = std::filesystem::temp_directory_path() / ("moonbase-cpp-hardlink-" + unique_suffix()); + std::filesystem::create_directories(dir); + const auto path = dir / "license.mb"; + const auto alias = dir / "alias.mb"; + file_license_store(path).store_local_license(sample_license()); + std::filesystem::create_hard_link(path, alias); + + auto updated = sample_license(); + updated.id = "license-456"; + file_license_store(alias).store_local_license(updated); + + const auto loaded = file_license_store(path).load_local_license(); + REQUIRE(loaded.has_value()); + CHECK(loaded->id == "license-456"); + + std::error_code ec; + std::filesystem::remove_all(dir, ec); +} + TEST_CASE("file_license_store::lock_for_update serializes concurrent acquirers") { const auto path = std::filesystem::temp_directory_path() / @@ -290,6 +370,52 @@ TEST_CASE("file_license_store::lock_for_update survives delete_local_license") std::filesystem::remove(lock_path, ec); } +TEST_CASE("file_license_store::cancel_lock_waits ends a wait for a lock held elsewhere") +{ + const auto path = std::filesystem::temp_directory_path() / + ("moonbase-cpp-lock-cancel-" + unique_suffix() + ".json"); + + // Another instance holds the lock, as it does for its whole online check. + file_license_store sibling(path); + file_license_store store(path); + auto held = sibling.lock_for_update(); + + std::atomic finished{false}; + std::atomic gave_up{false}; + std::thread waiting([&] { + try { + auto guard = store.lock_for_update(); + } catch (const storage_error&) { + gave_up = true; + } + finished = true; + }); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); // waiting by now + + store.cancel_lock_waits(); + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(2); + while (!finished.load() && std::chrono::steady_clock::now() < deadline) { + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } + CHECK(finished.load()); + CHECK(gave_up.load()); + held.reset(); // a waiter that ignored the cancel finishes now, rather than hang the run + waiting.join(); + + // Later calls don't wait for a lock that is taken either... + held = sibling.lock_for_update(); + CHECK_THROWS_AS((void) store.lock_for_update(), storage_error); + // ...but still take one that is free. + held.reset(); + CHECK(store.lock_for_update() != nullptr); + + std::error_code ec; + std::filesystem::remove(path, ec); + auto lock_path = path; + lock_path += ".lock"; + std::filesystem::remove(lock_path, ec); +} + TEST_CASE("memory_license_store::lock_for_update blocks concurrent mutations") { // Regression: the default store used to return nullptr from @@ -479,4 +605,88 @@ TEST_CASE("file_license_store reports a storage_error with the path when the dir std::error_code ec; std::filesystem::remove_all(dir, ec); } + +TEST_CASE("file_license_store keeps a license restricted to its owner that way") +{ + const auto dir = std::filesystem::temp_directory_path() / ("moonbase-cpp-mode-" + unique_suffix()); + std::filesystem::create_directories(dir); + const auto path = dir / "license.mb"; + + // A umask that leaves new files readable by everyone, as most do. + const auto previous_umask = ::umask(022); + file_license_store store(path); + store.store_local_license(sample_license()); + REQUIRE(::chmod(path.c_str(), 0600) == 0); + + store.store_local_license(sample_license()); + ::umask(previous_umask); + + struct stat info {}; + REQUIRE(::stat(path.c_str(), &info) == 0); + CHECK((info.st_mode & 0777) == 0600); + + std::error_code ec; + std::filesystem::remove_all(dir, ec); +} + +TEST_CASE("file_license_store writes through a symlinked license path") +{ + // The link must keep pointing at the license, not be replaced by a copy + // while what it pointed at keeps the old one. + const auto dir = std::filesystem::temp_directory_path() / ("moonbase-cpp-symlink-" + unique_suffix()); + std::filesystem::create_directories(dir); + const auto target = dir / "target.mb"; + const auto link = dir / "license.mb"; + file_license_store(target).store_local_license(sample_license()); + std::filesystem::create_symlink("target.mb", link); // relative, as links often are + + auto updated = sample_license(); + updated.id = "license-456"; + file_license_store(link).store_local_license(updated); + + CHECK(std::filesystem::is_symlink(link)); + const auto loaded = file_license_store(target).load_local_license(); + REQUIRE(loaded.has_value()); + CHECK(loaded->id == "license-456"); + + int entries = 0; + for (const auto& entry : std::filesystem::directory_iterator(dir)) { + (void) entry; + ++entries; + } + CHECK(entries == 2); // the link and its target, no leftovers + + std::error_code ec; + std::filesystem::remove_all(dir, ec); +} + +TEST_CASE("file_license_store rewrites the license in place in a folder that takes no new files") +{ + if (::geteuid() == 0) + return; + + const auto dir = std::filesystem::temp_directory_path() / + ("moonbase-cpp-fixed-dir-" + unique_suffix()); + std::filesystem::create_directories(dir); + const auto path = dir / "license.mb"; + { + file_license_store seed(path); + seed.store_local_license(sample_license()); + } + // The license stays writable, but nothing new can be created beside it, so + // there is nowhere to write a replacement first. + REQUIRE(::chmod(dir.c_str(), 0500) == 0); + + file_license_store store(path); + auto updated = sample_license(); + updated.id = "license-456"; + CHECK_NOTHROW(store.store_local_license(updated)); + const auto loaded = store.load_local_license(); + REQUIRE(loaded.has_value()); + CHECK(loaded->id == "license-456"); + + ::chmod(dir.c_str(), 0700); + std::error_code ec; + std::filesystem::remove_all(dir, ec); +} #endif // _WIN32