From 0efb67b51e0d9d34c65bbdbd9df567a8a991cc4c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 26 Apr 2014 22:11:23 +0200 Subject: 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. --- src/event/DeferredMonitor.cxx | 28 ++++++++++++++++++---------- src/event/DeferredMonitor.hxx | 5 ++++- 2 files changed, 22 insertions(+), 11 deletions(-) (limited to 'src/event') 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 #endif @@ -48,7 +49,9 @@ class DeferredMonitor #else EventLoop &loop; - std::atomic source_id; + Mutex mutex; + + guint source_id; #endif public: -- cgit v1.2.3