classification
Title: Race condition in os.makedirs
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: duplicate
Dependencies: Superseder: os.makedirs(): Add a keyword argument to suppress "File exists" exception
View: 9299
Assigned To: Nosy List: draghuram, ggenellina, gvanrossum, ijmorlan, terry.reedy, zooko
Priority: low Keywords: easy

Created on 2007-12-20 19:49 by ijmorlan, last changed 2010-07-21 02:37 by terry.reedy. This issue is now closed.

Files
File name Uploaded Description Edit
makedirs.py ijmorlan, 2007-12-20 20:04
patch.txt ijmorlan, 2007-12-21 18:19
Messages (12)
msg58919 - (view) Author: Isaac Morland (ijmorlan) Date: 2007-12-20 19:49
There appears to be a race condition in os.makedirs.  Suppose two
processes simultaneously try to create two different directories with a
common non-existent ancestor.  For example process 1 tries to create
"a/b" and process 2 tries to create "a/c".  They both check that "a"
does not exist, then both invoke makedirs on "a".  One of these will
throw OSError (due to the underlying EEXIST system error), and this
exception will be propagated.  Note that this happens even though the
two processes are trying to create two different directories and so one
would not expect either to report a problem with the directory already
existing.
msg58921 - (view) Author: Raghuram Devarakonda (draghuram) Date: 2007-12-20 19:58
I don't think os.makedirs() can do anything here. It should be caller's
responsibility to check for this kind of issues.
msg58922 - (view) Author: Isaac Morland (ijmorlan) Date: 2007-12-20 19:59
The only thing I found in the bug database concerning os.makedirs was
Issue 766910 (http://bugs.python.org/issue766910).  I realized
os.makedirs had a race condition because in my application I want to
create directories but it's perfectly fine if they already exist.  This
is exactly what trace.py in Issue 766910 seems to need.

I started writing my own, which was basically just os.makedirs but
calling my own version of os.mkdir which didn't worry about
already-existing directories, but realized that wouldn't work. 
Eventually I ended up with the routines I've put in the attached
makedirs.py.

I think os.makedirs can be fixed by making what is now its recursive
call instead call my version of makedirs.  I also think both my mkdir
and my makedirs should be present in the standard library as well as the
existing versions.  Possibly this could be done by adding a flag to the
existing versions, defaulted to obtain the current behaviour.
msg58924 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-20 20:03
I think we can fix this as follows: whenever it calls os.mkdir() and an
error is returned, check if that is EISDIR or EEXISTS, and if so, check
that indeed it now exists as a directory, and then ignore the error.

Moreover, I'd like to do this for the ultimate path to be created as
well, so that os.makedirs(<existing directory>) will succeed instead of
failing.  This would make the common usage pattern much simpler.

I think it should still fail if the path exists as a file though.  (Or
as a symlink to a file.)

Patch welcome!

I think this is a feature request and hence should only be fixed in 2.6.
msg58925 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-20 20:06
Can you rephrase this as svn diff output?

Also, mkdir() is a confusing name for the helper -- I'd call it
forgiving_mkdir() or something like that.
msg58926 - (view) Author: Isaac Morland (ijmorlan) Date: 2007-12-20 20:13
Yes, I'm really combining two things here - the race condition, which I
argue is a (minor) bug, and a feature request to be able to "ensure
exists" a directory.

I have not produced a proper Python patch before and I have other things
to do so this will take longer than one might hope, but I would be happy
to create a patch.  Note too that the file I uploaded is from my
project; I will attempt to make the patch be more appropriate for the
standard library than an extract from my project.
msg58950 - (view) Author: Isaac Morland (ijmorlan) Date: 2007-12-21 18:19
Attached is an svn diff against the trunk.  I was looking at os.py from
Python 2.5 not the trunk, and it appears that an attempt at fixing the
race condition has already been put into os.py, but I don't believe it's
correct.

The attached patch renames the existing mkdir to _mkdir, and creates a
new mkdir with an additional "excl" parameter to select
error-if-already-exists or not.  It defaults to the current behaviour. 
Similarly, makedirs gets the same extra parameter which is passed down
to mkdir.

By simply using the new versions as before, one obtains the old
behaviour unchanged except that the race condition is corrected.  By
using excl=False one gets the new behaviour.

I have updated the documentation also but I don't really know what I'm
doing there so my use of the rst format may not be right.
msg58951 - (view) Author: Isaac Morland (ijmorlan) Date: 2007-12-21 18:25
I should add that the new parameter is called "excl" by analogy with the
O_EXCL option to os.open().

Also, I'm not absolutely certain about the test for which exceptions
should be ignored when excl == False:

e.errno == errno.EEXIST and path.isdir (name)

This will not work if errno is set to something other than EEXIST when
mkdir fails due to the directory already existing.  The above works on
my system but I can't be certain that all mkdir implementations report
EEXIST.

It should be safe to drop the errno check altogether, and I'm starting
to think that we should; at present it's really just an optimization to
avoid using .isdir, but only in what should be rather uncommon
circumstances.  I think the smell of "premature optimization" may be
hard to ignore.

So the if statement would be:

if excl or not path.isdir (name):
    raise
msg75766 - (view) Author: Zooko O'Whielacronx (zooko) Date: 2008-11-11 22:59
Here's the version of this that I've been using for almost a decade now:

http://allmydata.org/trac/pyutil/browser/pyutil/pyutil/fileutil.py?rev=127#L241

Actually I used to have a bigger version that could optionally require
certain things of the mode of the directory, but it turned out that I
wasn't going to need it.

def make_dirs(dirname, mode=0777):
    """
    An idempotent version of os.makedirs().  If the dir already exists, do
    nothing and return without raising an exception.  If this call
creates the
    dir, return without raising an exception.  If there is an error that
    prevents creation or if the directory gets deleted after make_dirs()
creates
    it and before make_dirs() checks that it exists, raise an exception.
    """
    tx = None
    try:
        os.makedirs(dirname, mode)
    except OSError, x:
        tx = x

    if not os.path.isdir(dirname):
        if tx:
            raise tx
        raise exceptions.IOError, "unknown error prevented creation of
directory, or deleted the directory immediately after creation: %s" %
dirname # careful not to construct an IOError with a 2-tuple, as that
has a special meaning...
msg110751 - (view) Author: Isaac Morland (ijmorlan) Date: 2010-07-19 13:40
This is again being discussed in Issue 9299.
msg110772 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-07-19 16:11
The precipitating issue for this and #9299 are different: parent race leading to error versus tail existence leading to error. However, both patches address both issues.

See #9299 for my comparison of this patch and that.

I am consolidating nosy lists there. Perhaps most/all further discussion should be directed there.
msg110992 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-07-21 02:37
Isaac, thank you for the report and patch. With more attention, this might have been tweaked and applied a couple of years ago. We are trying to get better at timely responses.
History
Date User Action Args
2010-07-21 02:37:05terry.reedysetmessages: + msg110992
2010-07-21 02:13:26terry.reedysetstatus: open -> closed
resolution: duplicate
superseder: os.makedirs(): Add a keyword argument to suppress "File exists" exception
2010-07-19 16:11:55terry.reedysetversions: + Python 3.2, - Python 2.6
nosy: + terry.reedy

messages: + msg110772

type: behavior -> enhancement
stage: patch review
2010-07-19 13:40:16ijmorlansetmessages: + msg110751
2008-11-11 22:59:35zookosetnosy: + zooko
messages: + msg75766
2008-01-20 20:34:54ggenellinasetnosy: + ggenellina
2008-01-12 04:23:52christian.heimessetkeywords: + easy
components: + Library (Lib)
versions: + Python 2.6, - Python 2.5
2007-12-21 18:25:14ijmorlansetmessages: + msg58951
2007-12-21 18:19:57ijmorlansetfiles: + patch.txt
messages: + msg58950
2007-12-20 20:13:37ijmorlansetmessages: + msg58926
2007-12-20 20:06:42gvanrossumsetmessages: + msg58925
2007-12-20 20:04:26ijmorlansetfiles: + makedirs.py
2007-12-20 20:03:33gvanrossumsetpriority: low
nosy: + gvanrossum
messages: + msg58924
2007-12-20 19:59:57ijmorlansetmessages: + msg58922
2007-12-20 19:58:39draghuramsetnosy: + draghuram
messages: + msg58921
2007-12-20 19:49:31ijmorlancreate