rjeczalik / notify

File system event notification library on steroids.
MIT License
902 stars 128 forks source link

readdcw: Check state on every loop iteration #127

Closed imsodin closed 7 years ago

imsodin commented 7 years ago

An incoming event can contain an actual change (n != 0) and at the same time have a changed state (filter&onlyMachineStates == 0). If this changed state is not handled, it wont be as it will not trigger another iteration (syscall.GetQueuedCompletionStatus doesn't return again).

Scenario:
Stop a watch at the same time as an event is created (in original case: file removed). Then Watch it again. After that no more events are generated and calling Stop again logs an error.

I added some debug statements to the tests where this error occurs, they should be self-explanatory (tell me if not or if I should add them to the PR). In the following log the first time after Stop the change in state is picked up, the second time it is not and then no more events are registered even though files are created. The third time Stop itself reports an error.

2017/08/27 14:35:08.535964 [D] Stop(0xc04203aea0) error: <nil>
2017/08/27 14:35:08.535964 [D] loopstate: stateUnwatch
2017/08/27 14:35:09.950001 [D] dispatching notify.FileActionRenamedOldName on "C:\\Temp\\syncthing-d3d5715d\\src\\github.com\\syncthing\\syncthing\\lib\\fswatcher\\temporary_test_fswatcher\\oldfile"
2017/08/27 14:35:09.950001 [D] dispatching notify.FileActionRenamedNewName on "C:\\Temp\\syncthing-d3d5715d\\src\\github.com\\syncthing\\syncthing\\lib\\fswatcher\\temporary_test_fswatcher\\newfile"
2017/08/27 14:35:13.651173 [D] Stop(0xc04203b500) error: <nil>
2017/08/27 14:35:13.653176 [D] dispatching notify.FileActionRemoved on "C:\\Temp\\syncthing-d3d5715d\\src\\github.com\\syncthing\\syncthing\\lib\\fswatcher\\temporary_test_fswatcher\\newfile"
2017/08/27 14:35:16.366990 [D] Stop(0xc04203b980) error: notify: another re/unwatching operation in progress
imsodin commented 7 years ago

Apparently this also triggers the race already found in #124, so I include the fix into this PR (the other way is mainly about fsevents/mac anyway).

Edit: Should I squash the two commits?

imsodin commented 7 years ago

See #129