Skip to content
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

rmtree() failing sporadically on Windows #4910

Closed
jurko-gospodnetic opened this issue Dec 7, 2017 · 1 comment · Fixed by #6976
Closed

rmtree() failing sporadically on Windows #4910

jurko-gospodnetic opened this issue Dec 7, 2017 · 1 comment · Fixed by #6976
Labels
auto-locked Outdated issues that have been locked by automation good first issue A good item for first time contributors to work on kind: crash For situations where pip crashes OS: windows Windows specific state: awaiting PR Feature discussed, PR is needed type: bug A confirmed bug or unintended behavior

Comments

@jurko-gospodnetic
Copy link

jurko-gospodnetic commented Dec 7, 2017

  • Pip version: 9.0.1 as well as current development version (ce674d2), but likely older versions as well
  • Python version: irrelevant, tested with python 2.7.13 & 3.6.1
  • Operating system: Windows 10

Description

On Windows I've often encountered 'false failure reports' when removing a folder. Even when running for instance rd /s /q <folder> on the command-line. It reports an error like the folder not being empty or an access violation error, but actually deletes the folder or at least parts of it, and just repeating the command a few times works correctly.

The underlying cause for the OS operation failures is likely either some OS or external application folder scanning logic keeping live handles referring to the files or folders being removed, and race conditions between those file handles' life times and the file/folder removal process. I've seen the issue on Windows 10 machines with no anti-virus software installed other than what the OS does internally.

This also affects pip installations where it fails to remove some of its internal folders for the same reason. In those cases it typically retries removing the folder several times but then it often happens that one of those removals fails hard because the folder was actually already removed after the previously reported failure. The most often encountered use-case has been a failure in upgrading pip itself as its installation has quite a deep folder structure, and then the final effect of the error is that the target Python environment is left without any valid pip installation and you need to restore it to some version (e.g. using python -m ensurepip) in order to try the upgrade again.

What has been found thus far

All tracebacks I encontered for such cases show failures on various operations in the rmtree_errorhandler() error handler for the rmtree() utility function, e.g. on the os.stat() or os.chmod() calls, or on one of the operations coming from inside the standard library shutil.rmtree() operation.

Suggested solution

And what can fix the problem is making the rmtree_errorhandler() error handler consider the file being already removed as a successful error resolution instead of reporting that as an error.

The only externally visible change then is that calling rmtree() on a non-existing folder will now be treated as success. but that does not seem like a bad change to begin with as the whole does the thing I'm trying to remove exist issue is naturally riddled with race conditions and can not be answered perfectly anyway.

@chrahunt
Copy link
Member

chrahunt commented Sep 1, 2019

This should be straightforward. The specific change would be in src.pip._internal.utils.misc around here. If on error the provided path does not exist then we should just return and not re-raise the exception. The check should be before os.stat in that function.

For tests we'd need at least one showing the new behavior - that deleting a non-existent directory doesn't fail. Another test could be of rmtree_errorhandler directly and should check that it doesn't raise any exception on being passed a non-existent path.

@chrahunt chrahunt added good first issue A good item for first time contributors to work on state: awaiting PR Feature discussed, PR is needed labels Sep 1, 2019
atugushev added a commit to atugushev/pip that referenced this issue Sep 4, 2019
atugushev added a commit to atugushev/pip that referenced this issue Sep 4, 2019
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Oct 18, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation good first issue A good item for first time contributors to work on kind: crash For situations where pip crashes OS: windows Windows specific state: awaiting PR Feature discussed, PR is needed type: bug A confirmed bug or unintended behavior
Projects
None yet
3 participants