lock rustup across concurrent toolchain installs - #631
Conversation
|
OK, going to merge this without a review as it is causing so much pain. We can fix forward any issues. |
| let mut do_build = needs_build(); | ||
| if do_build { | ||
| _file_lock = Some(FileLock::lock(&install_dir.join("lockfile"))?); | ||
| // check again after acquiring lock, another process could have built it in the meantime | ||
| do_build = needs_build(); | ||
| } | ||
| (dest_dylib_path, do_build || self.rebuild_codegen) | ||
| }; | ||
|
|
||
| if do_build { | ||
| Self::write_source_files(&source, &install_dir).context("writing source files")?; | ||
| } else { | ||
| log::info!( | ||
| "cargo-gpu artifacts found in '{}', skipping install", | ||
| install_dir.display() | ||
| ); | ||
| } | ||
|
|
||
| // TODO cache toolchain channel in a file? | ||
| log::debug!("resolving toolchain version to use"); | ||
| let dummy_metadata = CrateMetadata::query(install_dir.clone()) | ||
| .context("resolving toolchain version: get `rustc_codegen_spirv_dummy` metadata")?; | ||
| let rustc_codegen_spirv = dummy_metadata.find_package("rustc_codegen_spirv").context( | ||
| "resolving toolchain version: expected a dependency on `rustc_codegen_spirv`", | ||
| )?; | ||
| let toolchain_channel = | ||
| get_channel_from_rustc_codegen_spirv_build_script(rustc_codegen_spirv).context( | ||
| "resolving toolchain version: read toolchain from `rustc_codegen_spirv`'s build.rs", | ||
| )?; | ||
| log::info!("selected toolchain channel `{toolchain_channel:?}`"); | ||
|
|
||
| log::debug!("ensure_toolchain_and_components_exist"); | ||
| crate::install_toolchain::ensure_toolchain_and_components_exist( | ||
| &toolchain_channel, | ||
| self.auto_install_rust_toolchain, | ||
| ) | ||
| .context("ensuring toolchain and components exist")?; |
There was a problem hiding this comment.
I don't see how this PR is improving the situation. Top of this comment range is acquiring the file lock, and afterwards we call ensure_toolchain_and_components_exist, within which this PR adds another lock.
@nazar-pc 's CI only uses a single rust-gpu version, so it can't be two rust-gpu installs with separate locks running rustup in parallel either.
There was a problem hiding this comment.
I do use a single version, but I have multiple target directories used concurrently, which is most likely how I triggered it. I have not had a single failure since this PR was merged, while previously I was hitting it all the time.
There was a problem hiding this comment.
I wonder if rustup races even when just querying about installed components and such
There was a problem hiding this comment.
I believe so, but that is going to change (thankfully) in the future: https://blog.rust-lang.org/inside-rust/2026/07/03/rustup-update-1.30/
3 participants
Fixes #629