From 6099bf0b015bb014b2c147443480f5e1af8036a7 Mon Sep 17 00:00:00 2001
From: Zygo Blaxell <bees@furryterror.org>
Date: Sun, 22 Jan 2017 21:50:21 -0500
Subject: [PATCH] crucible: fix further instances of copy-after-unlock bug

Before:

	unique_lock<mutex> lock(some_mutex);
	// run lock.~unique_lock() because return
	// return reference to unprotected heap
	return foo[bar];

After:

	unique_lock<mutex> lock(some_mutex);
	// make copy of object on heap protected by mutex lock
	auto tmp_copy = foo[bar];
	// run lock.~unique_lock() because return
	// pass locally allocated object to copy constructor
	return tmp_copy;

Signed-off-by: Zygo Blaxell <bees@furryterror.org>
---
 include/crucible/lockset.h   | 5 ++++-
 include/crucible/resource.h  | 8 ++++++--
 include/crucible/workqueue.h | 7 +++++--
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/include/crucible/lockset.h b/include/crucible/lockset.h
index dcf82f2..2af782d 100644
--- a/include/crucible/lockset.h
+++ b/include/crucible/lockset.h
@@ -161,7 +161,10 @@ namespace crucible {
 	LockSet<T>::copy()
 	{
 		unique_lock<mutex> lock(m_mutex);
-		return m_set;
+		// Make temporary copy of set while protected by mutex
+		auto rv = m_set;
+		// Return temporary copy after releasing lock
+		return rv;
 	}
 
 	template <class T>
diff --git a/include/crucible/resource.h b/include/crucible/resource.h
index 1f93004..cc49bea 100644
--- a/include/crucible/resource.h
+++ b/include/crucible/resource.h
@@ -332,7 +332,9 @@ namespace crucible {
 	ResourceHandle<Key, Resource>::get_resource_ptr() const
 	{
 		unique_lock<mutex> lock(s_ptr_mutex);
-		return m_ptr;
+		// Make isolated copy of pointer with lock held, and return the copy
+		auto rv = m_ptr;
+		return rv;
 	}
 
 	template <class Key, class Resource>
@@ -343,7 +345,9 @@ namespace crucible {
 		if (!m_ptr) {
 			THROW_ERROR(out_of_range, __PRETTY_FUNCTION__ << " called on null Resource");
 		}
-		return m_ptr;
+		// Make isolated copy of pointer with lock held, and return the copy
+		auto rv = m_ptr;
+		return rv;
 	}
 
 	template <class Key, class Resource>
diff --git a/include/crucible/workqueue.h b/include/crucible/workqueue.h
index 2ba13ec..bf3732f 100644
--- a/include/crucible/workqueue.h
+++ b/include/crucible/workqueue.h
@@ -124,7 +124,9 @@ namespace crucible {
 		if (m_set.empty()) {
 			return key_type();
 		} else {
-			return *m_set.begin();
+			// Make copy with lock held
+			auto rv = *m_set.begin();
+			return rv;
 		}
 	}
 
@@ -149,7 +151,8 @@ namespace crucible {
 	WorkQueue<Task>::copy()
 	{
 		unique_lock<mutex> lock(m_mutex);
-		return m_set;
+		auto rv = m_set;
+		return rv;
 	}
 
 	template <class Task>