From 0efb67b51e0d9d34c65bbdbd9df567a8a991cc4c Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@duempel.org>
Date: Sat, 26 Apr 2014 22:11:23 +0200
Subject: [PATCH] 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.
---
 NEWS                          |  1 +
 src/event/DeferredMonitor.cxx | 28 ++++++++++++++++++----------
 src/event/DeferredMonitor.hxx |  5 ++++-
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/NEWS b/NEWS
index 27c43988a..ff0d0f141 100644
--- a/NEWS
+++ b/NEWS
@@ -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: