From 5177995530557521d330486a3971469e1573d6fc Mon Sep 17 00:00:00 2001
From: Jason Evans <jasone@canonware.com>
Date: Sun, 5 Feb 2017 23:59:53 -0800
Subject: [PATCH] Fix extent_record().

Read adjacent rtree elements while holding element locks, since the
extents mutex only protects against relevant like-state extent mutation.

Fix management of the 'coalesced' loop state variable to merge
forward/backward results, rather than overwriting the result of forward
coalescing if attempting to coalesce backward.  In practice this caused
no correctness issues, but could cause extra iterations in rare cases.

These regressions were introduced by
d27f29b468ae3e9d2b1da4a9880351d76e5a1662 (Disentangle arena and extent
locking.).
---
 src/extent.c | 51 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/src/extent.c b/src/extent.c
index 234be54b..4a83f694 100644
--- a/src/extent.c
+++ b/src/extent.c
@@ -1035,12 +1035,9 @@ extent_can_coalesce(const extent_t *a, const extent_t *b) {
 }
 
 static bool
-extent_try_coalesce(tsdn_t *tsdn, arena_t *arena,
-    extent_hooks_t **r_extent_hooks, extent_t *a, extent_t *b,
-    extents_t *extents) {
-	if (!extent_can_coalesce(a, b)) {
-		return true;
-	}
+extent_coalesce(tsdn_t *tsdn, arena_t *arena, extent_hooks_t **r_extent_hooks,
+    extent_t *a, extent_t *b, extents_t *extents) {
+	assert(extent_can_coalesce(a, b));
 	assert(extent_arena_get(a) == arena);
 	assert(extent_arena_get(b) == arena);
 
@@ -1062,7 +1059,6 @@ extent_try_coalesce(tsdn_t *tsdn, arena_t *arena,
 static void
 extent_record(tsdn_t *tsdn, arena_t *arena, extent_hooks_t **r_extent_hooks,
     extents_t *extents, extent_t *extent) {
-	extent_t *prev, *next;
 	rtree_ctx_t rtree_ctx_fallback;
 	rtree_ctx_t *rtree_ctx = tsdn_rtree_ctx(tsdn, &rtree_ctx_fallback);
 
@@ -1090,21 +1086,40 @@ extent_record(tsdn_t *tsdn, arena_t *arena, extent_hooks_t **r_extent_hooks,
 		coalesced = false;
 
 		/* Try to coalesce forward. */
-		next = rtree_read(tsdn, &extents_rtree, rtree_ctx,
-		    (uintptr_t)extent_past_get(extent), false);
-		if (next != NULL) {
-			coalesced = !extent_try_coalesce(tsdn, arena,
-			    r_extent_hooks, extent, next, extents);
+		rtree_elm_t *next_elm = rtree_elm_acquire(tsdn, &extents_rtree,
+		    rtree_ctx, (uintptr_t)extent_past_get(extent), false,
+		    false);
+		if (next_elm != NULL) {
+			extent_t *next = rtree_elm_read_acquired(tsdn,
+			    &extents_rtree, next_elm);
+			/*
+			 * extents->mtx only protects against races for
+			 * like-state extents, so call extent_can_coalesce()
+			 * before releasing the next_elm lock.
+			 */
+			bool can_coalesce = (next != NULL &&
+			    extent_can_coalesce(extent, next));
+			rtree_elm_release(tsdn, &extents_rtree, next_elm);
+			if (can_coalesce && !extent_coalesce(tsdn, arena,
+			    r_extent_hooks, extent, next, extents)) {
+				coalesced = true;
+			}
 		}
 
 		/* Try to coalesce backward. */
-		prev = rtree_read(tsdn, &extents_rtree, rtree_ctx,
-		    (uintptr_t)extent_before_get(extent), false);
-		if (prev != NULL) {
-			coalesced = !extent_try_coalesce(tsdn, arena,
-			    r_extent_hooks, prev, extent, extents);
-			if (coalesced) {
+		rtree_elm_t *prev_elm = rtree_elm_acquire(tsdn, &extents_rtree,
+		    rtree_ctx, (uintptr_t)extent_before_get(extent), false,
+		    false);
+		if (prev_elm != NULL) {
+			extent_t *prev = rtree_elm_read_acquired(tsdn,
+			    &extents_rtree, prev_elm);
+			bool can_coalesce = (prev != NULL &&
+			    extent_can_coalesce(prev, extent));
+			rtree_elm_release(tsdn, &extents_rtree, prev_elm);
+			if (can_coalesce && !extent_coalesce(tsdn, arena,
+			    r_extent_hooks, prev, extent, extents)) {
 				extent = prev;
+				coalesced = true;
 			}
 		}
 	} while (coalesced);