Fix DirDeletedEvent not being raised for deleted directories on Windows#1156
Draft
wingding12 wants to merge 1 commit into
Draft
Fix DirDeletedEvent not being raised for deleted directories on Windows#1156wingding12 wants to merge 1 commit into
wingding12 wants to merge 1 commit into
Conversation
On Windows, when a directory was deleted, watchdog incorrectly raised FileDeletedEvent instead of DirDeletedEvent. This happened because deleted items cannot be queried with os.path.isdir() - they no longer exist. The fix uses ReadDirectoryChangesExW (available on Windows 10+) instead of ReadDirectoryChangesW. The extended API returns FILE_NOTIFY_EXTENDED_INFORMATION which includes FileAttributes, allowing us to correctly identify whether a deleted item was a directory. Changes: - Add ReadDirectoryChangesExW function and FileNotifyExtendedInformation structure to winapi.py - Add is_directory field to WinAPINativeEvent dataclass - Update _parse_event_buffer to extract FileAttributes and determine if the item was a directory - Update WindowsApiEmitter.queue_events to use is_directory for deleted events instead of always using FileDeletedEvent - Add tests for directory and file deletion events Fixes gorakhargosh#1153 Related to gorakhargosh#1154
Collaborator
|
I was not aware of this API, that's fantastic! It means will drop support for Windows <10. Please add a line in the changelog for the change + Windows support change. |
BoboTiG
reviewed
Jan 30, 2026
| 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 |
Collaborator
There was a problem hiding this comment.
You can remove the comment.
BoboTiG
requested changes
Jan 30, 2026
| LPVOID, # FileIOCompletionRoutine # lpCompletionRoutine | ||
| ) | ||
|
|
||
| # ReadDirectoryChangesExW is available on Windows 10 and later. |
Collaborator
There was a problem hiding this comment.
Then you can remove all stuff related to ReadDirectoryChangesW.
|
|
||
|
|
||
| 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]]: |
Collaborator
There was a problem hiding this comment.
No need to change the signature. Use extended attrs by default.
| class WinAPINativeEvent: | ||
| action: int | ||
| src_path: str | ||
| is_directory: bool = False # Whether the item was a directory (from extended info) |
Collaborator
There was a problem hiding this comment.
Remove the comment since the name is pretty explicit.
| buf = self._buf_queue.get(timeout=timeout) | ||
| except queue.Empty: | ||
| break | ||
| events.extend(_parse_event_buffer(buf)) |
Collaborator
There was a problem hiding this comment.
Since the signature won't change, those edits can be reverted.
| assert not emitter.should_keep_running() | ||
|
|
||
|
|
||
| def test_directory_deleted_event(event_queue, emitter): |
Collaborator
There was a problem hiding this comment.
Out of curiosity, existing tests should fail at some point with those changes, can you fix them instead of adding new ones?
Collaborator
|
Are you still on it @wingding12? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Windows, when a directory was deleted, watchdog incorrectly raised
FileDeletedEventinstead ofDirDeletedEvent. This happened because deleted items cannot be queried withos.path.isdir()- they no longer exist.Solution
Use
ReadDirectoryChangesExW(available on Windows 10+) instead ofReadDirectoryChangesW. The extended API returnsFILE_NOTIFY_EXTENDED_INFORMATIONwhich includesFileAttributes, allowing us to correctly identify whether a deleted item was a directory.Changes
src/watchdog/observers/winapi.py:ReadDirectoryChangesExWfunction definitionFileNotifyExtendedInformationstructure matching the Windows APIis_directoryfield toWinAPINativeEventdataclass_parse_event_bufferto extractFileAttributesand determine if the item was a directoryDirectoryChangeReader._run_innerto useReadDirectoryChangesExWsrc/watchdog/observers/read_directory_changes.py:WindowsApiEmitter.queue_eventsto useis_directoryfor deleted events instead of always usingFileDeletedEventtests/test_observers_winapi.py:test_directory_deleted_eventto verify directories produceDirDeletedEventtest_file_deleted_eventto verify files still produceFileDeletedEventNotes
ReadDirectoryChangesExWis only available on Windows 10 and later_parse_event_bufferfunction now accepts anextendedparameter to support both old and new formats for backward compatibilityRelated Issues
Fixes #1153
Related to #1154
Test Plan
DirDeletedEventis raised for deleted directoriesFileDeletedEventis still raised for deleted files