Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deadlock between Watcher and Debounce threads (fix #187) #189

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

bpasero
Copy link
Contributor

@bpasero bpasero commented Sep 7, 2024

fix #187

There are two threads involved in Watcher.cc and Debounce.cc each
calling into respective methods of the other and thus each potentially
holding onto locks of the other. This can lead to a deadlock in the
following scenario:

While the Debounce.cc thread is processing callbacks in the
Debounce::notify() method, it holds its own lock. The method loops
over callbacks to process in Watcher.cc which itself requires a lock
in Watcher.cc. If an event gets reported while the debouncer is in
Watcher.triggerCallbacks(), a deadlock is present, because:

  • assume the event thread is thread A
  • assume the debouncer thread is thread B
  • A holds its own lock in Watcher::notify() and calls into
    Debounce.trigger() which requires the debouncer lock
  • B at this time is inside Debounce::notify() holding its own lock
    processing callbacks and about to call into Watcher.triggerCallbacks()
  • A deadlocks waiting for B to release the debouncer lock
  • B deadlocks waiting for A to release the watcher lock

There are two threads involved in `Watcher.cc` and `Debounce.cc` each
calling into respective methods of the other and thus each potentially
holding onto locks of the other. This can lead to a deadlock in the
following scenario:

While the `Debounce.cc` thread is processing callbacks in the
`Debounce::notify()` method, it holds its own lock. The method loops
over callbacks to process in `Watcher.cc` which itself requires a lock
in `Watcher.cc`. If an event gets reported while the debouncer is in
`Watcher.triggerCallbacks()`, a deadlock is present, because:
- assume the event thread is thread A
- assume the debouncer thread is thread B
- A holds its own lock in `Watcher::notify()` and calls into
  `Debounce.trigger()` which requires the debouncer lock
- B at this time is inside `Debounce::notify()` holding its own lock
  processing callbacks and about to call into `Watcher.triggerCallbacks()`
- A deadlocks waiting for B to release the debouncer lock
- B deadlocks waiting for A to release the watcher lock
@devongovett
Copy link
Member

Thanks for the deep investigation here! This looks good to me.

@jmeinke
Copy link

jmeinke commented Oct 10, 2024

Could this be merged and published soon, please? We're desperately waiting for a fix :)

@bpasero
Copy link
Contributor Author

bpasero commented Oct 26, 2024

@devongovett 👋 yeah, would be great to see a new version with the fix included

@devongovett devongovett merged commit 9b7dac3 into parcel-bundler:master Nov 4, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deadlock between Watcher and Debounce threads
3 participants