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.

Sign up for GitHub

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

Jump to bottom

shutil.rmtree(..., ignore_errors=True) doesn't ignore errors from os.close() #79513

Closed
rabraham mannequin opened this issue Nov 27, 2018 · 6 comments
Closed

shutil.rmtree(..., ignore_errors=True) doesn't ignore errors from os.close() #79513

rabraham mannequin opened this issue Nov 27, 2018 · 6 comments
Labels
3.11 only security fixes 3.12 bugs and security fixes 3.13 new features, bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@rabraham
Copy link
Mannequin

rabraham mannequin commented Nov 27, 2018

BPO 35332
Nosy @serhiy-storchaka, @ZackerySpytz, @akulakov
PRs
  • bpo-35332: shutil.rmtree(ignore_errors=True) doesn't handle os.close() #23766
    		•

    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 2018-11-27.19:51:53.891>
labels = ['3.7', '3.8', 'type-bug', 'library']
title = "shutil.rmtree(..., ignore_errors=True) doesn't ignore errors from os.close()"
updated_at = <Date 2021-07-16.05:13:34.549>
user = 'https://bugs.python.org/rabraham'

    bugs.python.org fields:

    activity = <Date 2021-07-16.05:13:34.549>
actor = 'andrei.avk'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2018-11-27.19:51:53.891>
creator = 'rabraham'
dependencies = []
files = []
hgrepos = []
issue_num = 35332
keywords = ['patch']
message_count = 5.0
messages = ['330553', '330856', '383038', '397598', '397599']
nosy_count = 5.0
nosy_names = ['serhiy.storchaka', 'David.Edelsohn', 'ZackerySpytz', 'rabraham', 'andrei.avk']
pr_nums = ['23766']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue35332'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']
    @rabraham
    Copy link
    Mannequin Author

    rabraham mannequin commented Nov 27, 2018

    These lines throw intermittently for me on AIX when removing a directory on NFS (python version 3.6.6-1), even when "ignore_errors=True":

    https://github.com/python/cpython/blob/v3.6.6rc1/Lib/shutil.py#L433
    https://github.com/python/cpython/blob/v3.6.6rc1/Lib/shutil.py#L492

    Should there be try-except blocks around these lines and calls to onerror(...)?

    @rabraham rabraham mannequin added 3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Nov 27, 2018
    @rabraham
    Copy link
    Mannequin Author

    rabraham mannequin commented Dec 1, 2018

    I forgot to mention: the exception raised is an OSError and the errno is 52 (ESTALE on AIX).

    @serhiy-storchaka
    Copy link
    Member

    I am wondering whether we should silence that error in os.close() unconditionally.

    In what circumstances the error is raised? Can it be reproduced on Linux (by monkey-patching os.rmdir)? What can happen in worst case when the error is ignored?

    @akulakov
    Copy link
    Contributor

    I've looked a bit into this and it seems like ESTALE can be caused in two cases:

    https://www.ibm.com/support/pages/apar/IV50544

    When using WPAR setup under AIX, with each WPAR configured as an NFS client, it is possible that asynchronous READDIR operations fail with ESTALE error

    In the first case, the solution given is to run READDIR under a different CID. I've found the definition of CID just 10 minutes ago but now can't find it again :(. Anyway, it seems to be an ID corresponding to the WPAR environment.

    In the 2nd case, it's pretty clear that we shouldn't silence it: if you unmount a virtual partition while running rmtree on it, I'm 99.9% sure it was unintended!

    In the first case, I don't think Python has any specific support for WPAR? Maybe? If not, we shouldn't do anything, if yes, maybe it can be fixed to handle this.

    It sounds like Ronal may have ran into the first case, or something else entirely.

    @akulakov
    Copy link
    Contributor

    May be related: https://bugs.python.org/issue43666

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @serhiy-storchaka serhiy-storchaka mentioned this issue Dec 5, 2023
    Merged
    @serhiy-storchaka
    Copy link
    Member

    There is other, more severe bug in the current code: os.close() is retried after error. Is best case this can cause a "bad file descriptor" error. In worst case it can lead to race condition in multithtread or multiprocessing program.

    @serhiy-storchaka serhiy-storchaka added 3.11 only security fixes 3.12 bugs and security fixes 3.13 new features, bugs and security fixes and removed 3.8 only security fixes 3.7 (EOL) end of life labels Dec 5, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.11 only security fixes 3.12 bugs and security fixes 3.13 new features, bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @akulakov @serhiy-storchaka