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.
This commit is contained in:
		
							
								
								
									
										1
									
								
								NEWS
									
									
									
									
									
								
							
							
						
						
									
										1
									
								
								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
 | 
			
		||||
 
 | 
			
		||||
@@ -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
 | 
			
		||||
 
 | 
			
		||||
@@ -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:
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user