From d5c259fd4504ceea97aac7bfdba86b94b82ae25c Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Thu, 30 Mar 2017 13:49:30 -0700 Subject: [PATCH 1/5] Fix symlinking race condition --- readthedocs/core/symlink.py | 17 +++++++++-------- readthedocs/core/utils/__init__.py | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/readthedocs/core/symlink.py b/readthedocs/core/symlink.py index 28a720495..583cbf8b4 100644 --- a/readthedocs/core/symlink.py +++ b/readthedocs/core/symlink.py @@ -62,6 +62,7 @@ from django.conf import settings from readthedocs.builds.models import Version from readthedocs.core.utils.extend import SettingsOverrideObject +from readthedocs.core.utils import safe_makedirs from readthedocs.projects import constants from readthedocs.projects.models import Domain from readthedocs.projects.utils import run @@ -100,19 +101,19 @@ class Symlink(object): if os.path.islink(self.project_root) and not self.project.single_version: self._log("Removing single version symlink") os.unlink(self.project_root) - os.makedirs(self.project_root) + safe_makedirs(self.project_root) elif (self.project.single_version and not os.path.islink(self.project_root) and os.path.exists(self.project_root)): shutil.rmtree(self.project_root) elif not os.path.lexists(self.project_root): - os.makedirs(self.project_root) + safe_makedirs(self.project_root) # CNAME root directories if not os.path.lexists(self.CNAME_ROOT): - os.makedirs(self.CNAME_ROOT) + safe_makedirs(self.CNAME_ROOT) if not os.path.lexists(self.PROJECT_CNAME_ROOT): - os.makedirs(self.PROJECT_CNAME_ROOT) + safe_makedirs(self.PROJECT_CNAME_ROOT) def run(self): """ @@ -177,7 +178,7 @@ class Symlink(object): if rels.count(): # Don't creat the `projects/` directory unless subprojects exist. if not os.path.exists(self.subproject_root): - os.makedirs(self.subproject_root) + safe_makedirs(self.subproject_root) for rel in rels: # A mapping of slugs for the subproject URL to the actual built # documentation @@ -194,7 +195,7 @@ class Symlink(object): ) symlink_dir = os.sep.join(symlink.split(os.path.sep)[:-1]) if not os.path.lexists(symlink_dir): - os.makedirs(symlink_dir) + safe_makedirs(symlink_dir) run('ln -nsf %s %s' % (docs_dir, symlink)) # Remove old symlinks @@ -219,7 +220,7 @@ class Symlink(object): if os.path.islink(language_dir): os.unlink(language_dir) if not os.path.lexists(language_dir): - os.makedirs(language_dir) + safe_makedirs(language_dir) for (language, slug) in translations.items(): self._log(u"Symlinking translation: {0}->{1}".format(language, slug)) @@ -271,7 +272,7 @@ class Symlink(object): version_queryset = self.get_version_queryset() if version_queryset.count(): if not os.path.exists(version_dir): - os.makedirs(version_dir) + safe_makedirs(version_dir) for version in version_queryset: self._log(u"Symlinking Version: %s" % version) symlink = os.path.join(version_dir, version.slug) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index 3855c6194..c35240e8e 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -1,3 +1,4 @@ +import errno import getpass import logging import os @@ -153,3 +154,16 @@ def slugify(value, *args, **kwargs): slugify = allow_lazy(slugify, six.text_type, SafeText) + + +def safe_makedirs(directory_name): + """ + Makedirs has an issue where it has a race condition around + checking for a directory and then creating it. + This catches the exception in this case. + """ + + try: + os.makedirs(directory_name) + except OSError: + pass From 0432ee8f0c897a9f60a7d8813275924f9e12487c Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Thu, 30 Mar 2017 22:05:56 -0700 Subject: [PATCH 2/5] Raise on non-existing --- readthedocs/core/utils/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index c35240e8e..05c057792 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -166,4 +166,6 @@ def safe_makedirs(directory_name): try: os.makedirs(directory_name) except OSError: - pass + if e.errno == errno.EEXIST + pass + raise From f681fd26dfe38aa1a0c4cd7777ddce4f8de15b91 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Thu, 30 Mar 2017 22:06:24 -0700 Subject: [PATCH 3/5] CLean up docstring --- readthedocs/core/utils/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index 05c057792..9006c4c69 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -160,7 +160,7 @@ def safe_makedirs(directory_name): """ Makedirs has an issue where it has a race condition around checking for a directory and then creating it. - This catches the exception in this case. + This catches the exception in the case where the dir already exists. """ try: From 4ca931d406666057c46bcbab1e7d5c3c77f7e653 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Thu, 30 Mar 2017 22:09:09 -0700 Subject: [PATCH 4/5] Fix syntax :) --- readthedocs/core/utils/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index 9006c4c69..8a550c92f 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -166,6 +166,6 @@ def safe_makedirs(directory_name): try: os.makedirs(directory_name) except OSError: - if e.errno == errno.EEXIST + if e.errno == errno.EEXIST: pass raise From a395814f2765a904d193a684338133a2544b9c24 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Thu, 30 Mar 2017 22:45:06 -0700 Subject: [PATCH 5/5] Fix exception --- readthedocs/core/utils/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index 8a550c92f..e31bf2ca0 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -165,7 +165,7 @@ def safe_makedirs(directory_name): try: os.makedirs(directory_name) - except OSError: + except OSError as e: if e.errno == errno.EEXIST: pass raise