implementations/Crafter.Build-Clang.cpp

@@ -215,24 +215,13 @@ void Configuration::GetInterfacesAndImplementations(std::span<fs::path> interfac
 BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, std::shared_future<BuildResult>>& depResults, std::mutex& depMutex) {
     // Reset per-build cached state on every Module/ModulePartition so that
     // successive Build() calls on the same Configuration re-evaluate mtimes
-    // (incremental-rebuild test scenarios).
+    // (incremental-rebuild test scenarios). Walks cfg.dependencies recursively
-    //
+    // with a seen-set so diamond deps don't loop.
-    // Reset ONLY this configuration's own modules — never recurse into
+    {
-    // dependencies. Configuration objects are shared across the dependency DAG
+        std::unordered_set<Configuration*> resetSeen;
-    // (diamond deps point at the same Configuration*), and every config in the
+        std::function<void(Configuration*)> reset = [&](Configuration* c) {
-    // tree gets its own Build() call: the per-PcmDir builder registered in
+            if (!resetSeen.insert(c).second) return;
-    // depResults resets its own state here, at the top of its own Build(),
+            for (auto& iface : c->interfaces) {
-    // sequenced before that config's compile threads spawn. Recursing into
-    // dependencies from here would re-clear a shared dependency's `compiled`
-    // atomic from a parent/sibling thread *while that dependency's own Build()
-    // is concurrently compiling it* — after its module-compile thread set the
-    // flag true and exited but before its impl/partition waiter ran
-    // compiled.wait(false). The waiter would then block forever on a flag
-    // nothing re-signals: the build freezes mid-compile, idle, with no compiler
-    // process alive (issue #16). Cross-config module state is consulted only via
-    // PCM file mtimes and the depResults futures, never via these flags, so the
-    // narrower reset is correct.
-    for (auto& iface : config.interfaces) {
                 iface->checked = false;
                 iface->compiled.store(false);
                 for (auto& part : iface->partitions) {
@@ -240,6 +229,12 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, s
                     part->compiled.store(false);
                 }
             }
+            for (Configuration* dep : c->dependencies) {
+                reset(dep);
+            }
+        };
+        reset(&config);
+    }
 
     // Auto-detect the WASI sysroot before any compile step runs so BuildStdPcm
     // and the main compile command see the same value. Linux-only — Windows

project.cpp

@@ -113,7 +113,6 @@ extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> a
         // for project.so.
         cfg.AddTest("ConcurrentCacheRace").Dependencies({ crafterBuildLib.get() })
             .LinkFlag("-Wl,--export-dynamic").LinkFlag("-ldl");
-        cfg.AddTest("ConcurrentDependencyReset").Dependencies({ crafterBuildLib.get() });
     }
 
     return cfg;

tests/ConcurrentDependencyReset/fixture/app/main.cpp

@@ -1,7 +0,0 @@
-import std;
-import DepMod;
-
-int main() {
-    std::print("{}", dep_value() * 2);
-    return 0;
-}

tests/ConcurrentDependencyReset/fixture/deplib/DepMod-impl.cpp

@@ -1,7 +0,0 @@
-module DepMod;
-
-// Implementation unit of the DepMod interface. Because it declares
-// `module DepMod;` it gains an intra-config module dependency on the DepMod
-// interface, so its compile waits on DepMod's `compiled` flag — the exact
-// flag a stale recursive reset used to clobber (issue #16).
-int dep_value() { return 21; }

tests/ConcurrentDependencyReset/fixture/deplib/DepMod.cppm

@@ -1,2 +0,0 @@
-export module DepMod;
-export int dep_value();

tests/ConcurrentDependencyReset/main.cpp

@@ -1,126 +0,0 @@
-import std;
-import Crafter.Build;
-namespace fs = std::filesystem;
-using namespace Crafter;
-
-// Regression for the concurrent dependency-reset deadlock (issue #16).
-//
-// Build() resets each Module/ModulePartition's per-build `compiled` /`checked`
-// flags so a reused Configuration re-evaluates mtimes. That reset MUST stay
-// scoped to the configuration being built. It used to recurse into
-// cfg.dependencies — but dependency Configurations are shared across the build
-// DAG and each is compiled concurrently by its own Build() call. A parent's
-// recursive reset could therefore clear a dependency's module `compiled` flag
-// *after* that dependency's module-compile thread had set it true and exited,
-// but before an intra-config waiter (its impl / a dependent partition) ran
-// `compiled.wait(false)`. The waiter then blocked forever on a flag nothing
-// re-signalled: the build froze mid-compile, idle, with no compiler process
-// alive.
-//
-// This test pins the invariant deterministically, without depending on thread
-// timing: build a static-lib dependency fully (its module ends up compiled),
-// then build a consumer that depends on it while the dependency is already
-// cached in depResults (so it is NOT rebuilt). Before the fix, the consumer's
-// recursive reset flipped the dependency's module `compiled` flag back to false
-// and, because the dependency never rebuilds, it stayed false. After the fix
-// the consumer resets only its own modules, so the dependency's flag is left
-// intact. We assert it is still set once the consumer build completes.
-int main() {
-    fs::path fixtureRoot = fs::current_path() / "tests" / "ConcurrentDependencyReset" / "fixture";
-
-    // Cold output: a stale archive/PCM from a previous run would let the
-    // dependency build short-circuit and skip the module compile we rely on.
-    std::error_code ec;
-    fs::remove_all(fixtureRoot / "deplib" / "build", ec);
-    fs::remove_all(fixtureRoot / "deplib" / "bin", ec);
-    fs::remove_all(fixtureRoot / "app" / "build", ec);
-    fs::remove_all(fixtureRoot / "app" / "bin", ec);
-
-    Configuration dep;
-    dep.path = fixtureRoot / "deplib";
-    dep.name = "depmod";
-    dep.outputName = "depmod";
-    dep.target = HostTarget();
-    dep.type = ConfigurationType::LibraryStatic;
-    {
-        std::array<fs::path, 1> ifaces = { "DepMod" };
-        std::array<fs::path, 1> impls = { "DepMod-impl" };
-        dep.GetInterfacesAndImplementations(ifaces, impls);
-    }
-
-    if (dep.interfaces.size() != 1) {
-        std::println(std::cerr, "expected 1 dependency interface, got {}", dep.interfaces.size());
-        return 1;
-    }
-
-    {
-        std::unordered_map<fs::path, std::shared_future<BuildResult>> depResults;
-        std::mutex depMutex;
-        BuildResult r = Build(dep, depResults, depMutex);
-        if (!r.result.empty()) {
-            std::println(std::cerr, "dependency build failed: {}", r.result);
-            return 1;
-        }
-        if (!dep.interfaces[0]->compiled.load()) {
-            std::println(std::cerr, "dependency module not marked compiled after its own build");
-            return 1;
-        }
-        // Seed a consumer-facing cache so the consumer reuses this build instead
-        // of rebuilding the dependency — mirrors how a shared dep is built once
-        // and consumed by multiple parents through depResults.
-        std::promise<BuildResult> promise;
-        promise.set_value(std::move(r));
-        std::shared_future<BuildResult> ready = promise.get_future().share();
-
-        std::unordered_map<fs::path, std::shared_future<BuildResult>> consumerDeps;
-        consumerDeps.emplace(dep.PcmDir(), ready);
-
-        Configuration app;
-        app.path = fixtureRoot / "app";
-        app.name = "resetapp";
-        app.outputName = "resetapp";
-        app.target = HostTarget();
-        app.type = ConfigurationType::Executable;
-        app.dependencies = { &dep };
-        {
-            std::array<fs::path, 0> appIfaces = {};
-            std::array<fs::path, 1> appImpls = { "main" };
-            app.GetInterfacesAndImplementations(appIfaces, appImpls);
-        }
-
-        BuildResult ar = Build(app, consumerDeps, depMutex);
-        if (!ar.result.empty()) {
-            std::println(std::cerr, "consumer build failed: {}", ar.result);
-            return 1;
-        }
-
-        // The core regression assertion: building the consumer must not have
-        // reset the (cached, never-rebuilt) dependency's module flag. Before the
-        // fix this was false and an intra-config waiter would have deadlocked.
-        if (!dep.interfaces[0]->compiled.load()) {
-            std::println(std::cerr,
-                "FAIL: consumer build reset a cached dependency's module 'compiled' "
-                "flag (issue #16 regression) — this is the state that deadlocks a "
-                "concurrent build");
-            return 1;
-        }
-
-        fs::path bin = app.BinDir() / "resetapp";
-        if (!fs::exists(bin)) {
-            std::println(std::cerr, "consumer binary not produced at {}", bin.string());
-            return 1;
-        }
-        auto run = RunCommandWithTimeout(bin.string(), std::chrono::seconds(10));
-        if (run.exitCode != 0 || run.timedOut || run.crashed) {
-            std::println(std::cerr, "consumer exe did not exit cleanly: exit={} output={}",
-                run.exitCode, run.output);
-            return 1;
-        }
-        if (run.output != "42") {
-            std::println(std::cerr, "expected '42', got '{}'", run.output);
-            return 1;
-        }
-    }
-
-    return 0;
-}