Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,15 @@ patch is `## [<component> 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

Expand Down
57 changes: 56 additions & 1 deletion src/engine/clangd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string, std::string> 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<Clock::time_point> due;
};
static constexpr int LOCK_RETRIES { 3 };
static constexpr std::chrono::milliseconds LOCK_RETRY_DELAY { 500 };
std::map<std::string, LockRetry, std::less<>> lockRetries_;
std::string stubDirectory_; // stand-ins for modules nothing usable provides (robustness design C2)

// Plan.
bool planApplied_ { false };
Expand Down Expand Up @@ -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<std::string> released;
for (auto it = aside_.begin(); it != aside_.end();) {
Expand Down Expand Up @@ -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_) {
Expand Down Expand Up @@ -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_();
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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()) {
Expand Down
6 changes: 6 additions & 0 deletions src/engine/clangd/process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) };
Expand Down
5 changes: 5 additions & 0 deletions src/engine/clangd/process.cppm
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,11 @@ std::vector<std::string> 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 <cache>/.locks/<hash>: 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
Expand Down
87 changes: 87 additions & 0 deletions tests/unit/test_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down Expand Up @@ -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<BuildingProcess::Shared>();
cld::Options options;
options.executable = executable;
options.version = "23.1.0";
options.primeImplementationUnits = false;
options.processFactory = [shared] { return std::make_unique<BuildingProcess>(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;
Expand Down
Loading