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

shutil.rmtree should not fail with FileNotFoundError (race condition) #73885

Closed
dkg mannequin opened this issue Mar 2, 2017 · 11 comments
Closed

shutil.rmtree should not fail with FileNotFoundError (race condition) #73885

dkg mannequin opened this issue Mar 2, 2017 · 11 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@dkg
Copy link
Mannequin

dkg mannequin commented Mar 2, 2017

BPO 29699
Nosy @giampaolo, @serhiy-storchaka, @dkg, @jessefarnham, @websurfer5, @iritkatriel, @coroa
PRs
  • bpo-29699: shutil.rmtree should not fail with FileNotFoundError (race condition) #13580
  • Files
  • breaker.c: C program to tickle the shutil.rmtree race condition
  • demo.py
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2017-03-02.19:12:23.996>
    labels = ['type-bug', 'library']
    title = 'shutil.rmtree should not fail with FileNotFoundError (race condition)'
    updated_at = <Date 2021-12-14.21:57:38.726>
    user = 'https://github.com/dkg'

    bugs.python.org fields:

    activity = <Date 2021-12-14.21:57:38.726>
    actor = 'coroa'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2017-03-02.19:12:23.996>
    creator = 'dkg'
    dependencies = []
    files = ['46687', '46688']
    hgrepos = []
    issue_num = 29699
    keywords = ['patch']
    message_count = 10.0
    messages = ['288822', '288823', '319374', '321458', '343530', '396167', '396177', '396185', '396186', '408570']
    nosy_count = 8.0
    nosy_names = ['giampaolo.rodola', 'serhiy.storchaka', 'dkg', 'jesse.farnham', 'Jeffrey.Kintscher', 'iritkatriel', 'noamda', 'coroa']
    pr_nums = ['13580']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue29699'
    versions = []

    @dkg
    Copy link
    Mannequin Author

    dkg mannequin commented Mar 2, 2017

    There is a race condition in shutil.rmtree, where if a file gets removed between when rmtree plans to remove it and when it gets around to removing it, a FileNotFound exception gets raised.

    The expected semantics of rmtree imply that if the filesystem tree is removed, then the command has succeeded, so it doesn't make sense for rmtree to raise a FileNotFound error if someone else happened to have deleted the file before rmtree gets to it.

    I'm attaching a C program (for GNU/Linux) which uses inotify to remove the other file in a directory when either file is removed. This triggers the rmtree failure.

    This behavior has caused a number of workarounds in external projects, like:

    https://bitbucket.org/vinay.sajip/python-gnupg/commits/492fd45ca073a90aac434320fb0c8fe8d01f782b
    https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gpgme.git;a=commitdiff;h=de8494b16bc50c60a8438f2cae1f8c88e8949f7a

    It would be better for shutil.rmtree to ignore this particular exception (FileNotFoundError).

    Another option for users is to set ignore_errors=True, but this ends up ignoring *all* errors, which doesn't seem like the right decision.

    Finally, of course, a user could specify some sort of onerror function that explictly ignores FileNotFoundError, but this seems pretty complicated for the common pattern.

    It's possible that shutil.rmtree() wants to raise FileNotFoundError if the actual argument passed by the user does not itself exist, but it really doesn't make sense to raise that error for any of the elements further down in the tree.

    @dkg dkg mannequin added type-crash A hard crash of the interpreter, possibly with a core dump stdlib Python modules in the Lib dir labels Mar 2, 2017
    @dkg
    Copy link
    Mannequin Author

    dkg mannequin commented Mar 2, 2017

    and here is python demonstration script that will build breaker.c and then use it to cause the error to be raised from shutils.rmtree.

    the output of demo.py looks like this:

    make: 'breaker' is up to date.
    Traceback (most recent call last):
      File "./demo.py", line 14, in <module>
        shutil.rmtree('xx')
      File "/usr/lib/python3.5/shutil.py", line 480, in rmtree
        _rmtree_safe_fd(fd, path, onerror)
      File "/usr/lib/python3.5/shutil.py", line 438, in _rmtree_safe_fd
        onerror(os.unlink, fullname, sys.exc_info())
      File "/usr/lib/python3.5/shutil.py", line 436, in _rmtree_safe_fd
        os.unlink(name, dir_fd=topfd)
    FileNotFoundError: [Errno 2] No such file or directory: 'b'

    @giampaolo
    Copy link
    Contributor

    +1. It looks reasonable to ignore FileNotFoundError on os.rmdir(), os.unlink() and also os.open() and os.scandir().

    @serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error and removed type-crash A hard crash of the interpreter, possibly with a core dump labels Jul 11, 2018
    @serhiy-storchaka
    Copy link
    Member

    Shouldn't this be considered as a new feature? There are no guaranties that shutil.rmtree() should work if files or directories are concurrently removed, or created, or made read-only in other threads or processes.

    @websurfer5
    Copy link
    Mannequin

    websurfer5 mannequin commented May 26, 2019

    I created pull request bpo-29699 to fix this issue. It adds an additional exception handler to ignore FileNotFoundError for most of the try blocks that already handle OSError.

    I decided not to add it to the initial os.open() call. This should provide the same semantics as the "rm -r" shell command. It will fail with FileNotFoundError when foo is missing, which is the same behavior as "rm -r foo" returning "rm: foo: No such file or directory" when foo is missing. Similarly, "rm -rf foo" always succeeds and is equivalent to setting "ignore_errors=true" in the shutil.rmtree() call.

    @noamda
    Copy link
    Mannequin

    noamda mannequin commented Jun 20, 2021

    Is this still alive? If decided to decline PR why is this still open?

    @iritkatriel
    Copy link
    Member

    seel also bpo-37260

    @noamda
    Copy link
    Mannequin

    noamda mannequin commented Jun 20, 2021

    Hi Irit,

    Sorry, I'm still not following,

    The other issue you stated, states that PR is ready(06-2019), and this was in 2019, is this going to be fixed? or declined (and both should be closed)?

    What is the current updated status?

    @iritkatriel
    Copy link
    Member

    PR13580 was not declined, the reviewer requested unit tests. If you want to advance this, perhaps you can help fill that in.

    The other PR looks like a duplicate that should be closed.

    @coroa
    Copy link
    Mannequin

    coroa mannequin commented Dec 14, 2021

    Just realised that the "race condition" is triggered consistently on Mac OSX machines working on non-Mac filesystems, where they store extended attributes like last access time in meta files with a ._ prefix. These files are listed by os.scandir after their original files and are removed, as soon as the original file is unlinked.

    Ie. a test.txt, ._test.txt pair will always raise FileNotFoundError for ._test.txt since the file system driver itself already removed it when the unlink on test.txt was executed.

    Refer also to https://stackoverflow.com/a/70355470/2873952.

    Current status seems to be:

    1. https://bugs.python.org/issue29699 and https://bugs.python.org/issue37260 are duplicates.
    2. bpo-29699: shutil.rmtree should not fail with FileNotFoundError (race condition) #13580 linked from here has been closed due to lack of unit tests (but with the possibility to re-open after providing those)
    3. gh-81441: shutil.rmtree() FileNotFoundError race condition #14064 is still open and seems to be mostly the same patch, does have unit tests and awaiting a second review.

    What is the next step?

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    dongjoon-hyun added a commit to apache/spark that referenced this issue Jun 27, 2022
    …rmtree` on MacOS
    
    ### What changes were proposed in this pull request?
    
    This PR aims to make `run-tests.py` robust by avoiding `rmtree` on MacOS.
    
    ### Why are the changes needed?
    
    There exists a race condition in Python and it causes flakiness in MacOS
    - https://bugs.python.org/issue29699
    - python/cpython#73885
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    After passing CIs, this should be tested on MacOS.
    
    Closes #37010 from dongjoon-hyun/SPARK-39621.
    
    Authored-by: Dongjoon Hyun <dongjoon@apache.org>
    Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
    (cherry picked from commit 432945d)
    Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
    dongjoon-hyun added a commit to apache/spark that referenced this issue Jun 27, 2022
    …rmtree` on MacOS
    
    ### What changes were proposed in this pull request?
    
    This PR aims to make `run-tests.py` robust by avoiding `rmtree` on MacOS.
    
    ### Why are the changes needed?
    
    There exists a race condition in Python and it causes flakiness in MacOS
    - https://bugs.python.org/issue29699
    - python/cpython#73885
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    After passing CIs, this should be tested on MacOS.
    
    Closes #37010 from dongjoon-hyun/SPARK-39621.
    
    Authored-by: Dongjoon Hyun <dongjoon@apache.org>
    Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
    dongjoon-hyun added a commit to apache/spark that referenced this issue Jun 27, 2022
    …rmtree` on MacOS
    
    This PR aims to make `run-tests.py` robust by avoiding `rmtree` on MacOS.
    
    There exists a race condition in Python and it causes flakiness in MacOS
    - https://bugs.python.org/issue29699
    - python/cpython#73885
    
    No.
    
    After passing CIs, this should be tested on MacOS.
    
    Closes #37010 from dongjoon-hyun/SPARK-39621.
    
    Authored-by: Dongjoon Hyun <dongjoon@apache.org>
    Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
    (cherry picked from commit 432945d)
    Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
    dongjoon-hyun added a commit to apache/spark that referenced this issue Jun 27, 2022
    …rmtree` on MacOS
    
    This PR aims to make `run-tests.py` robust by avoiding `rmtree` on MacOS.
    
    There exists a race condition in Python and it causes flakiness in MacOS
    - https://bugs.python.org/issue29699
    - python/cpython#73885
    
    No.
    
    After passing CIs, this should be tested on MacOS.
    
    Closes #37010 from dongjoon-hyun/SPARK-39621.
    
    Authored-by: Dongjoon Hyun <dongjoon@apache.org>
    Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
    (cherry picked from commit 432945d)
    Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
    sunchao pushed a commit to sunchao/spark that referenced this issue Jun 2, 2023
    …rmtree` on MacOS
    
    This PR aims to make `run-tests.py` robust by avoiding `rmtree` on MacOS.
    
    There exists a race condition in Python and it causes flakiness in MacOS
    - https://bugs.python.org/issue29699
    - python/cpython#73885
    
    No.
    
    After passing CIs, this should be tested on MacOS.
    
    Closes apache#37010 from dongjoon-hyun/SPARK-39621.
    
    Authored-by: Dongjoon Hyun <dongjoon@apache.org>
    Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
    (cherry picked from commit 432945d)
    Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
    @serhiy-storchaka
    Copy link
    Member

    Done in the duplicate issue #81441.

    @serhiy-storchaka serhiy-storchaka closed this as not planned Won't fix, can't repro, duplicate, stale Dec 5, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants