Merge pull request #375 from danilobellini/inotify

Inotify event order/duplication, fix #117 and #233

src/watchdog/observers/inotify.py

@@ -162,9 +162,6 @@ def queue_events(self, timeout, full_events=False):
             elif event.is_modify:
                 cls = DirModifiedEvent if event.is_directory else FileModifiedEvent
                 self.queue_event(cls(src_path))
-            elif event.is_delete_self:
-                cls = DirDeletedEvent if event.is_directory else FileDeletedEvent
-                self.queue_event(cls(src_path))
             elif event.is_delete or (event.is_moved_from and not full_events):
                 cls = DirDeletedEvent if event.is_directory else FileDeletedEvent
                 self.queue_event(cls(src_path))

src/watchdog/observers/inotify_c.py

@@ -248,7 +248,8 @@ def remove_watch(self, path):
             Path string for which the watch will be removed.
         """
         with self._lock:
-            wd = self._remove_watch_bookkeeping(path)
+            wd = self._wd_for_path.pop(path)
+            del self._path_for_wd[wd]
             if inotify_rm_watch(self._inotify_fd, wd) == -1:
                 Inotify._raise_error()
 
@@ -322,7 +323,9 @@ def _recursive_simulate(src_path):
 
                 if inotify_event.is_ignored:
                     # Clean up book-keeping for deleted watches.
-                    self._remove_watch_bookkeeping(src_path)
+                    path = self._path_for_wd.pop(wd)
+                    if self._wd_for_path[path] == wd:
+                        del self._wd_for_path[path]
                     continue
 
                 event_list.append(inotify_event)
@@ -387,11 +390,6 @@ def _add_watch(self, path, mask):
         self._path_for_wd[wd] = path
         return wd
 
-    def _remove_watch_bookkeeping(self, path):
-        wd = self._wd_for_path.pop(path)
-        del self._path_for_wd[wd]
-        return wd
-
     @staticmethod
     def _raise_error():
         """

tests/test_emitter.py

@@ -35,6 +35,7 @@
 from watchdog.observers.inotify import InotifyFullEmitter
 
 logging.basicConfig(level=logging.DEBUG)
+logger = logging.getLogger(__name__)
 
 
 def setup_function(function):
@@ -200,6 +201,34 @@ def test_delete_self():
     event_queue.get(timeout=5)[0]
 
 
+def test_fast_subdirectory_creation_deletion():
+    root_dir = p('dir1')
+    sub_dir = p('dir1', 'subdir1')
+    times = 30
+    mkdir(root_dir)
+    start_watching(root_dir)
+    for unused in range(times):
+        mkdir(sub_dir)
+        rm(sub_dir, True)
+    count = {DirCreatedEvent: 0,
+             DirModifiedEvent: 0,
+             DirDeletedEvent: 0}
+    etype_for_dir = {DirCreatedEvent: sub_dir,
+                     DirModifiedEvent: root_dir,
+                     DirDeletedEvent: sub_dir}
+    for unused in range(times * 4):
+        event = event_queue.get(timeout=5)[0]
+        logger.debug(event)
+        etype = type(event)
+        count[etype] += 1
+        assert event.src_path == etype_for_dir[etype]
+        assert count[DirCreatedEvent] >= count[DirDeletedEvent]
+        assert count[DirCreatedEvent] + count[DirDeletedEvent] >= count[DirModifiedEvent]
+    assert count == {DirCreatedEvent: times,
+                     DirModifiedEvent: times * 2,
+                     DirDeletedEvent: times}
+
+
 def test_passing_unicode_should_give_unicode():
     start_watching(p(''))
     touch(p('a'))

tests/test_inotify_c.py

@@ -0,0 +1,117 @@
+from __future__ import unicode_literals
+import os
+import pytest
+import logging
+import contextlib
+from tests import Queue
+from functools import partial
+from .shell import rm, mkdtemp
+from watchdog.utils import platform
+from watchdog.events import DirCreatedEvent, DirDeletedEvent, DirModifiedEvent
+from watchdog.observers.api import ObservedWatch
+
+if platform.is_linux():
+    from watchdog.observers.inotify import InotifyFullEmitter, InotifyEmitter
+
+
+logging.basicConfig(level=logging.DEBUG)
+logger = logging.getLogger(__name__)
+
+
+def setup_function(function):
+    global p, event_queue
+    tmpdir = os.path.realpath(mkdtemp())
+    p = partial(os.path.join, tmpdir)
+    event_queue = Queue()
+
+
+@contextlib.contextmanager
+def watching(path=None, use_full_emitter=False):
+    path = p('') if path is None else path
+    global emitter
+    Emitter = InotifyFullEmitter if use_full_emitter else InotifyEmitter
+    emitter = Emitter(event_queue, ObservedWatch(path, recursive=True))
+    emitter.start()
+    yield
+    emitter.stop()
+    emitter.join(5)
+
+
+def teardown_function(function):
+    rm(p(''), recursive=True)
+    assert not emitter.is_alive()
+
+
+@pytest.mark.skipif(not platform.is_linux(),
+                    reason="Testing with inotify messages (Linux only)")
+def test_late_double_deletion(monkeypatch):
+    inotify_fd = type(str("FD"), (object,), {})() # Empty object
+    inotify_fd.last = 0
+    inotify_fd.wds = []
+
+    # CREATE DELETE CREATE DELETE DELETE_SELF IGNORE DELETE_SELF IGNORE
+    inotify_fd.buf = (
+      # IN_CREATE|IS_DIR (wd = 1, path = subdir1)
+      b"\x01\x00\x00\x00\x00\x01\x00\x40\x00\x00\x00\x00\x10\x00\x00\x00"
+      b"\x73\x75\x62\x64\x69\x72\x31\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+      # IN_DELETE|IS_DIR (wd = 1, path = subdir1)
+      b"\x01\x00\x00\x00\x00\x02\x00\x40\x00\x00\x00\x00\x10\x00\x00\x00"
+      b"\x73\x75\x62\x64\x69\x72\x31\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+    ) * 2 + (
+      # IN_DELETE_SELF (wd = 2)
+      b"\x02\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+      # IN_IGNORE (wd = 2)
+      b"\x02\x00\x00\x00\x00\x80\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+      # IN_DELETE_SELF (wd = 3)
+      b"\x03\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+      # IN_IGNORE (wd = 3)
+      b"\x03\x00\x00\x00\x00\x80\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+    )
+
+    os_read_bkp = os.read
+    def fakeread(fd, length):
+        if fd is inotify_fd:
+            result, fd.buf = fd.buf[:length], fd.buf[length:]
+            return result
+        return os_read_bkp(fd, length)
+
+    os_close_bkp = os.close
+    def fakeclose(fd):
+        if fd is not inotify_fd:
+            os_close_bkp(fd)
+
+    def inotify_init():
+        return inotify_fd
+
+    def inotify_add_watch(fd, path, mask):
+        fd.last += 1
+        logger.debug("New wd = %d" % fd.last)
+        fd.wds.append(fd.last)
+        return fd.last
+
+    def inotify_rm_watch(fd, wd):
+        logger.debug("Removing wd = %d" % wd)
+        fd.wds.remove(wd)
+        return 0
+
+    # Mocks the API!
+    from watchdog.observers import inotify_c
+    monkeypatch.setattr(os, "read", fakeread)
+    monkeypatch.setattr(os, "close", fakeclose)
+    monkeypatch.setattr(inotify_c, "inotify_init", inotify_init)
+    monkeypatch.setattr(inotify_c, "inotify_add_watch", inotify_add_watch)
+    monkeypatch.setattr(inotify_c, "inotify_rm_watch", inotify_rm_watch)
+
+    with watching(p('')):
+        # Watchdog Events
+        for evt_cls in [DirCreatedEvent, DirDeletedEvent] * 2:
+            event = event_queue.get(timeout=5)[0]
+            assert isinstance(event, evt_cls)
+            assert event.src_path == p('subdir1')
+            event = event_queue.get(timeout=5)[0]
+            assert isinstance(event, DirModifiedEvent)
+            assert event.src_path == p('').rstrip(os.path.sep)
+
+    assert inotify_fd.last == 3 # Number of directories
+    assert inotify_fd.buf == b"" # Didn't miss any event
+    assert inotify_fd.wds == [2, 3] # Only 1 is removed explicitly