Inotify event order/duplication, fix #117 and #233 #375
Conversation
This file contains 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
- This is related to gorakhargosh#117 and gorakhargosh#233 - It's about a single path that is has 2 wd values, i.e., a subdirectory path that was created again before the "bookkeeping removal" event (IN_IGNORE) of a deletion was processed - The test is using a stochastical approach to reach the bug, in the sense that being "fast" isn't enough to enforce a particular event order; however, a "lucky" event synchronization seems extremely rare and the package should pass on every case - Even with an unknown order for the events, the test is counting and enforces: - Watchdog emits the 120 events that happened - Specific number of events for each type - Order constraing as required for minimal consistency (e.g. not deleting a directory that wasn't created)
- The call on inotify_event.is_ignored was removing the wrong wd, due to another wd with the same path (self._wd_for_path[src_path] was replaced) - Fix gorakhargosh#117 and gorakhargosh#233 - Still not enough to fix every bug regarding fast/unordered events, but doesn't crash anymore
c = IN_CREATE|IS_DIR d = IN_DELETE|IS_DIR ds = IN_DELETE_SELF i = IN_IGNORE These are inotify events, not watchdog events The new test is written in a new module because it required a different teardown function to stop the emitter before undoing the mock
Inotify emits a IN_DELETE|IS_DIR event and a IN_DELETE_SELF event (in this order) when a watched directory is removed, the former event happens for all directories and generates a pair of a DirDeletedEvent and a DirModifiedEvent instances, the latter used to generate a (duplicated and unrequired) DirDeletedEvent With this change, the test_fast_subdirectory_creation_deletion no longer fails
|
I'm testing this with #368. Before pushing, I've checked that the tests are passing on all environments. Commands like |
|
Since 17 Aug, this fix is also in a patch applied by dose to use watchdog. |
|
I wonder why no one commented on this PR yet. |
|
Thank you for spending the time to fix these issues. Sorry that we took so long to get around to answering, I didn't know I was the only one looking at these. |
CCP-Aporia
pushed a commit
to CCP-Aporia/watchdog
that referenced
this pull request
Aug 13, 2020
Inotify event order/duplication, fix gorakhargosh#117 and gorakhargosh#233
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Add 2 new tests that addresses #117 and #233, fix them and fix a
DirDeletedEventduplication found on Linux when deleting a directory:observers.inotify_c.Inotify._remove_watch_bookkeepinghelper method, as it would mixwdandsrc_pathwhen removing a path that was assigned to more than onewd. The items on thewdto/fromsrc_pathmappings are still being removed, but consistently and inlined;DirDeletedEventwas emitted twice as both theIN_DELETE|IS_DIRevent and theIN_DELETE_SELFevent were triggering a watchdog event, that was fixed by removing the latter event;IN_DELETE_SELF, mocking the Inotify API for that. This test doesn't depend on randomness/speed). The mock/monkeypatch required another test module to properly stop the emitter.The
IN_DELETE_SELFis a directory-only event, so this inotify event doesn't have anIS_DIRflag. The former code that evaluatedcls = DirDeletedEvent if event.is_directory else FileDeletedEventon such an event is an evidence that it wasn't being tested.