diff options
| author | Max Kellermann <max@duempel.org> | 2014-04-26 22:11:23 +0200 |
|---|---|---|
| committer | Max Kellermann <max@duempel.org> | 2014-04-26 22:11:23 +0200 |
| commit | 0efb67b51e0d9d34c65bbdbd9df567a8a991cc4c (patch) | |
| tree | 0eabbb79cdf144e36d37b38c8cc9040da5b1ae2a | |
| parent | 54ebf2a699777961ebb30bc4b4fca459880d4329 (diff) | |
| download | mpd-0efb67b51e0d9d34c65bbdbd9df567a8a991cc4c.tar.gz mpd-0efb67b51e0d9d34c65bbdbd9df567a8a991cc4c.tar.xz mpd-0efb67b51e0d9d34c65bbdbd9df567a8a991cc4c.zip | |
DeferredMonitor: fix race condition when using GLib event loop
Turns out the lock-free code using atomics was not thread-safe. The
given callback could be invoked by GLib before the source_id attribute
was assigned. This commit changes the DeferredMonitor class to use a
Mutex to block the event loop until source_id is assigned. This bug
does not exist in the 0.19 branch because it does not use the GLib
main loop anymore.
| -rw-r--r-- | NEWS | 1 | ||||
| -rw-r--r-- | src/event/DeferredMonitor.cxx | 28 | ||||
| -rw-r--r-- | src/event/DeferredMonitor.hxx | 5 |
3 files changed, 23 insertions, 11 deletions
@@ -1,4 +1,5 @@ ver 0.18.11 (not yet released) +* fix race condition when using GLib event loop (non-Linux) ver 0.18.10 (2014/04/10) * decoder diff --git a/src/event/DeferredMonitor.cxx b/src/event/DeferredMonitor.cxx index 4ffffaa89..62edb7817 100644 --- a/src/event/DeferredMonitor.cxx +++ b/src/event/DeferredMonitor.cxx @@ -27,9 +27,11 @@ DeferredMonitor::Cancel() #ifdef USE_EPOLL pending = false; #else - const auto id = source_id.exchange(0); - if (id != 0) - g_source_remove(id); + const ScopeLock protect(mutex); + if (source_id != 0) { + g_source_remove(source_id); + source_id = 0; + } #endif } @@ -40,10 +42,9 @@ DeferredMonitor::Schedule() if (!pending.exchange(true)) fd.Write(); #else - const unsigned id = loop.AddIdle(Callback, this); - const auto old_id = source_id.exchange(id); - if (old_id != 0) - g_source_remove(old_id); + const ScopeLock protect(mutex); + if (source_id == 0) + source_id = loop.AddIdle(Callback, this); #endif } @@ -65,9 +66,16 @@ DeferredMonitor::OnSocketReady(unsigned) void DeferredMonitor::Run() { - const auto id = source_id.exchange(0); - if (id != 0) - RunDeferred(); + { + const ScopeLock protect(mutex); + if (source_id == 0) + /* cancelled */ + return; + + source_id = 0; + } + + RunDeferred(); } gboolean diff --git a/src/event/DeferredMonitor.hxx b/src/event/DeferredMonitor.hxx index 2380fb66f..2ac832a0a 100644 --- a/src/event/DeferredMonitor.hxx +++ b/src/event/DeferredMonitor.hxx @@ -27,6 +27,7 @@ #include "SocketMonitor.hxx" #include "WakeFD.hxx" #else +#include "thread/Mutex.hxx" #include <glib.h> #endif @@ -48,7 +49,9 @@ class DeferredMonitor #else EventLoop &loop; - std::atomic<guint> source_id; + Mutex mutex; + + guint source_id; #endif public: |