diff --git a/src/watchdog/observers/read_directory_changes.py b/src/watchdog/observers/read_directory_changes.py index 5d00980b..d249bd59 100644 --- a/src/watchdog/observers/read_directory_changes.py +++ b/src/watchdog/observers/read_directory_changes.py @@ -94,7 +94,9 @@ def queue_events(self, timeout: float) -> None: for sub_created_event in generate_sub_created_events(src_path): self.queue_event(sub_created_event) elif winapi_event.is_removed: - self.queue_event(FileDeletedEvent(src_path)) + # Use is_directory from extended file info to correctly identify + # deleted directories (fixes issue #1153) + self.queue_event((DirDeletedEvent if winapi_event.is_directory else FileDeletedEvent)(src_path)) elif winapi_event.is_removed_self: self.queue_event(DirDeletedEvent(self.watch.path)) should_stop = True diff --git a/src/watchdog/observers/winapi.py b/src/watchdog/observers/winapi.py index 6bdd5a11..e198567d 100644 --- a/src/watchdog/observers/winapi.py +++ b/src/watchdog/observers/winapi.py @@ -120,6 +120,28 @@ def _errcheck_dword(value: Any | None, func: Any, args: Any) -> Any: LPVOID, # FileIOCompletionRoutine # lpCompletionRoutine ) +# ReadDirectoryChangesExW is available on Windows 10 and later. +# It provides extended information including FileAttributes which allows +# us to determine if a deleted item was a directory. +ReadDirectoryChangesExW = kernel32.ReadDirectoryChangesExW +ReadDirectoryChangesExW.restype = BOOL +ReadDirectoryChangesExW.errcheck = _errcheck_bool +ReadDirectoryChangesExW.argtypes = ( + HANDLE, # hDirectory + LPVOID, # lpBuffer + DWORD, # nBufferLength + BOOL, # bWatchSubtree + DWORD, # dwNotifyFilter + ctypes.POINTER(DWORD), # lpBytesReturned + ctypes.POINTER(OVERLAPPED), # lpOverlapped + LPVOID, # lpCompletionRoutine + DWORD, # ReadDirectoryNotifyInformationClass +) + +# ReadDirectoryNotifyInformationClass values +ReadDirectoryNotifyInformation = 1 +ReadDirectoryNotifyExtendedInformation = 2 + CreateFileW = kernel32.CreateFileW CreateFileW.restype = HANDLE CreateFileW.errcheck = _errcheck_handle @@ -224,6 +246,33 @@ class FileNotifyInformation(ctypes.Structure): LPFNI = ctypes.POINTER(FileNotifyInformation) +# FILE_NOTIFY_EXTENDED_INFORMATION structure for ReadDirectoryChangesExW +# https://learn.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-file_notify_extended_information +class FileNotifyExtendedInformation(ctypes.Structure): + _fields_ = ( + ("NextEntryOffset", DWORD), + ("Action", DWORD), + ("CreationTime", ctypes.c_longlong), + ("LastModificationTime", ctypes.c_longlong), + ("LastChangeTime", ctypes.c_longlong), + ("LastAccessTime", ctypes.c_longlong), + ("AllocatedLength", ctypes.c_longlong), + ("FileSize", ctypes.c_longlong), + ("FileAttributes", DWORD), + ("ReparsePointTag", DWORD), + ("FileId", ctypes.c_longlong), + ("ParentFileId", ctypes.c_longlong), + ("FileNameLength", DWORD), + ("FileName", (ctypes.c_char * 1)), + ) + + +LPFNEI = ctypes.POINTER(FileNotifyExtendedInformation) + +# File attribute constant for directories +FILE_ATTRIBUTE_DIRECTORY = 0x10 + + # We don't need to recalculate these flags every time a call is made to # the win32 API functions. WATCHDOG_FILE_FLAGS = FILE_FLAG_BACKUP_SEMANTICS @@ -262,15 +311,35 @@ class FileNotifyInformation(ctypes.Structure): PATH_BUFFER_SIZE = 2048 -def _parse_event_buffer(read_buffer: bytes) -> list[tuple[int, str]]: +def _parse_event_buffer(read_buffer: bytes, *, extended: bool = False) -> list[tuple[int, str, bool]]: + """Parse the event buffer from ReadDirectoryChangesW/ReadDirectoryChangesExW. + + Args: + read_buffer: The raw buffer from the Windows API call. + extended: If True, parse as FILE_NOTIFY_EXTENDED_INFORMATION (from ReadDirectoryChangesExW). + If False, parse as FILE_NOTIFY_INFORMATION (from ReadDirectoryChangesW). + + Returns: + List of tuples: (action, filename, is_directory) + When extended=False, is_directory is always False (unknown). + """ n_bytes = len(read_buffer) results = [] while n_bytes > 0: - fni = ctypes.cast(read_buffer, LPFNI)[0] # type: ignore[arg-type] - ptr = ctypes.addressof(fni) + FileNotifyInformation.FileName.offset - filename = ctypes.string_at(ptr, fni.FileNameLength) - results.append((fni.Action, filename.decode("utf-16"))) - num_to_skip = fni.NextEntryOffset + if extended: + fnei = ctypes.cast(read_buffer, LPFNEI)[0] # type: ignore[arg-type] + ptr = ctypes.addressof(fnei) + FileNotifyExtendedInformation.FileName.offset + filename = ctypes.string_at(ptr, fnei.FileNameLength) + is_dir = bool(fnei.FileAttributes & FILE_ATTRIBUTE_DIRECTORY) + results.append((fnei.Action, filename.decode("utf-16"), is_dir)) + num_to_skip = fnei.NextEntryOffset + else: + fni = ctypes.cast(read_buffer, LPFNI)[0] # type: ignore[arg-type] + ptr = ctypes.addressof(fni) + FileNotifyInformation.FileName.offset + filename = ctypes.string_at(ptr, fni.FileNameLength) + # Without extended info, we cannot determine if it was a directory + results.append((fni.Action, filename.decode("utf-16"), False)) + num_to_skip = fni.NextEntryOffset if num_to_skip <= 0: break read_buffer = read_buffer[num_to_skip:] @@ -329,6 +398,7 @@ def _close_directory_handle(handle: HANDLE) -> None: class WinAPINativeEvent: action: int src_path: str + is_directory: bool = False # Whether the item was a directory (from extended info) @property def is_added(self) -> bool: @@ -373,13 +443,18 @@ def __init__(self, path: str, *, recursive: bool) -> None: def _run_inner(self, handle: HANDLE, event_buffer: ctypes.Array[ctypes.c_char], nbytes: DWORD) -> None: # This runs in its own thread and it makes a single call to - # ReadDirectoryChangesW(). This blocks until at least some events are + # ReadDirectoryChangesExW(). This blocks until at least some events are # available (or there is an error). We will re-use the _event_buffer # and so we copy the data into a second buffer (double-buffering # technique) and queue it for another thread to process. This reduces # the time window in which we could miss events. + # + # We use ReadDirectoryChangesExW instead of ReadDirectoryChangesW to get + # extended information including FileAttributes, which allows us to + # correctly identify deleted directories (see issue #1153). + # Note: ReadDirectoryChangesExW is only available on Windows 10+. try: - ReadDirectoryChangesW( + ReadDirectoryChangesExW( handle, ctypes.byref(event_buffer), len(event_buffer), @@ -388,6 +463,7 @@ def _run_inner(self, handle: HANDLE, event_buffer: ctypes.Array[ctypes.c_char], ctypes.byref(nbytes), None, None, + ReadDirectoryNotifyExtendedInformation, ) buf = event_buffer.raw[: nbytes.value] except OSError as e: @@ -396,7 +472,7 @@ def _run_inner(self, handle: HANDLE, event_buffer: ctypes.Array[ctypes.c_char], if _is_observed_path_deleted(handle, self._path): # Handle the case when the root path is deleted with self._lock: - # Additional calls to ReadDirectoryChangesW() will fail so stop. + # Additional calls to ReadDirectoryChangesExW() will fail so stop. self._should_stop = True # Create synthetic event for deletion buf = _generate_observed_path_deleted_event() @@ -464,5 +540,6 @@ def get_events(self, timeout: float) -> list[WinAPINativeEvent]: buf = self._buf_queue.get(timeout=timeout) except queue.Empty: break - events.extend(_parse_event_buffer(buf)) - return [WinAPINativeEvent(action, src_path) for action, src_path in events] + # Use extended=True since we're using ReadDirectoryChangesExW + events.extend(_parse_event_buffer(buf, extended=True)) + return [WinAPINativeEvent(action, src_path, is_dir) for action, src_path, is_dir in events] diff --git a/tests/test_observers_winapi.py b/tests/test_observers_winapi.py index 42d509ce..4b51c42e 100644 --- a/tests/test_observers_winapi.py +++ b/tests/test_observers_winapi.py @@ -7,7 +7,7 @@ import pytest -from watchdog.events import DirCreatedEvent, DirMovedEvent +from watchdog.events import DirCreatedEvent, DirDeletedEvent, DirMovedEvent, FileCreatedEvent, FileDeletedEvent from watchdog.observers.api import ObservedWatch from watchdog.utils import platform @@ -127,3 +127,85 @@ def test_root_deleted(event_queue, emitter): # The emitter is automatically stopped, with no error assert not emitter.should_keep_running() + + +def test_directory_deleted_event(event_queue, emitter): + """Test that deleting a directory produces DirDeletedEvent, not FileDeletedEvent. + + This is a regression test for issue #1153 where deleted directories were + incorrectly reported as FileDeletedEvent because os.path.isdir() cannot + determine the type of a deleted path. + + The fix uses ReadDirectoryChangesExW which provides FileAttributes in the + event data, allowing us to correctly identify deleted directories. + """ + emitter.start() + sleep(SLEEP_TIME) + + # Create a directory and a file + mkdir(p("testdir")) + sleep(SLEEP_TIME) + + # Delete the directory + rm(p("testdir"), recursive=True) + sleep(SLEEP_TIME) + + emitter.stop() + sleep(SLEEP_TIME) + + got = [] + while True: + try: + event, _ = event_queue.get_nowait() + except Empty: + break + else: + if event.event_type == "modified": + # Ignore modified events for deterministic tests + continue + got.append(event) + + # Should have DirCreatedEvent and DirDeletedEvent (not FileDeletedEvent) + assert DirCreatedEvent(p("testdir")) in got + assert DirDeletedEvent(p("testdir")) in got + # Make sure we didn't get FileDeletedEvent for the directory + assert FileDeletedEvent(p("testdir")) not in got + + +def test_file_deleted_event(event_queue, emitter): + """Test that deleting a file still produces FileDeletedEvent. + + This ensures our fix for directory deletion doesn't break file deletion events. + """ + from .shell import touch + + emitter.start() + sleep(SLEEP_TIME) + + # Create a file + touch(p("testfile.txt")) + sleep(SLEEP_TIME) + + # Delete the file + rm(p("testfile.txt")) + sleep(SLEEP_TIME) + + emitter.stop() + sleep(SLEEP_TIME) + + got = [] + while True: + try: + event, _ = event_queue.get_nowait() + except Empty: + break + else: + if event.event_type == "modified": + continue + got.append(event) + + # Should have FileCreatedEvent and FileDeletedEvent + assert FileCreatedEvent(p("testfile.txt")) in got + assert FileDeletedEvent(p("testfile.txt")) in got + # Make sure we didn't get DirDeletedEvent for the file + assert DirDeletedEvent(p("testfile.txt")) not in got