From 4113a171bedbb1f89a7cc0a0ad5369048f268e5f Mon Sep 17 00:00:00 2001
From: Zygo Blaxell <bees@furryterror.org>
Date: Mon, 23 Jan 2017 21:12:34 -0500
Subject: [PATCH] crucible: cache: clean up use of iterators

check_overflow() will invalidate iterators if it decides there are too
many cache entries.

If items are deleted from the cache, search for the inserted item again
to ensure the iterator is valid.

Increase size of timestamp to size_t.

Signed-off-by: Zygo Blaxell <bees@furryterror.org>
---
 include/crucible/cache.h | 41 ++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/include/crucible/cache.h b/include/crucible/cache.h
index 60d2a7f..767b790 100644
--- a/include/crucible/cache.h
+++ b/include/crucible/cache.h
@@ -18,7 +18,7 @@ namespace crucible {
 	public:
 		using Key = tuple<Arguments...>;
 		using Func = function<Return(Arguments...)>;
-		using Time = unsigned;
+		using Time = size_t;
 		using Value = pair<Time, Return>;
 	private:
 		Func		m_fn;
@@ -28,7 +28,7 @@ namespace crucible {
 		size_t		m_max_size;
 		mutex		m_mutex;
 
-		void check_overflow();
+		bool check_overflow();
 	public:
 		LRUCache(Func f = Func(), size_t max_size = 100);
 
@@ -52,21 +52,24 @@ namespace crucible {
 	}
 
 	template <class Return, class... Arguments>
-	void
+	bool
 	LRUCache<Return, Arguments...>::check_overflow()
 	{
-		if (m_map.size() <= m_max_size) return;
-		vector<pair<Key, Time>> map_contents;
-		map_contents.reserve(m_map.size());
-		for (auto i : m_map) {
-			map_contents.push_back(make_pair(i.first, i.second.first));
+		if (m_map.size() <= m_max_size) {
+			return false;
 		}
-		sort(map_contents.begin(), map_contents.end(), [](const pair<Key, Time> &a, const pair<Key, Time> &b) {
+		vector<pair<Key, Time>> key_times;
+		key_times.reserve(m_map.size());
+		for (auto i : m_map) {
+			key_times.push_back(make_pair(i.first, i.second.first));
+		}
+		sort(key_times.begin(), key_times.end(), [](const pair<Key, Time> &a, const pair<Key, Time> &b) {
 			return a.second < b.second;
 		});
-		for (size_t i = 0; i < map_contents.size() / 2; ++i) {
-			m_map.erase(map_contents[i].first);
+		for (size_t i = 0; i < key_times.size() / 2; ++i) {
+			m_map.erase(key_times[i].first);
 		}
+		return true;
 	}
 
 	template <class Return, class... Arguments>
@@ -141,9 +144,14 @@ namespace crucible {
 				// We hold a lock on this key so we are the ones to insert it
 				THROW_CHECK0(runtime_error, inserted);
 
-				// Release key lock and clean out overflow
+				// Release key lock, keep the cache lock
 				key_lock.unlock();
-				check_overflow();
+
+				// Check to see if we have too many items and reduce if so.
+				if (check_overflow()) {
+					// Reset iterator
+					found = m_map.find(k);
+				}
 			}
 		}
 
@@ -207,7 +215,12 @@ namespace crucible {
 
 				// Release key lock and clean out overflow
 				key_lock.unlock();
-				check_overflow();
+
+				// Check to see if we have too many items and reduce if so.
+				if (check_overflow()) {
+					// Reset iterator
+					found = m_map.find(k);
+				}
 			}
 		}