fix: cancel license lock waits on teardown and write the license atomically

docs/core-sdk.md

@@ -336,7 +336,11 @@ auto resolver = std::make_shared<my_resolver>();
 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

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:

include/moonbase/detail/file_lock.hpp

@@ -1,10 +1,14 @@
 #pragma once
 
+#include <algorithm>
+#include <atomic>
 #include <cerrno>
+#include <chrono>
 #include <cstdio>
 #include <filesystem>
 #include <string>
 #include <system_error>
+#include <thread>
 #include <utility>
 
 #include "moonbase/errors.hpp"
@@ -34,18 +38,26 @@ 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
 // against the byte lock held by an earlier acquirer and surface as EACCES.
 // This intentionally avoids <windows.h> 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<bool>& 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;
 };
 

include/moonbase/store.hpp

@@ -1,13 +1,23 @@
 #pragma once
 
+#include <algorithm>
+#include <atomic>
 #include <cerrno>
+#include <chrono>
 #include <filesystem>
 #include <fstream>
 #include <memory>
 #include <mutex>
 #include <optional>
 #include <string>
 #include <system_error>
+#include <thread>
+
+#if defined(_WIN32)
+#include <process.h>
+#else
+#include <unistd.h>
+#endif
 
 #include <nlohmann/json.hpp>
 
@@ -143,14 +153,26 @@ 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) {
             const int err = errno;
             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<file_store_lock>(lock_path);
+        return std::make_unique<file_store_lock>(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<long long>(::_getpid());
+#else
+        const auto process = static_cast<long long>(::getpid());
+#endif
+        static std::atomic<unsigned long long> 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<bool>& 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<std::atomic<bool>> lock_waits_cancelled_ = std::make_shared<std::atomic<bool>>(false);
 };
 
 } // namespace moonbase

modules/moonbase_licensing/juce/ActivationController.cpp

@@ -146,7 +146,7 @@ ActivationController::ActivationController(ActivationConfig config)
     try
     {
         licensing_ = std::make_shared<moonbase::licensing>(
-            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();

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<moonbase::licensing> licensing,
                          juce::String deviceName = "Test Device",