When handling errors according to errno, catch both IOError and OSError

Different Python versions are not completely consistent about the
error that is raised and its class hierarchy:

Python 3.5.3rc1 (default, Jan  3 2017, 04:40:57)
>>> try: open('/foo')
... except Exception as e: print(e.__class__.__mro__)
(<class 'FileNotFoundError'>, <class 'OSError'>, <class 'Exception'>, <class 'BaseException'>, <class 'object'>)

Python 2.7.13 (default, Dec 18 2016, 20:19:42)
>>> try: open('/foo')
... except Exception as e: print e.__class__.__mro
(<type 'exceptions.IOError'>, <type 'exceptions.EnvironmentError'>, <type 'exceptions.StandardError'>, <type 'exceptions.Exception'>, <type 'exceptions.BaseException'>, <type 'object'>)

This can lead to a race condition during cache cleaning, where two
processes both try to delete the same file, and the one that loses
the race fails.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Iain Lane <laney@ubuntu.com>
Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=772173

2 files changed, 10 insertions, 16 deletions

diff --git a/giscanner/cachestore.py b/giscanner/cachestore.py
index 007d992b..58b3193c 100644
--- a/giscanner/cachestore.py
+++ b/giscanner/cachestore.py
@@ -76,7 +76,7 @@ class CacheStore(object):
         try:
             with open(version, 'r') as version_file:
                 cache_hash = version_file.read()
-        except IOError as e:
+        except (IOError, OSError) as e:
             # File does not exist
             if e.errno == errno.ENOENT:
                 cache_hash = 0
@@ -96,7 +96,7 @@ class CacheStore(object):
             # On Unix, this would just be os.rename() but Windows
             # doesn't allow that.
             shutil.move(tmp_filename, version)
-        except IOError as e:
+        except (IOError, OSError) as e:
             # Permission denied
             if e.errno == errno.EACCES:
                 return
@@ -121,15 +121,9 @@ class CacheStore(object):
     def _remove_filename(self, filename):
         try:
             os.unlink(filename)
-        except IOError as e:
-            # Permission denied
-            if e.errno == errno.EACCES:
-                return
-            else:
-                raise
-        except OSError as e:
-            # File does not exist
-            if e.errno == errno.ENOENT:
+        except (IOError, OSError) as e:
+            # Ignore "permission denied", "file does not exist"
+            if e.errno in (errno.EACCES, errno.ENOENT):
                 return
             else:
                 raise
@@ -152,7 +146,7 @@ class CacheStore(object):
         try:
             with os.fdopen(tmp_fd, 'wb') as tmp_file:
                 pickle.dump(data, tmp_file)
-        except IOError as e:
+        except (IOError, OSError) as e:
             # No space left on device
             if e.errno == errno.ENOSPC:
                 self._remove_filename(tmp_filename)
@@ -162,7 +156,7 @@ class CacheStore(object):
 
         try:
             shutil.move(tmp_filename, store_filename)
-        except IOError as e:
+        except (IOError, OSError) as e:
             # Permission denied
             if e.errno == errno.EACCES:
                 self._remove_filename(tmp_filename)
@@ -175,7 +169,7 @@ class CacheStore(object):
             return
         try:
             fd = open(store_filename, 'rb')
-        except IOError as e:
+        except (IOError, OSError) as e:
             if e.errno == errno.ENOENT:
                 return None
             else:
diff --git a/giscanner/utils.py b/giscanner/utils.py
index df512d7b..4865ca8c 100644
--- a/giscanner/utils.py
+++ b/giscanner/utils.py
@@ -228,7 +228,7 @@ def makedirs(name, mode=0o777, exist_ok=False):
     if head and tail and not os.path.exists(head):
         try:
             makedirs(head, mode, exist_ok)
-        except OSError as e:
+        except (IOError, OSError) as e:
             # be happy if someone already created the path
             if e.errno != errno.EEXIST:
                 raise
@@ -236,7 +236,7 @@ def makedirs(name, mode=0o777, exist_ok=False):
             return
     try:
         os.mkdir(name, mode)
-    except OSError as e:
+    except (IOError, OSError) as e:
         if not exist_ok or e.errno != errno.EEXIST or not os.path.isdir(name):
             raise