-
Notifications
You must be signed in to change notification settings - Fork 895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rustup gets confused when it can't create a temp directory #2546
Comments
rustup shouldn't be installed at .local/lib/cargo anyway - have you set some environment variables perhaps? what does 'rustup show' output? |
This might be a case of #2417 but I doubt it, I think something different is going on here. |
I have no idea what we do when we get ENOSPC trying to make a tempdir, nor how we ended up with a broken set of metadata. How to attempt to reproduce this will be "interesting" |
@rbtcollins here's a smaller reproduction, with only CARGO_HOME set but not RUSTUP_HOME.
Strangely,
So I consider this 'works for me'. But it would still be nice to fix the inconsistent toolchain state. |
@kinnison here's that strace:
|
So the syscall sequence which fills me with the heebie jeebies in particular is:
|
I smell transactional rollback leaping blindly into the unknown... which if correct makes it a case of #2417 indeed, at least for the corrupt components list |
Hmm no the rollback starts later; the corruption here starts when we have a failure signalled via close but not detected in our code. I suspect we need some file::flush() calls to fix this - note that this doesn't imply fsync on rust. So from a performance perspective this is a no-op: all we are doing is just ensuring we have flushed the intermediate buffers that are accumulating the content out to the OS before the drop-is-close semantics kick in. For most-but-not-all file systems that will be enough to get this error earlier. Except NFS, where we can't flush the OS buffer short of an fsync, and that - well we may have to for some select files, but as it won't actually fix the out of space issue, we'll need to think more on this anyway: the goal here should be to not corrupt things, rather than 'succeeding'. |
Though the fact that copy_file_range succeeded and close(4) failed leaves me in some doubt about this, and possibly we are just flat out of luck and really really we need |
We can access close() ourselves by getting the fd via https://doc.rust-lang.org/std/os/unix/io/trait.AsRawFd.html + https://docs.rs/libc/0.2.65/libc/fn.close.html |
@rbtcollins if you want to push a change using |
There's some chance that sync_all would force the error to be exposed, but it's also a crippling performance impact to make, so other than exploring the space to understand things, I really don't want to do that. And flush() is a no-op on the file, so for our case here I think it will have no effect. |
To keep all the info in one place, there is https://crates.io/crates/close-file but it's not quite right in the head. Alex also wrote https://gist.github.com/alexcrichton/0489d44efb7b3a6aa96fae044dd1be23 which is a bit better though still not fully correct or ideal |
Problem
Notes
Output of
rustup --version
: rustup 1.20.2 (13979c9 2019-10-16)Output of
rustup show
:I'd test with a newer version, but apparently
rustup self update
is also confused:The text was updated successfully, but these errors were encountered: