-
Notifications
You must be signed in to change notification settings - Fork 955
Can't update to stable 1.35 (Windows + McAffee/Symantec) #1870
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
Comments
I have the same issue. The renaming seems to fail because the file separator is incorrect ('/' instead of ''). |
Hi, so all the errors about renaming from 'bk' are rollback errors - they are victims, not causes. I don't see in your output the file handle closing tracking we added for Defender; possibly McAfee quarantines with different logic. We have a join() there, so we can be sure that the CloseHandle calls have all been submitted to the OS and completed. That means that the unpacking process completed. We changed from making a whole new copy of the docs at the end of the unpack to moving the docs at the end of the unpack process. So here is hypothesis one: we go to move() the file, the file is open in McAfee doing its scan, and as such the rename, which involves opening the file on Windows, cannot take place. I think its worth noting that an on-demand scan of a full disk could race with a regular unpack and rename anyway to cause the unpack-then-rename operation to fail: we don't do anything to prevent other processes operating in the temporary directory, and so adding in some resiliency in these code paths, due to Windows file system semantics, makes a lot of sense to me, as in this model this is just a long-tail failure mode, which our increased performance has uncovered. @Geobert if this is reproducible, could you please get a trace, e.g. with ProcessMonitor, or WPA or some similar tool, and confirm what is going on? Don't filter for rustup.exe - filter for all file activity in your If it is not reproducible, then we can prepare a patch regardless and see if this reoccurs with some cautious settings. If, after gathering traces etc, you still cannot update, you could try rustup toolchain uninstall stable; rustup toolchain install stable - but since the problem appears to be a race condition I don't expect better results. Other things that are more likely to work:
@neelakantankkno, thats not it. See #1647 |
rustup 1.18.0 did the move rather than copy thing, but was much slower at extraction - about 4x slower. So this reinforces the race condition hypothesis. @neelakantankk do you also have McAfee ? |
No, there's only Symantec Endpoint Protection running on this. |
Workaround: Download https://static.rust-lang.org/rustup/archive/1.18.0/x86_64-pc-windows-msvc/rustup-init.exe This will run the previous release that does not have the major performance improvements. |
@neelakantankk what OS ? |
Windows 10. Toolschain's stable-x86_64-pc-windows-gnu |
Thank you for your answer! It's reproductible, I have generated the traces, what am I looking for? |
@Geobert Some process must have a handle open on C:\Users\Geobomatic.rustup\toolchains\stable-x86_64-pc-windows-msvc\share/doc/rust/html at the time the Access Denied error is returned to rustup. or, some file within that path similarly, and then the error won't occur on the CreateFile call to open for the rename, but on the ContentDisposition setting, if I remember correctly. - its late here, but you get the idea of what we're looking for I hope. If that process is rustup itself, its a bug in the new threading code - unlikely but possible I guess? Maybe some thing with mcafee interaction - we'll have to dig. If that process is not rustup, lets start with what it is and try to either confirm or reject the hypothesis that its mcafee interfering with our IO. |
McAffee is definitively guilty. Worth noticing that when recapturing traces (Process Monitor crashed) the update went through. I had to update my i686 toolchain to reproduce again (I thought it was systematic at first as I got it 3/3 and then the 4th passes) EDIT: Note that I've hidden SearchProtocolHost.exe in the traces |
Some virus scanners take out handles on files we are going to rename, causing access errors and preventing the rename. Typically this is short lived. Retry rather than erroring. No feedback given at the moment, and depends on an unreleased retry, so WIP...
I've put a release build of rustup with a possible fix for this at https://s3-ap-southeast-2.amazonaws.com/rbtcollins-experiments/rustup/rustup.exe It should have md5 hash 6fb8c3b34904b26f8e2771bc18185435 If anyone is still experiencing this, please try downloading that build and running it (e.g. .\rustup.exe toolchain update stable). It has a retry and backoff mechanism added around the failing rename. We cannot merge that until jimmycuadra/retry#20 goes through review/fixup/merge/release, so this will be a workaround for a bit. |
Thanks @Geobert that definitely confirms the hypothesis and I hope my patch will work around it acceptably; if you have time to do some uninstall +_ reinstalls of nightly using it that would be great - that should be enough to trigger (or not) the race. |
I'll test your patch monday first thing as it's on my corporate machine :) Thanks for your work! |
Some virus scanners take out handles on files we are going to rename, causing access errors and preventing the rename. Typically this is short lived. Retry rather than erroring. No feedback given at the moment, and it will be a much larger patch to do so - as this is a regression fix I'd like to get it in and released and follow up with something thread all the UI channels down to this layer later (e.g. an implicit context mechanism of some sort perhaps?)
Fix #1870: Race condition with some virus scanners
The candidate fix is merged to master but we still need testing - please do confirm that my test binary fixes the problem. Thank you! |
I didn't mean to close this so I'll reopen it until we get an Okay from the OP. |
Hi all, I tried twice and it passes twice! Looks good to me! Well done! |
When we spin retrying a rename, show a UI notification. Rather raw but much better than silently doing nothing for an extended period.
When we spin retrying a rename, show a UI notification. Rather raw but much better than silently doing nothing for an extended period.
When we spin retrying a rename, show a UI notification. Rather raw but much better than silently doing nothing for an extended period.
When we spin retrying a rename, show a UI notification. Rather raw but much better than silently doing nothing for an extended period.
Is this supposed to be live? I encounter the issue yesterday while upgrading to 1.37, and then I tried the experimental build above (I kept it) to succeed the update. |
I have the same problem. Corporate laptop with McAfee that can not be turn off. I need this release in order to start learning rust. Please hurry :-) |
Same problem. McAfee can't be disabled on my work laptop. It's a big pain when running rustup-update every time. |
Same here. Is there some W/A to try, until there is a fix? Here's my output:
|
The latest release should be a bit better, but if McAfee is really being a pain, there's not a lot we can do short-term. |
try the build from this comment above: #1870 (comment) |
@Geobert and @rbtcollins - this build doesn't seem to work:
|
@rbtcollins You're dirty patch version worked on my corporate box. Thanks! |
Problem
rustup update stable
fails on Windows 7 laptop with this kind of errors:Steps
rustup update stable
Notes
This laptop is crap (corporate machine) and has McAffee anti virus + disk encryption installed.
With previous rustup, it was slow but it worked.
Output of
rustup --version
:rustup 1.18.3 (435397f 2019-05-22)
Output of
rustup show
:Default host: x86_64-pc-windows-msvc
installed toolchains
stable-i686-pc-windows-msvc
stable-x86_64-pc-windows-msvc (default)
nightly-x86_64-pc-windows-msvc
installed targets for active toolchain
i686-pc-windows-msvc
x86_64-pc-windows-msvc
active toolchain
stable-x86_64-pc-windows-msvc (default)
rustc 1.34.2 (6c2484dc3 2019-05-13)
The text was updated successfully, but these errors were encountered: