Use ensuredir() instead of os.makedirs() to fix race conditions

Use the ensuredir() function consistently across Sphinx code to avoid
race conditions e.g. when multiple Sphinx instances attempt to create
the same output directory. The ensuredir() function that was already
present in sphinx.util.osutil correctly catches EEXIST exception that
occurs if the specified directory already exists (i.e. it was created
between the call to os.path.isdir() and os.makedirs() that follows it).

While at it, remove redundant os.path.isdir() calls when they only
guarded the os.makedirs() call, and replace mkdir_p() which had pretty
much the same purpose, except for being prone to race conditions.

I did not modify testing-related code as race conditions mostly affect
real applications and not the test environment.

Fix #4281: Race conditions when creating output directory

CHANGES

@@ -33,6 +33,7 @@ Bugs fixed
 * #4279: Sphinx crashes with pickling error when run with multiple processes and
   remote image
 * #1421: Respect the quiet flag in sphinx-quickstart
+* #4281: Race conditions when creating output directory
 
 Testing
 --------

sphinx/apidoc.py

@@ -26,7 +26,7 @@ from fnmatch import fnmatch
 from sphinx import __display_version__
 from sphinx.quickstart import EXTENSIONS
 from sphinx.util import rst
-from sphinx.util.osutil import FileAvoidWrite, walk
+from sphinx.util.osutil import FileAvoidWrite, ensuredir, walk
 
 if False:
     # For type annotation
@@ -375,9 +375,8 @@ Note: By default this script will not overwrite already created files.""")
     if not path.isdir(rootpath):
         print('%s is not a directory.' % rootpath, file=sys.stderr)
         sys.exit(1)
-    if not path.isdir(opts.destdir):
-        if not opts.dryrun:
-            os.makedirs(opts.destdir)
+    if not opts.dryrun:
+        ensuredir(opts.destdir)
     rootpath = path.abspath(rootpath)
     excludes = normalize_excludes(rootpath, excludes)
     modules = recurse_tree(rootpath, excludes, opts)

sphinx/application.py

@@ -41,7 +41,7 @@ from sphinx.util import import_object
 from sphinx.util import logging
 from sphinx.util import status_iterator, old_status_iterator, display_chunk
 from sphinx.util.tags import Tags
-from sphinx.util.osutil import ENOENT
+from sphinx.util.osutil import ENOENT, ensuredir
 from sphinx.util.console import bold, darkgreen  # type: ignore
 from sphinx.util.docutils import is_html5_writer_available, directive_helper
 from sphinx.util.i18n import find_catalog_source_files
@@ -160,7 +160,7 @@ class Sphinx(object):
 
         if not path.isdir(outdir):
             logger.info('making output directory...')
-            os.makedirs(outdir)
+            ensuredir(outdir)
 
         # read config
         self.tags = Tags(tags)

sphinx/builders/__init__.py

@@ -9,7 +9,6 @@
     :license: BSD, see LICENSE for details.
 """
 
-import os
 from os import path
 import warnings
 
@@ -24,7 +23,7 @@ from docutils import nodes
 from sphinx.deprecation import RemovedInSphinx20Warning
 from sphinx.environment.adapters.asset import ImageAdapter
 from sphinx.util import i18n, path_stabilize, logging, status_iterator
-from sphinx.util.osutil import SEP, relative_uri
+from sphinx.util.osutil import SEP, ensuredir, relative_uri
 from sphinx.util.i18n import find_catalog
 from sphinx.util.console import bold  # type: ignore
 from sphinx.util.parallel import ParallelTasks, SerialTasks, make_chunks, \
@@ -79,8 +78,7 @@ class Builder(object):
         self.confdir = app.confdir
         self.outdir = app.outdir
         self.doctreedir = app.doctreedir
-        if not path.isdir(self.doctreedir):
-            os.makedirs(self.doctreedir)
+        ensuredir(self.doctreedir)
 
         self.app = app              # type: Sphinx
         self.env = None             # type: BuildEnvironment

sphinx/quickstart.py

@@ -35,7 +35,7 @@ from six.moves.urllib.parse import quote as urlquote
 from docutils.utils import column_width
 
 from sphinx import __display_version__, package_dir
-from sphinx.util.osutil import make_filename
+from sphinx.util.osutil import ensuredir, make_filename
 from sphinx.util.console import (  # type: ignore
     purple, bold, red, turquoise, nocolor, color_terminal
 )
@@ -69,13 +69,6 @@ EXTENSIONS = ('autodoc', 'doctest', 'intersphinx', 'todo', 'coverage',
 PROMPT_PREFIX = '> '
 
 
-def mkdir_p(dir):
-    # type: (unicode) -> None
-    if path.isdir(dir):
-        return
-    os.makedirs(dir)
-
-
 # function to get input from terminal -- overridden by the test suite
 def term_input(prompt):
     # type: (unicode) -> unicode
@@ -433,11 +426,11 @@ def generate(d, overwrite=True, silent=False, templatedir=None):
         d[key + '_str'] = d[key].replace('\\', '\\\\').replace("'", "\\'")
 
     if not path.isdir(d['path']):
-        mkdir_p(d['path'])
+        ensuredir(d['path'])
 
     srcdir = d['sep'] and path.join(d['path'], 'source') or d['path']
 
-    mkdir_p(srcdir)
+    ensuredir(srcdir)
     if d['sep']:
         builddir = path.join(d['path'], 'build')
         d['exclude_patterns'] = ''
@@ -448,9 +441,9 @@ def generate(d, overwrite=True, silent=False, templatedir=None):
             'Thumbs.db', '.DS_Store',
         ])
         d['exclude_patterns'] = ', '.join(exclude_patterns)
-    mkdir_p(builddir)
-    mkdir_p(path.join(srcdir, d['dot'] + 'templates'))
-    mkdir_p(path.join(srcdir, d['dot'] + 'static'))
+    ensuredir(builddir)
+    ensuredir(path.join(srcdir, d['dot'] + 'templates'))
+    ensuredir(path.join(srcdir, d['dot'] + 'static'))
 
     def write_file(fpath, content, newline=None):
         # type: (unicode, unicode, unicode) -> None