Repository navigation
Conversation
097782b to
cf77453
Compare
Close queues a reply channel on done and waits for the reader to send on it after its cleanup. sendError takes that channel when the watcher is closing and drops it, so the reader never sees the shutdown request and Close blocks forever. These tests pin the channel handoff and the end-to-end Close; they fail before the fix.
sendError received the reply channel that Close queued on done and discarded it, unlike sendEvent, which puts it back. When nobody reads Errors any more and the backend then reports an error (an overflow, a failed CancelIo or CloseHandle), the reader handled the wakeup without the shutdown request, skipped the cleanup of its watches and the completion port, and Close waited on the reply forever. sendError now puts the channel back, as sendEvent does; TestSendErrorKeepsCloseReply and TestCloseAfterUnreadError fail without this change on windows-latest and pass with it.
Close read closed under the lock, released it, and set it under a second acquisition, so two concurrent calls could both proceed and queue two reply channels. done holds one, so the second Close blocked sending, and with an undeliverable error between them the reader, now keeping the first reply, could not drain the second: both calls and the reader hung. The check and the set now happen under one lock, so a second Close returns at once. TestConcurrentClose runs eight concurrent Close calls under a timeout and checks that no reply is left on done; the interleaving is a race, so it guards rather than reproduces.
…reply windows: keep Close's reply channel when an error cannot be delivered
PR SummaryMedium Risk Overview Recursive watching is turned on by default (
Reviewed by Cursor Bugbot for commit b260f7c. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b260f7c. Configure here.
|
|
||
| // Send the previous event first to maintain proper ordering, then create a new event for the current directory | ||
| // The function expects to return only one event, so we need to send the previous event first and then create a new one | ||
| w.sendEvent(ev) |
There was a problem hiding this comment.
Deadlock: sendEvent called while holding mutex
High Severity
w.sendEvent(ev) is called inside the WalkDir callback while w.mu is held (acquired at line 422 via handleEvent). On Linux, defaultBufferSize is 0, making Events unbuffered. If the channel blocks waiting for a consumer, and that consumer then calls Add, Remove, or WatchList (all of which acquire w.mu), a deadlock occurs. The existing sendEvent in readEvents (line 411) is safe because it runs after handleEvent returns and the deferred unlock fires; this new call inside the lock is not.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b260f7c. Configure here.
Tracks diff against the upstream