From 19eb494dce9fc01c9821a0b909755c3cca94593f Mon Sep 17 00:00:00 2001 From: Carey Metcalfe Date: Wed, 20 Dec 2017 00:09:05 -0500 Subject: [PATCH] Avoid race condition crash when a directory is swapped for a file This commit builds off the race condition checking introduced in 2e8319992, but also correctly handles the case where a directory is replaced with a file before it can be walked. For example: 1. [external] create /a/b/c.txt 2. [watchdog] walk /a (contains directory /a/b/ at this point) 3. [external] rm -rf /a/b/ 4. [external] touch /a/b 5. [watchdog] walk /a/b/ (raises NotADirectoryError/ENOTDIR) Also adds a test case that runs through the above example to ensure that an exception is no longer raised. --- src/watchdog/utils/dirsnapshot.py | 5 +++-- tests/test_snapshot_diff.py | 19 ++++++++++++++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/watchdog/utils/dirsnapshot.py b/src/watchdog/utils/dirsnapshot.py index d4aaeb8a8..320406c1e 100644 --- a/src/watchdog/utils/dirsnapshot.py +++ b/src/watchdog/utils/dirsnapshot.py @@ -218,8 +218,9 @@ def walk(root): except OSError as e: # Directory may have been deleted between finding it in the directory # list of its parent and trying to delete its contents. If this - # happens we treat it as empty. - if e.errno == errno.ENOENT: + # happens we treat it as empty. Likewise if the directory was replaced + # with a file of the same name (less likely, but possible). + if e.errno == errno.ENOENT or e.errno == errno.ENOTDIR: return else: raise diff --git a/tests/test_snapshot_diff.py b/tests/test_snapshot_diff.py index f1bce3548..1c61925a6 100644 --- a/tests/test_snapshot_diff.py +++ b/tests/test_snapshot_diff.py @@ -14,8 +14,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os import time -from .shell import mkdir, touch, mv +from .shell import mkdir, touch, mv, rm from watchdog.utils.dirsnapshot import DirectorySnapshot from watchdog.utils.dirsnapshot import DirectorySnapshotDiff from watchdog.utils import platform @@ -105,3 +106,19 @@ def test_detect_modify_for_moved_files(p): diff = DirectorySnapshotDiff(ref, DirectorySnapshot(p(''))) assert diff.files_moved == [(p('a'), p('b'))] assert diff.files_modified == [p('a')] + +def test_replace_dir_with_file(p): + # Replace a dir with a file of the same name just before the normal listdir + # call and ensure it doesn't cause an exception + + def listdir_fcn(path): + if path == p("root", "dir"): + rm(path, recursive=True) + touch(path) + return os.listdir(path) + + mkdir(p('root')) + mkdir(p('root', 'dir')) + + # Should NOT raise an OSError (ENOTDIR) + DirectorySnapshot(p('root'), listdir=listdir_fcn)