Reimplement sw::Resource with modern C++ primitives

src/Common/Resource.hpp

@@ -30,7 +30,7 @@ namespace sw
 	// Resource is a form of shared mutex that guards an internally allocated
 	// buffer. Resource has an exclusive lock mode (sw::Accessor) and lock
 	// count, defaulting to sw::Accessor::PUBLIC and 0, respectively.
-	// Resource does treat any of the sw::Accessor enumerator lock modes
+	// Resource doesn't treat any of the sw::Accessor enumerator lock modes
 	// differently, all semantic meaning comes from the usage of Resource.
 	// You can have multiple locks in mode sw::Accessor::EXCLUSIVE.
 	class Resource

src/System/Resource.cpp

@@ -21,12 +21,6 @@ namespace sw
 {
 	Resource::Resource(size_t bytes) : size(bytes)
 	{
-		blocked = 0;
-
-		accessor = PUBLIC;
-		count = 0;
-		orphaned = false;
-
 		buffer = allocate(bytes);
 	}
 
@@ -37,144 +31,74 @@ namespace sw
 
 	void *Resource::lock(Accessor claimer)
 	{
-		criticalSection.lock();
-
-		while(count > 0 && accessor != claimer)
-		{
-			blocked++;
-			criticalSection.unlock();
-
-			unblock.wait();
-
-			criticalSection.lock();
-			blocked--;
-		}
-
-		accessor = claimer;
-		count++;
-
-		criticalSection.unlock();
-
-		return buffer;
+		std::unique_lock<std::mutex> lock(mutex);
+		return acquire(lock, claimer);
 	}
 
 	void *Resource::lock(Accessor relinquisher, Accessor claimer)
 	{
-		criticalSection.lock();
+		std::unique_lock<std::mutex> lock(mutex);
 
 		// Release
-		while(count > 0 && accessor == relinquisher)
+		if (count > 0 && accessor == relinquisher)
 		{
-			count--;
-
-			if(count == 0)
-			{
-				if(blocked)
-				{
-					unblock.signal();
-				}
-				else if(orphaned)
-				{
-					criticalSection.unlock();
-
-					delete this;
-
-					return 0;
-				}
-			}
+			release(lock);
 		}
 
 		// Acquire
-		while(count > 0 && accessor != claimer)
-		{
-			blocked++;
-			criticalSection.unlock();
+		acquire(lock, claimer);
 
-			unblock.wait();
+		return buffer;
+	}
+
+	void Resource::unlock()
+	{
+		std::unique_lock<std::mutex> lock(mutex);
+		release(lock);
+	}
 
-			criticalSection.lock();
+	void *Resource::acquire(std::unique_lock<std::mutex> &lock, Accessor claimer)
+	{
+		while (count > 0 && accessor != claimer)
+		{
+			blocked++;
+			released.wait(lock, [&] { return count == 0 || accessor == claimer; });
 			blocked--;
 		}
 
 		accessor = claimer;
 		count++;
-
-		criticalSection.unlock();
-
 		return buffer;
 	}
 
-	void Resource::unlock()
+	void Resource::release(std::unique_lock<std::mutex> &lock)
 	{
-		criticalSection.lock();
 		ASSERT(count > 0);
 
 		count--;
 
 		if(count == 0)
 		{
-			if(blocked)
-			{
-				unblock.signal();
-			}
-			else if(orphaned)
+			if(orphaned)
 			{
-				criticalSection.unlock();
-
+				lock.unlock();
 				delete this;
-
 				return;
 			}
+			released.notify_one();
 		}
-
-		criticalSection.unlock();
-	}
-
-	void Resource::unlock(Accessor relinquisher)
-	{
-		criticalSection.lock();
-		ASSERT(count > 0);
-
-		while(count > 0 && accessor == relinquisher)
-		{
-			count--;
-
-			if(count == 0)
-			{
-				if(blocked)
-				{
-					unblock.signal();
-				}
-				else if(orphaned)
-				{
-					criticalSection.unlock();
-
-					delete this;
-
-					return;
-				}
-			}
-		}
-
-		criticalSection.unlock();
 	}
 
 	void Resource::destruct()
 	{
-		criticalSection.lock();
-
-		if(count == 0 && !blocked)
+		std::unique_lock<std::mutex> lock(mutex);
+		if(count == 0 && blocked == 0)
 		{
-			criticalSection.unlock();
-
+			lock.unlock();
 			delete this;
-
 			return;
 		}
-
 		orphaned = true;
-
-		criticalSection.unlock();
 	}
 
 	const void *Resource::data() const

src/System/Resource.hpp

@@ -15,8 +15,7 @@
 #ifndef sw_Resource_hpp
 #define sw_Resource_hpp
 
-#include "Synchronization.hpp"
-
+#include <condition_variable>
 #include <mutex>
 
 namespace sw
@@ -29,32 +28,78 @@ namespace sw
 		EXCLUSIVE
 	};
 
+	// Resource is a form of shared mutex that guards an internally allocated
+	// buffer. Resource has an exclusive lock mode (sw::Accessor) and lock
+	// count, defaulting to sw::Accessor::PUBLIC and 0, respectively.
+	// Resource doesn't treat any of the sw::Accessor enumerator lock modes
+	// differently, all semantic meaning comes from the usage of Resource.
+	// You can have multiple locks in mode sw::Accessor::EXCLUSIVE.
 	class Resource
 	{
 	public:
 		Resource(size_t bytes);
 
-		void destruct();   // Asynchronous destructor
+		// destruct() is an asynchronous destructor, that will atomically:
+		//   When the resource is unlocked:
+		//     * Delete itself.
+		//   When the resource is locked:
+		//     * Flag itself for deletion when next fully unlocked.
+		void destruct();
 
+		// lock() will atomically:
+		//   When the resource is unlocked OR the lock mode equals claimer:
+		//     * Increment the lock count.
+		//     * Return a pointer to the buffer.
+		//   When the resource is locked AND the lock mode does not equal claimer:
+		//     * Block until all existing locks are released (lock count equals 0).
+		//     * Switch lock mode to claimer.
+		//     * Increment the lock count.
+		//     * Return a pointer to the buffer.
 		void *lock(Accessor claimer);
+
+		// lock() will atomically:
+		//   When the resource is unlocked OR the lock mode equals claimer:
+		//     * Increment the lock count.
+		//     * Return a pointer to the buffer.
+		//   When the resource is locked AND the lock mode equals relinquisher:
+		//     * Release *all* existing locks (regardless of prior count).
+		//     * Delete itself and return nullptr if Resource::destruct() had been called while locked.
+		//     * Switch lock mode to claimer.
+		//     * Increment the lock count to 1.
+		//     * Return a pointer to the buffer.
+		//   When the resource is locked AND the lock mode does not equal relinquisher:
+		//     * Block until all existing locks are released (lock count equals 0)
+		//     * Switch lock mode to claimer
+		//     * Increment the lock count to 1.
+		//     * Return a pointer to the buffer.
 		void *lock(Accessor relinquisher, Accessor claimer);
+
+		// unlock() will atomically:
+		// * Assert if there are no locks.
+		// * Release a single lock.
+		// * Delete itself if Resource::destruct() had been called while locked.
 		void unlock();
-		void unlock(Accessor relinquisher);
 
+		// data() will return the Resource's buffer pointer regardless of lock
+		// state.
 		const void *data() const;
+
+		// size is the size in bytes of the Resource's buffer.
 		const size_t size;
 
 	private:
-		~Resource();   // Always call destruct() instead
+		void *acquire(std::unique_lock<std::mutex> &lock, Accessor claimer);
+		void release(std::unique_lock<std::mutex> &lock);
 
-		std::mutex criticalSection;
-		Event unblock;
-		volatile int blocked;
+		~Resource();   // Always call destruct() instead
 
-		volatile Accessor accessor;
-		volatile int count;
-		bool orphaned;
+		std::mutex mutex;
+		std::condition_variable released;
 
+		Accessor accessor = PUBLIC; // guarded by mutex
+		int blocked = 0; // guarded by mutex
+		int count = 0; // guarded by mutex
+		bool orphaned = false; // guarded by mutex
 		void *buffer;
 	};
 }