From b6da65b9e79063693ae831ffe8cf35ccd3d8549e Mon Sep 17 00:00:00 2001
From: florijan <florijan@memgraph.io>
Date: Wed, 14 Jun 2017 13:15:48 +0200
Subject: [PATCH] Skiplist - GC - race condition fix
Summary: An unpleasant race-condition detected. Solution proposed. It's not very pretty. Perhaps consider using the ConcurrentPushQueue. Not 100% sure, but it should make the GC code easier to work with.
Reviewers: buda, mislav.bradac, teon.banek, dgleich
Reviewed By: buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D469
---
src/data_structures/concurrent/skiplist_gc.hpp | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/data_structures/concurrent/skiplist_gc.hpp b/src/data_structures/concurrent/skiplist_gc.hpp
index 014db3901..a23492a79 100644
--- a/src/data_structures/concurrent/skiplist_gc.hpp
+++ b/src/data_structures/concurrent/skiplist_gc.hpp
@@ -110,12 +110,19 @@ class SkipListGC : public Loggable {
// We can only modify this in a not-thread safe way because we are the only
// thread ever accessing it here, i.e. there is at most one thread doing
// this GarbageCollection.
+
+ // find the oldest not deletable record
+ // since we can't copy a concurrent list iterator, we must use two
+ // separate ones, while ensuring they point to the same record
+ // in the beginning of the search-loop
+ auto it = deleted_list_.begin();
auto oldest_not_deletable = deleted_list_.begin();
+ while (oldest_not_deletable != it) oldest_not_deletable++;
+ // we need a bool to track if oldest_not_deletable should get
+ // deleted too (we have not skipped any record due to safe_id condition)
bool delete_all = true;
- for (auto it = deleted_list_.begin(); it != deleted_list_.end(); ++it) {
+ for (; it != deleted_list_.end(); ++it) {
if (it->first > safe_id) {
- // We have to increase iterator manually because the copy assignment
- // operator is deleted.
while (oldest_not_deletable != it) ++oldest_not_deletable;
delete_all = false;
}