diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b7f5c15..8c234c97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,15 @@ patch is `## [ X.Y.Z]` (`server`, `vscode`, `clion`, `zed`, `claude-c not read its database yet, and answered with what the module index alone knows -- the declaration. Only an engine's reports count now. CI met it as the first round of `stability` on a fresh runner; a failed `cli` check of the conformance runner now says what the command printed. +- **On macOS a file could stay without its modules for the whole session after a cold start**: no + definition or hover across an import, and "module ... could not be built: Failed to open lock file + anchor ..." until the file was edited. clangd builds several files' prerequisites at once when a + session starts, and the lock that orders builds of one module sometimes cannot be opened then (the + engine's code since mcppls-clangd 0.0.20; met in 1 to 2 of 81 stability rounds on darwin-arm64 + with this release's engine, none with 0.22.0's). Nothing about the project is wrong and the same + build asked again succeeds: the server no longer records it as the file's module failure, and asks + clangd to build that file again half a second later (`forceRebuild`), up to three times until the + sources change. The race itself is follow-up in the engine. ### Added diff --git a/src/engine/clangd.cpp b/src/engine/clangd.cpp index 74b76742..d4e7b5a0 100644 --- a/src/engine/clangd.cpp +++ b/src/engine/clangd.cpp @@ -169,7 +169,17 @@ class ClangdEngine final : public Engine { // A3 (plan 2026-10-14): importer (path key) -> why a module it imports could not be built, when the unit that failed // is none the plan names; until the sources change. std::map unplacedFailures_; - std::string stubDirectory_; // stand-ins for modules nothing usable provides (robustness design C2) + // A failure of the engine's own module lock (transient_module_failure): the importer (path key) is asked to be + // built again at `due`, at most LOCK_RETRIES times until the sources change -- a session that met the race + // recovers instead of keeping a file without its modules until the next edit. + struct LockRetry { + int asked { 0 }; + std::optional due; + }; + static constexpr int LOCK_RETRIES { 3 }; + static constexpr std::chrono::milliseconds LOCK_RETRY_DELAY { 500 }; + std::map> lockRetries_; + std::string stubDirectory_; // stand-ins for modules nothing usable provides (robustness design C2) // Plan. bool planApplied_ { false }; @@ -1257,6 +1267,7 @@ class ClangdEngine final : public Engine { lastSourceChangeAt_ = Clock::now(); modulesFailedAt_.clear(); // a module that still does not compile is reported again unplacedFailures_.clear(); + lockRetries_.clear(); // A unit set aside because its module did not compile goes back to clangd: the module may compile now. std::vector released; for (auto it = aside_.begin(); it != aside_.end();) { @@ -1599,6 +1610,7 @@ class ClangdEngine final : public Engine { if (upSince_) consider(*upSince_ + RECOVERED_AFTER); for (const auto& [uri, closing] : closingAfterBuild_) consider(closing.second); consider(diskRecheckAt_); + for (const auto& [key, retry] : lockRetries_) consider(retry.due); if (accepting_) consider(spin_.next_due()); // only acted on while accepting (handle_spins_) // While a reading of clangd's CPU is on its way, its event is what wakes the loop. if (!cpuReadInFlight_) { @@ -1739,6 +1751,7 @@ class ClangdEngine final : public Engine { request_restart_("clangd did not answer initialize"); } if (closureRetryCheck_ && !primer_.busy()) retry_after_closure_rebuilt_(); + rebuild_after_lock_failure_(now); if (ledgerCheckAt_ && *ledgerCheckAt_ <= now) { ledgerCheckAt_.reset(); // asked now; hand_back_ledger_files_ sets the next one, if a term is still running hand_back_ledger_files_(); @@ -2036,6 +2049,7 @@ class ClangdEngine final : public Engine { if (!process_) process_ = make_process_(); lastConfig_ = config; building_.clear(); + lockRetries_.clear(); // a new clangd builds every file it is given anyway paused_ = false; // Issue 86 S4: what this start spends of the sandbox's table -- one slot for each read-only grant, for the life // of the server process (openkal through 0.25; a platform that returns them makes the count harmless). @@ -3412,9 +3426,50 @@ class ClangdEngine final : public Engine { host_->status_changed(); } + // The importers whose modules clangd could not build for its own lock: each open one is asked to be built again, + // its preamble and with it its prerequisites (clangd's `forceRebuild`), the text as clangd has it. A unit clangd has + // that the editor does not (a prime unit, one opened for the index) is the primer's and the index's to ask again. + void rebuild_after_lock_failure_(Clock::time_point now) { + if (lockRetries_.empty() || !accepting_) return; + for (auto& [key, retry] : lockRetries_) { + if (!retry.due || *retry.due > now) continue; + retry.due.reset(); + ++retry.asked; + for (const auto& document : host_->documents()) { + if (document.path.empty() || base::path_key(document.path) != key) continue; + if (excluded_path_(document.path) || quarantined_(document.path) || held_path_(document.path) || doomed_path_(document.path)) continue; + const auto sent = sentVersions_.find(document.uri); + if (sent == sentVersions_.end()) continue; // not open in this clangd + if (!send_(lsp::make_notification("textDocument/didChange", + Json { { "textDocument", Json { { "uri", document.uri }, { "version", sent->second } } }, + { "contentChanges", Json::array() }, { "wantDiagnostics", true }, { "forceRebuild", true } }))) { + continue; + } + log::info("asking clangd to build {} again ({}): its module lock failed, attempt {} of {}", document.path, host_->root_directory(), + retry.asked, LOCK_RETRIES); + host_->record_event("module-lock-rebuild", Json { { "file", document.path }, { "attempt", retry.asked } }); + } + } + } + void handle_module_failure_(const Json& failure) { ModuleFailure parsed { failure.value("module", std::string {}), failure.value("reason", std::string {}), failure.value("source", std::string {}), failure.value("importer", std::string {}) }; + // The engine's module lock, not the module (transient_module_failure): the importer is built again shortly, and + // nothing is recorded against a module or a file -- not a stand-in, not a doomed closure, not an unplaced failure + // whose words an `ast_file_not_found` would then repeat until the next edit. + if (transient_module_failure(parsed) && !parsed.importer.empty()) { + const std::string importer { base::path_key(base::normalize_path(parsed.importer)) }; + LockRetry& retry { lockRetries_[importer] }; + if (retry.asked < LOCK_RETRIES) { + if (!retry.due) retry.due = Clock::now() + LOCK_RETRY_DELAY; + log::info("clangd could not open its module lock while building what {} imports ({}): {}; it is asked again", parsed.importer, + host_->root_directory(), parsed.reason); + host_->record_event("module-lock-retry", Json { { "importer", parsed.importer }, { "asked", retry.asked }, { "reason", parsed.reason } }); + return; + } + // Asked LOCK_RETRIES times and still failing: not the race this is for. Reported as any failure is. + } // mcppls-clangd names the importer and the unit that failed to compile, not the module: it is // the one the plan gave that unit. if (parsed.module.empty() && !parsed.failedSource.empty()) { diff --git a/src/engine/clangd/process.cpp b/src/engine/clangd/process.cpp index dbf4ff25..2bdb4fdc 100644 --- a/src/engine/clangd/process.cpp +++ b/src/engine/clangd/process.cpp @@ -615,6 +615,12 @@ FailureKind failure_kind(const ModuleFailure& failure) { return FailureKind::other; } +bool transient_module_failure(const ModuleFailure& failure) { + // The lock is the engine's own and names no unit: a failure that names one (a worker that compiled and failed) + // is the unit's, whatever words follow. + return failure.failedSource.empty() && failure.reason.find("Failed to open lock file anchor") != std::string::npos; +} + FailureAction failure_action(FailureKind kind, const FailureContext& context) { if (kind == FailureKind::unresolved && context.providerPlanned && !context.providerRead) return FailureAction::ignore; const bool kitHelps { kind == FailureKind::compile || (kind == FailureKind::unresolved && !context.providerPlanned) }; diff --git a/src/engine/clangd/process.cppm b/src/engine/clangd/process.cppm index c07d23f2..5d59097e 100644 --- a/src/engine/clangd/process.cppm +++ b/src/engine/clangd/process.cppm @@ -242,6 +242,11 @@ std::vector stale_module_builds(std::string_view databaseDirectory, // built. `compile`: the unit was found and did not compile; its importers get errors, not a hang (S3). enum class FailureKind { unresolved, compile, other }; FailureKind failure_kind(const ModuleFailure& failure); +// A failure of the engine's module cache, not of a module: the lock that orders builds of one unit could not be +// opened ("Failed to open lock file anchor /.locks/: No such file or directory", seen on macOS +// while several files' prerequisites are built at once; mcppls-clangd 0.0.20 on). Nothing about the project +// is wrong, and the same build asked again succeeds: the importer is rebuilt, not recorded as failing. +bool transient_module_failure(const ModuleFailure& failure); // What a module failure clangd reported means for the session (plan 2026-09-27 Q1-1, Q1-4). // ignore the unit joined an engine database this clangd has not read yet: the report is about the old one diff --git a/tests/unit/test_server.cpp b/tests/unit/test_server.cpp index 3bee73f5..e6e149d5 100644 --- a/tests/unit/test_server.cpp +++ b/tests/unit/test_server.cpp @@ -685,6 +685,17 @@ UTest server_tests{"server", [] { expect(worker->importer == "/p/src/plain-importer.cpp") << worker->importer; expect(worker->failedSource == "/p/src/broken/e.cppm") << worker->failedSource; expect(cld::failure_kind(*worker) == cld::FailureKind::compile) << worker->reason; + expect(!cld::transient_module_failure(*worker)); + // The engine's own lock, on macOS under concurrent prerequisite builds (release 0.23.0's stability rounds): + // a failure of no unit, which the same build asked again does not repeat. + const auto anchor = cld::parse_module_failure( + "E[05:15:40.539] Failed to build module prerequisites for /p/src/good/user.cppm; due to Failed to open lock file anchor " + "/c/contexts/clangd-23.1.0-mcppls.12-openkal-73feaa0a7a52/cdb/.cache/clangd/modules/.locks/90D2A255B17AD65E: No such file or directory"); + expect(fatal(anchor.has_value())); + expect(anchor->importer == "/p/src/good/user.cppm") << anchor->importer; + expect(anchor->module.empty() && anchor->failedSource.empty()) << anchor->module << " " << anchor->failedSource; + expect(cld::transient_module_failure(*anchor)) << anchor->reason; + expect(cld::failure_kind(*anchor) == cld::FailureKind::other); const auto windows = cld::parse_module_failure( R"(E[03:15:19.435] Failed to build module prerequisites for C:\p\main.cpp; due to module worker failed: C:\p\a b\e.cppm:12:3: fatal error: 'x.h' file not found)"); expect(fatal(windows.has_value())); @@ -2005,6 +2016,82 @@ UTest server_tests{"server", [] { std::filesystem::remove_all(root, ignored); }; + "a module lock clangd could not open is asked again, not recorded as the file's failure"_test = [] { + std::error_code ignored; + using mcppls::base::join_path; + using namespace std::chrono_literals; + const std::string root { base::canonical_path(join_path(base::spelled(os::dirs::temp()), + std::format("mcppls-test-lock-anchor-{}", std::chrono::steady_clock::now().time_since_epoch().count()))) }; + (void)std::filesystem::create_directories(join_path(root, "src"), ignored); + const std::string executable { join_path(root, "clangd") }; + (void)os::fs::write(executable, "pretend-clangd"); + const std::string main { join_path(root, "src/main.cpp") }; + const std::string greet { join_path(root, "src/greet.cppm") }; + const std::string mainText { "import hello.greet;\nint main() { return hello::add(1, 2); }\n" }; + (void)os::fs::write(main, mainText); + (void)os::fs::write(greet, "export module hello.greet;\nexport namespace hello { int add(int a, int b) { return a + b; } }\n"); + auto shared = std::make_shared(); + cld::Options options; + options.executable = executable; + options.version = "23.1.0"; + options.primeImplementationUnits = false; + options.processFactory = [shared] { return std::make_unique(shared); }; + EditorHost host { root }; + auto engine = cld::make_engine(std::move(options)); + engine->start(host); + host.pump(*engine); + mcppls::normalize::EnginePlan plan; + plan.entries.push_back(mcppls::normalize::EngineEntry { root, main, { "clang++", "-std=c++23", "-c", main }, "", "", { "hello.greet" }, {} }); + plan.entries.push_back(mcppls::normalize::EngineEntry { root, greet, { "clang++", "-std=c++23", "-c", greet }, "hello.greet", "hello.greet", {}, {} }); + engine->apply(&plan); + host.pump(*engine); + const eng::DocumentView mainView { mcppls::base::path_to_uri(main), main, "cpp", 1, mainText }; + host.open.push_back(mainView); + engine->document(eng::DocumentEvent { eng::DocumentChange::opened, mainView }); + engine->handle_timers(); + for (int turn { 0 }; turn < 3; ++turn) host.pump(*engine); + expect(fatal(std::ranges::count(BuildingProcess::files(*shared, "textDocument/didOpen"), main) == 1)); + const auto rebuilds = [&] { + const std::lock_guard lock { shared->mutex }; + return std::ranges::count_if(shared->sent, [&](const Json& message) { + return message.value("method", std::string {}) == "textDocument/didChange" && message["params"]["textDocument"]["uri"] == mainView.uri + && message["params"].value("forceRebuild", false) && message["params"]["contentChanges"].empty(); + }); + }; + const auto fail = [&] { + shared->log(std::format("E[05:15:40.539] Failed to build module prerequisites for {}; due to Failed to open lock file anchor " + "{}/.cache/clangd/modules/.locks/90D2A255B17AD65E: No such file or directory", main, root)); + host.pump(*engine); + }; + const auto wait_out = [&] { + std::this_thread::sleep_for(600ms); + engine->handle_timers(); + host.pump(*engine); + }; + const auto unplaced = [&] { return std::ranges::count(host.events, std::string { "module-failed-unplaced" }); }; + + fail(); + expect(rebuilds() == 0) << "not at once: the builds that raced are given a moment"; + expect(unplaced() == 0) << "the engine's lock is no failure of the file's"; + wait_out(); + expect(rebuilds() == 1) << "main.cpp is built again, the text clangd has, its prerequisites with it"; + wait_out(); + expect(rebuilds() == 1) << "once for each failure"; + + // A lock that keeps failing is not the race: after LOCK_RETRIES it is reported as any failure is. + fail(); + wait_out(); + fail(); + wait_out(); + expect(rebuilds() == 3); + fail(); + wait_out(); + expect(rebuilds() == 3) << "no fourth"; + expect(unplaced() == 1) << "reported once the retries are spent"; + engine->shut_down(); + std::filesystem::remove_all(root, ignored); + }; + "what clangd could not find under the provisional model is not carried into the build tool's plan"_test = [] { std::error_code ignored; using mcppls::base::join_path;