Race condition in parse-ns can break compilation under parallel build
Completed
Description fields
Description
I found this issue by trying to compile reagent with parallel-build. Once parallel-build is enabled the compilation fails most of the times, creating warnings about different undeclared vars. The issue is not deterministic: the vars are usually different, and sometimes it even works fine, without warnings.
By running with :verbose true I found that the warnings always occur after a line about Reading analysis cache for file: <file name for a namespace with macros> (in the case of reagent, that's the reagent.interop namespace). For example:
[snip] Compiling /home/nicolas/projects/reagent/src/reagent/dom.cljs Reading analysis cache for file:/home/nicolas/projects/reagent/src/reagent/interop.cljs WARNING: Use of undeclared Var reagent.dom/load-error at line 11 /home/nicolas/projects/reagent/src/reagent/dom.cljs [snip]
After a lot of debugging I arrived at the following technical explanation and fix proposal (taken from the commit message in the attached patch):
parse-ns allows for running the parse in two ways: updating the global compiler env or not updating it. This is controlled via the :restore option. To implement this, it isolates the parsing process by binding the compiler env to a snapshot of the compiler env when it starts. When the parsing is done, it checks the :restore option and if it's false it merges back parts of the resulting compiler env (the ::namespaces and ::constants keys) into the global compiler env.
The problem is that during the parsing, other compiler processes could have legitimately updated those same keys in the global compiler env, so this merge can occasionally result in compiler data loss.
The race condition can be avoided by using a different approach: only when :restore is true the compiler runs isolated from the global compiler env, by taking a snapshot of the env at the beginning. When :restore is false, no snapshot is taken: env/compiler binding is not modified, so the global compiler env is mutated in-place
I couldn't create a minimal repro, but I created a branch of the reagent project with parallel-build enabled. The branch can be found in https://github.com/nberger/reagent/tree/parallel-build. To run the build from a clean slate, run lein do clean, cljsbuild once client from the commmand line.
Versions affected: 1.8.40 and older (couldn't find 1.8.40 from the "Affects version/s" select)
Major
I found this issue by trying to compile reagent with
parallel-build. Once parallel-build is enabled the compilation fails most of the times, creating warnings about different undeclared vars. The issue is not deterministic: the vars are usually different, and sometimes it even works fine, without warnings.By running with :verbose true I found that the warnings always occur after a line about
Reading analysis cache for file: <file name for a namespace with macros>(in the case of reagent, that's thereagent.interopnamespace). For example:After a lot of debugging I arrived at the following technical explanation and fix proposal (taken from the commit message in the attached patch):
I couldn't create a minimal repro, but I created a branch of the reagent project with parallel-build enabled. The branch can be found in https://github.com/nberger/reagent/tree/parallel-build. To run the build from a clean slate, run
lein do clean, cljsbuild once clientfrom the commmand line.Versions affected: 1.8.40 and older (couldn't find 1.8.40 from the "Affects version/s" select)