windows: Fix server hangs under some circumstance

[JunkuiZhang]

Hi, thank you so much for creating such a high-quality Rust library.

We encountered a tricky issue while using this library in the development of Zed (https://github.com/zed-industries/zed). After testing, we identified the issue as a bug in notify. This PR provides a test case, test_windows_trash_dir(), which reproduces the bug.

The bug can be reproduced as follows: in a project directory /path/to/project, there is a subfolder /path/to/project/child. If we monitor changes in the child folder, then move the child folder to the trash (deleting the folder directly does not trigger the bug), and monitor the project directory, the server gets stuck at the WaitForSingleObjectEx command.

This issue seems to be related to the bAlertable flag in WaitForSingleObjectEx, but we are still unclear why setting bAlertable to TRUE causes this bug.

[JunkuiZhang]

Moving a folder to the Recycle Bin or deleting a folder triggers ERROR_ACCESS_DENIED, but this error wasn't handled properly, leading to an infinite loop. Here’s what happens in detail:

1. When dir_a is being watched, the server calls ReadDirectoryChangesW to monitor changes and registers handle_event as the completion routine.

2. After dir_a is moved to the Recycle Bin, a change is detected, triggering handle_event. At this point, error_code is ERROR_ACCESS_DENIED.

3. The original code does not handle this error, so execution continues, calling start_read, which in turn calls ReadDirectoryChangesW again.

4. Since dir_a no longer exists, ReadDirectoryChangesW immediately returns 1, which is interpreted as a successful call. This causes handle_event to be registered again.

5. Windows then calls handle_event again, with ERROR_ACCESS_DENIED, repeating the cycle indefinitely:

   handle_event(ERROR_ACCESS_DENIED) -> start_read(ReadDirectoryChangesW, return 1) -> handle_event(ERROR_ACCESS_DENIED) -> start_read(ReadDirectoryChangesW, return 1)...
   

Interestingly, when the folder is deleted instead of moved to the Recycle Bin, the second ReadDirectoryChangesW call returns 0, indicating failure, which prevents the loop. Additionally, setting bAlertable = FALSE stops the completion routine from being called, avoiding the issue entirely.

Changes in this PR:

• Handle ERROR_ACCESS_DENIED: If the folder no longer exists, it has either been deleted or moved to the Recycle Bin, so we unwatch it.
• Improve error handling: Known error codes are handled appropriately. For unknown error codes, start_read is not called to prevent potential infinite loops in the future.

[dfaust]

Hi @JunkuiZhang. Thanks so much for fixing this issue. The changes look good.
Unfortunately I don't have Windows to test. But I think we can merge this once the pipeline has succeeded. The pipeline works on the current main branch. So maybe a rebase will fix the issue.
Also, would you mind adding a change-log entry?

[JunkuiZhang]

Hi @JunkuiZhang. Thanks so much for fixing this issue. The changes look good. Unfortunately I don't have Windows to test. But I think we can merge this once the pipeline has succeeded. The pipeline works on the current main branch. So maybe a rebase will fix the issue. Also, would you mind adding a change-log entry?

I’ve rebased my changes onto the main branch and added the relevant entry to the changelog. BTW, if there’s anything that doesn’t look right or if there’s anything else you’d like me to tweak, just let me know!

[dfaust]

Thanks for the quick update.

[target.'cfg(windows)'.dev-dependencies]
trash.workspace = true

Should fix the CI issue.

[dfaust]

Found two micro issues :)

[dfaust]

All other log messages start lower case.

[JunkuiZhang]

Fixed

[dfaust]

happen

[JunkuiZhang]

Thanks!

[dfaust]

Thank you very much!