Repository navigation
fix: lake: fetch plugin and link facets on the fetching thread - #15696
Draft
rbeauchamp wants to merge 1 commit into
Draft
rbeauchamp wants to merge 1 commit into
rbeauchamp wants to merge 1 commit into
Conversation
This PR makes Lake fetch a library's or module's shared library, static library or object file named by key in `plugins`, `dynlibs`, `moreLinkObjs` or `moreLinkLibs`, such as `Plug:shared`, directly rather than in a job continuation when it prepares a module for elaboration or links an executable or a module's shared library. Previously, the modules of a library with `plugins = ["Plug:shared"]` could each start a job for `Plug:shared`, and the duplicate jobs could overwrite each other's outputs and fail the build. `PartialBuildKey.fetchIn` fetches a facet in a `bindM` continuation of its target's job. Continuations run on worker threads alongside other fetches, and the build store looks a key up and stores its job in separate steps, so two continuations could both miss the key and both create its job. The new internal `Target.fetchInBlocking` and `TargetArray.fetchInBlocking` bind the fetch synchronously, so that it runs before they return, with the continuation's cancellation check, failure handling and data kind, when the facet is one of Lake's own library facets `shared`, `static` and `static.export` or module facets `dynlib`, `o`, `c.o` and `bc.o` (with their variants), and the job of its target has finished, as the job of a module, a package or a configured target always has. Otherwise they bind it as `fetchIn` does. They are used only where Lake fetches these keys after waiting for a module's imports: in `Module.recFetchPreSetup`, `recComputeModuleLinkInfo`, `Module.recBuildDynlib`, `setupEditedModule` and `setupExternalModule`. The facet's fetch function now runs in place, and with it every fetch function that it calls in place, and they can block, as `Module.recFetchPreSetup` can since leanprover#13601 when it awaits `needs`. For `Plug:shared`, they include the `presetup` of each of `Plug`'s modules, which awaits `Plug`'s `needs`, and the fetch functions of the custom targets and `extern_lib`s that `Plug` links. The waits before the fetch return at once when the jobs they wait for have finished. So code that fetches a module's `presetup`, `dynlib` or link information from a `(sync := true)` task, which did not block there before, can now block, and `Task.get` panics in such a task. If one of those jobs or fetch functions waits for something that the fetching code does only after the fetch returns, such as resolving a promise, the build now hangs. Other fetches are unchanged. A package's own facets (e.g., from `library_facet`), the facets of an `extern_lib`, and a facet whose target's job is still running, such as a facet of a custom `target`, are still fetched in a continuation and can still start duplicate jobs. So are a library's `needs` (`LeanLib.recBuildExtraDepTargets`) and the `moreLinkObjs` and `moreLinkLibs` of its `static` and `shared` facets, which are fetched in functions with no earlier wait on every path. Naming a default facet by its target alone, e.g., `needs = ["a"]` for `a:exe`, avoids this, since Lake fetches a target's default facet in place. `PartialBuildKey.fetchIn`, `BuildKey.fetch` and `Target.fetchIn` behave as before, as do the continuation fetches of `Module.computeExportInfo`, `Module.computeMetaExportInfo` and `Module.recBuildIRArts`, which find stored keys under Lake's builtin facets. Code in a package that fetches a key not yet stored from its own job continuation, e.g., in `Job.async` or `Job.mapM`, can also still race in the same way.
Contributor
Author
|
Notes for review:
|
3 tasks done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes Lake fetch a library's or module's shared library, static library or object file named by key in
plugins,dynlibs,moreLinkObjsormoreLinkLibs, such asPlug:shared, directly rather than in a job continuation when it prepares a module for elaboration or links an executable or a module's shared library. Previously, the modules of a library withplugins = ["Plug:shared"]could each start a job forPlug:shared, and the duplicate jobs could overwrite each other's outputs and fail the build.PartialBuildKey.fetchInfetches a facet in abindMcontinuation of its target's job. Continuations run on worker threads alongside other fetches, and the build store looks a key up and stores its job in separate steps, so two continuations could both miss the key and both create its job. The new internalTarget.fetchInBlockingandTargetArray.fetchInBlockingbind the fetch synchronously, so that it runs before they return, with the continuation's cancellation check, failure handling and data kind, when the facet is one of Lake's own library facetsshared,staticandstatic.exportor module facetsdynlib,o,c.oandbc.o(with their variants), and the job of its target has finished, as the job of a module, a package or a configured target always has. Otherwise they bind it asfetchIndoes. They are used only where Lake fetches these keys after waiting for a module's imports: inModule.recFetchPreSetup,recComputeModuleLinkInfo,Module.recBuildDynlib,setupEditedModuleandsetupExternalModule.The facet's fetch function now runs in place, and with it every fetch function that it calls in place, and they can block, as
Module.recFetchPreSetupcan since #13601 when it awaitsneeds. ForPlug:shared, they include thepresetupof each ofPlug's modules, which awaitsPlug'sneeds, and the fetch functions of the custom targets andextern_libs thatPluglinks. The waits before the fetch return at once when the jobs they wait for have finished. So code that fetches a module'spresetup,dynlibor link information from a(sync := true)task, which did not block there before, can now block, andTask.getpanics in such a task. If one of those jobs or fetch functions waits for something that the fetching code does only after the fetch returns, such as resolving a promise, the build now hangs.Other fetches are unchanged. A package's own facets (e.g., from
library_facet), the facets of anextern_lib, and a facet whose target's job is still running, such as a facet of a customtarget, are still fetched in a continuation and can still start duplicate jobs. So are a library'sneeds(LeanLib.recBuildExtraDepTargets) and themoreLinkObjsandmoreLinkLibsof itsstaticandsharedfacets, which are fetched in functions with no earlier wait on every path. Naming a default facet by its target alone, e.g.,needs = ["a"]fora:exe, avoids this, since Lake fetches a target's default facet in place.PartialBuildKey.fetchIn,BuildKey.fetchandTarget.fetchInbehave as before, as do the continuation fetches ofModule.computeExportInfo,Module.computeMetaExportInfoandModule.recBuildIRArts, which find stored keys under Lake's builtin facets. Code in a package that fetches a key not yet stored from its own job continuation, e.g., inJob.asyncorJob.mapM, can also still race in the same way.AI assistance: Claude Opus 5.5 in Claude Code contributed to the fix, the tests and this description, and GPT-6.1 Sol (OpenAI) reviewed the change. I have reviewed the changes.
Part of #15435