Skip to content

Commit

Permalink
Avoid race condition crash when a directory is swapped for a file
Browse files Browse the repository at this point in the history
This commit builds off the race condition checking introduced in
2e83199, 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.
  • Loading branch information
pR0Ps authored and BoboTiG committed Feb 16, 2019
1 parent c7e6f8d commit 19eb494
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 3 deletions.
5 changes: 3 additions & 2 deletions src/watchdog/utils/dirsnapshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 18 additions & 1 deletion tests/test_snapshot_diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

0 comments on commit 19eb494

Please sign in to comment.