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

Prevent race condition in os.makedirs #42197

Closed
nirs mannequin opened this issue Jul 17, 2005 · 11 comments
Closed

Prevent race condition in os.makedirs #42197

nirs mannequin opened this issue Jul 17, 2005 · 11 comments

Comments

@nirs
Copy link
Mannequin

nirs mannequin commented Jul 17, 2005

BPO 1239890
Nosy @birkenfeld, @birkenfeld, @nirs
Files
  • makedirs.diff: prevent race conditions in os.makedirs, new tests
  • 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 = <Date 2006-12-09.09:24:10.000>
    created_at = <Date 2005-07-17.19:42:09.000>
    labels = []
    title = 'Prevent race condition in os.makedirs'
    updated_at = <Date 2006-12-09.09:24:10.000>
    user = 'https://github.com/nirs'

    bugs.python.org fields:

    activity = <Date 2006-12-09.09:24:10.000>
    actor = 'georg.brandl'
    assignee = 'none'
    closed = True
    closed_date = None
    closer = None
    components = ['None']
    creation = <Date 2005-07-17.19:42:09.000>
    creator = 'nirs'
    dependencies = []
    files = ['6732']
    hgrepos = []
    issue_num = 1239890
    keywords = ['patch']
    message_count = 11.0
    messages = ['48583', '48584', '48585', '48586', '48587', '48588', '48589', '48590', '48591', '48592', '48593']
    nosy_count = 4.0
    nosy_names = ['georg.brandl', 'georg.brandl', 'nirs', 'rbarran']
    pr_nums = []
    priority = 'normal'
    resolution = 'out of date'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue1239890'
    versions = []

    @nirs
    Copy link
    Mannequin Author

    nirs mannequin commented Jul 17, 2005

    makedirs check if a directory exists, and call mkdir to create it. In
    rare cases, the directory might have been created by another
    process or thread in the time passed from the exists() call to the
    mkdir() call.

    To prevent this rare race condition, use try: expect: to create the
    directory, then ignore "File exists" errors, which mean the directory
    was there.

    @nirs nirs mannequin closed this as completed Jul 17, 2005
    @birkenfeld
    Copy link
    Member

    Logged In: YES
    user_id=1188172

    See bug bpo-1223238.

    @nirs
    Copy link
    Mannequin Author

    nirs mannequin commented Jul 17, 2005

    Logged In: YES
    user_id=832344

    I can't find such bug or patch.

    @birkenfeld
    Copy link
    Member

    @nirs
    Copy link
    Mannequin Author

    nirs mannequin commented Jul 17, 2005

    Logged In: YES
    user_id=832344

    Here is another patch, using simpler design, maybe for 2.5?

    The attached patch will try once to create each leaf in the path, ignoring
    existing leafs.

    This should work for most cases. If one run application that can delete its
    own directories, he should use proper locking.

    @rbarran
    Copy link
    Mannequin

    rbarran mannequin commented Dec 2, 2005

    Logged In: YES
    user_id=1207189

    Hi,
    Your patch "makedirs.py" for python 2.5 will fail under
    Windows if a drive letter is specified:

    >>> os.getcwd()
    'C:\\Temp\\makedirs'
    >>> os.listdir('.')
    ['makedirs.py', 'makedirs.pyc', 'test.py']
    >>> makedirs.makedirs('c:/temp/makedirs/a/b')
    >>> os.listdir('.')
    ['makedirs.py', 'makedirs.pyc', 'temp', 'test.py']

    It will create the following path:
    C:\Temp\makedirs\temp\makedirs\a\b

    Also I ran it through the test suite (lib\test\test_os.py)
    and it failed one test:

    ======================================================================
    FAIL: test_makedir (main.MakedirTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "test_os.py", line 327, in test_makedir
        self.failUnlessRaises(OSError, os.makedirs, os.curdir)
    AssertionError: OSError not raised

    I haven't looked into *why* this happens - I'll dig a bit
    deeper into this subject sometime next week.

    HTH,
    Richard

    @nirs
    Copy link
    Mannequin Author

    nirs mannequin commented Dec 2, 2005

    Logged In: YES
    user_id=832344

    The 2.5 code is only a proof of concept, don't use it as is :-)

    All the directories are handled in the same way - ignore OSError for
    existing directories, which is not compatible with the docs, that say it should
    raise an OSError for the last directory. This has to be fixed of course.

    I'll get 2.5 development code and submit a real working patch.

    @nirs
    Copy link
    Mannequin Author

    nirs mannequin commented Dec 5, 2005

    Logged In: YES
    user_id=832344

    I deleted both patches as they are both wrong:

    The patch against 2.4.1 will not raise OSError when trying to create existing
    directories.

    The simpler code for 2.5 will not work because os.path.normpath is not safe
    to use if the path contain symbolic links.

    @nirs
    Copy link
    Mannequin Author

    nirs mannequin commented Dec 5, 2005

    Logged In: YES
    user_id=832344

    Please review this new patch, tested with current 2.5 code.

    I factored duplicate code in makedirs and removedirs into private _splits
    function, kind of the "super" version of os.path.split for the "super" directory
    utilities.

    I also simplified the test code for makedirs, and added more test cases.

    @nirs
    Copy link
    Mannequin Author

    nirs mannequin commented Dec 5, 2005

    Logged In: YES
    user_id=832344

    Sorry, here is the patch.

    @birkenfeld
    Copy link
    Member

    Something similar has been committed as patch bpo-1608267.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    None yet
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant