From 50944a6ef75c29555ecb36cbfa37559f788ce8b6 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sat, 7 Sep 2024 07:48:39 +0200 Subject: [PATCH] Deadlock between Watcher and Debounce threads (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 --- src/Watcher.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Watcher.cc b/src/Watcher.cc index abb7ae6..4c7e24e 100644 --- a/src/Watcher.cc +++ b/src/Watcher.cc @@ -66,6 +66,11 @@ void Watcher::notify() { mCond.notify_all(); if (mCallbacks.size() > 0 && mEvents.size() > 0) { + // We must release our lock before calling into the debouncer + // to avoid a deadlock: the debouncer thread itself will require + // our lock from its thread when calling into `triggerCallbacks` + // while holding its own debouncer lock. + lk.unlock(); mDebounce->trigger(); } }