diff src/modules/thread/Channel.cpp @ 1304:ee1448bb9680 minor

Fix race conditions and deadlocks in Channels (hopefully) (issue #554) Also, this is a giant commit because of the official Code Style (tm) was applied.
author Bart van Strien <bart.bes+projects@gmail.com>
date Wed, 30 Jan 2013 17:15:12 +0100
parents 1a59c21cd2f3
children
line wrap: on
line diff
--- a/src/modules/thread/Channel.cpp	Sun Jan 27 20:04:16 2013 +0100
+++ b/src/modules/thread/Channel.cpp	Wed Jan 30 17:15:12 2013 +0100
@@ -24,55 +24,63 @@
 
 namespace
 {
-	union uslong
-	{
-		unsigned long u;
-		long i;
-	};
+union uslong
+{
+	unsigned long u;
+	long i;
+};
 
-	// target <= current, but semi-wrapsafe, one wrap, anyway
-	inline bool past(unsigned int target, unsigned int current)
-	{
-		if (target > current)
-			return false;
-		if (target == current)
-			return true;
+// target <= current, but semi-wrapsafe, one wrap, anyway
+inline bool past(unsigned int target, unsigned int current)
+{
+	if (target > current)
+		return false;
+	if (target == current)
+		return true;
 
-		uslong t, c;
-		t.u = target;
-		c.u = current;
+	uslong t, c;
+	t.u = target;
+	c.u = current;
 
-		return !(t.i < 0 && c.i > 0);
-	}
+	return !(t.i < 0 && c.i > 0);
+}
 }
 
 namespace love
 {
 namespace thread
 {
-static std::map<std::string, Channel*> namedChannels;
+static std::map<std::string, Channel *> namedChannels;
 static Mutex *namedChannelMutex;
+
 Channel *Channel::getChannel(const std::string &name)
 {
 	if (!namedChannelMutex)
 		namedChannelMutex = newMutex();
+
 	Lock l(namedChannelMutex);
 	if (!namedChannels.count(name))
 		namedChannels[name] = new Channel(name);
 	else
 		namedChannels[name]->retain();
+
 	return namedChannels[name];
 }
 
 Channel::Channel()
-	: named(false), sent(0), received(0)
+	: named(false)
+	, sent(0)
+	, received(0)
 {
 	mutex = newMutex();
 	cond = newConditional();
 }
 
 Channel::Channel(const std::string &name)
-	: named(true), name(name), sent(0), received(0)
+	: named(true)
+	, name(name)
+	, sent(0)
+	, received(0)
 {
 	mutex = newMutex();
 	cond = newConditional();
@@ -88,6 +96,7 @@
 
 	delete mutex;
 	delete cond;
+
 	if (named)
 		namedChannels.erase(name);
 }
@@ -96,12 +105,15 @@
 {
 	if (!var)
 		return 0;
+
 	Lock l(mutex);
 	var->retain();
+
 	// Keep a reference to ourselves
 	// if we're non-empty and named.
 	if (named && queue.empty())
 		retain();
+
 	queue.push(var);
 	cond->broadcast();
 
@@ -113,13 +125,12 @@
 	if (!var)
 		return;
 
+	mutex->lock();
 	unsigned long id = push(var);
 
-	mutex->lock();
 	while (!past(id, received))
-	{
 		cond->wait(mutex);
-	}
+
 	mutex->unlock();
 }
 
@@ -139,18 +150,18 @@
 	// if we're empty and named.
 	if (named && queue.empty())
 		release();
+
 	return var;
 } // NOTE: Returns a retained Variant
 
 Variant *Channel::demand()
 {
 	Variant *var;
+	mutex->lock();
 	while (!(var = pop()))
-	{
-		mutex->lock();
 		cond->wait(mutex);
-		mutex->unlock();
-	}
+
+	mutex->unlock();
 	return var;
 }
 
@@ -185,12 +196,26 @@
 		queue.pop();
 	}
 
+	// Finish all the supply waits
+	received = sent;
+	cond->broadcast();
+
 	// Once again, release our own
 	// reference if we're named.
 	if (named)
 		release();
 }
 
+void Channel::lockMutex()
+{
+	mutex->lock();
+}
+
+void Channel::unlockMutex()
+{
+	mutex->unlock();
+}
+
 void Channel::retain()
 {
 	if (named)