WT-2733 Backport fixes for races between eviction and dead handle cleanup (#2841)

* WT-2313 Wrap clearing of WT_DHANDLE_OPEN with eviction exclusive calls.

(cherry picked from commit f4cc13a6dffda784be1fa33f8b46fefc5afab13e)

* WT-2434 Fix a race between forced drops and sweep.

(cherry picked from commit f6d0fa3645eb3f2b9932ffac9c287bba52052e04)

* Remove a stray line that was added during backport.

3 files changed, 60 insertions, 56 deletions

diff --git a/src/conn/conn_dhandle.c b/src/conn/conn_dhandle.c
index fed0012b211..1bb8e37249f 100644
--- a/src/conn/conn_dhandle.c
+++ b/src/conn/conn_dhandle.c
@@ -247,30 +247,6 @@ err:	WT_TRET(__wt_rwlock_destroy(session, &dhandle->rwlock));
 }
 
 /*
- * __conn_dhandle_mark_dead --
- *	Mark a data handle dead.
- */
-static int
-__conn_dhandle_mark_dead(WT_SESSION_IMPL *session)
-{
-	bool evict_reset;
-
-	WT_ASSERT(session, F_ISSET(session, WT_SESSION_LOCKED_HANDLE_LIST));
-
-	/*
-	 * Handle forced discard (e.g., when dropping a file).
-	 *
-	 * We need exclusive access to the file -- disable ordinary
-	 * eviction and drain any blocks already queued.
-	 */
-	WT_RET(__wt_evict_file_exclusive_on(session, &evict_reset));
-	F_SET(session->dhandle, WT_DHANDLE_DEAD);
-	if (evict_reset)
-		__wt_evict_file_exclusive_off(session);
-	return (0);
-}
-
-/*
  * __wt_conn_btree_sync_and_close --
  *	Sync and close the underlying btree handle.
  */
@@ -280,10 +256,11 @@ __wt_conn_btree_sync_and_close(WT_SESSION_IMPL *session, bool final, bool force)
 	WT_BTREE *btree;
 	WT_DATA_HANDLE *dhandle;
 	WT_DECL_RET;
-	bool no_schema_lock;
+	bool evict_reset, marked_dead, no_schema_lock;
 
 	dhandle = session->dhandle;
 	btree = S2BT(session);
+	evict_reset = marked_dead = false;
 
 	if (!F_ISSET(dhandle, WT_DHANDLE_OPEN))
 		return (0);
@@ -309,6 +286,13 @@ __wt_conn_btree_sync_and_close(WT_SESSION_IMPL *session, bool final, bool force)
 	__wt_spin_lock(session, &dhandle->close_lock);
 
 	/*
+	 * Ensure we aren't racing with the eviction server; inside the close
+	 * lock so threads won't race setting/clearing the tree's "no eviction"
+	 * flag.
+	 */
+	WT_ERR(__wt_evict_file_exclusive_on(session, &evict_reset));
+
+	/*
 	 * The close can fail if an update cannot be written, return the EBUSY
 	 * error to our caller for eventual retry.
 	 *
@@ -319,20 +303,31 @@ __wt_conn_btree_sync_and_close(WT_SESSION_IMPL *session, bool final, bool force)
 	 * invalid if the mapping is closed.
 	 */
 	if (!F_ISSET(btree,
-	    WT_BTREE_SALVAGE | WT_BTREE_UPGRADE | WT_BTREE_VERIFY))
-		WT_ERR(force && (btree->bm == NULL || btree->bm->map == NULL) ?
-		    __conn_dhandle_mark_dead(session) :
-		    __wt_checkpoint_close(session, final));
+	    WT_BTREE_SALVAGE | WT_BTREE_UPGRADE | WT_BTREE_VERIFY)) {
+		if (force && (btree->bm == NULL || btree->bm->map == NULL)) {
+			F_SET(session->dhandle, WT_DHANDLE_DEAD);
+			marked_dead = true;
+		}
+		if (!marked_dead || final)
+			WT_ERR(__wt_checkpoint_close(session, final));
+	}
 
 	WT_TRET(__wt_btree_close(session));
-	if (!force || final) {
+
+	/*
+	 * If we marked a handle dead it will be closed by sweep, via
+	 * another call to sync and close.
+	 */
+	if (!marked_dead) {
 		F_CLR(dhandle, WT_DHANDLE_OPEN);
 		if (dhandle->checkpoint == NULL)
 			--S2C(session)->open_btree_count;
 	}
 	F_CLR(btree, WT_BTREE_SPECIAL_FLAGS);
 
-err:	__wt_spin_unlock(session, &dhandle->close_lock);
+err:	if (evict_reset)
+		__wt_evict_file_exclusive_off(session);
+	__wt_spin_unlock(session, &dhandle->close_lock);
 
 	if (no_schema_lock)
 		F_CLR(session, WT_SESSION_NO_SCHEMA_LOCK);
diff --git a/src/conn/conn_sweep.c b/src/conn/conn_sweep.c
index 8854f5e0592..a89b2f5ca6f 100644
--- a/src/conn/conn_sweep.c
+++ b/src/conn/conn_sweep.c
@@ -64,11 +64,9 @@ __sweep_expire_one(WT_SESSION_IMPL *session)
 	WT_BTREE *btree;
 	WT_DATA_HANDLE *dhandle;
 	WT_DECL_RET;
-	bool evict_reset;
 
 	btree = S2BT(session);
 	dhandle = session->dhandle;
-	evict_reset = false;
 
 	/*
 	 * Acquire an exclusive lock on the handle and mark it dead.
@@ -92,19 +90,13 @@ __sweep_expire_one(WT_SESSION_IMPL *session)
 	    !__wt_txn_visible_all(session, btree->rec_max_txn))
 		goto err;
 
-	/* Ensure that we aren't racing with the eviction server */
-	WT_ERR(__wt_evict_file_exclusive_on(session, &evict_reset));
-
 	/*
-	 * Mark the handle as dead and close the underlying file
-	 * handle. Closing the handle decrements the open file count,
-	 * meaning the close loop won't overrun the configured minimum.
+	 * Mark the handle dead and close the underlying file handle.
+	 * Closing the handle decrements the open file count, meaning the close
+	 * loop won't overrun the configured minimum.
 	 */
 	ret = __wt_conn_btree_sync_and_close(session, false, true);
 
-	if (evict_reset)
-		__wt_evict_file_exclusive_off(session);
-
 err:	WT_TRET(__wt_writeunlock(session, dhandle->rwlock));
 
 	return (ret);
@@ -171,7 +163,7 @@ __sweep_discard_trees(WT_SESSION_IMPL *session, u_int *dead_handlesp)
 		    !F_ISSET(dhandle, WT_DHANDLE_DEAD))
 			continue;
 
-		/* If the handle is marked "dead", flush it from cache. */
+		/* If the handle is marked dead, flush it from cache. */
 		WT_WITH_DHANDLE(session, dhandle, ret =
 		    __wt_conn_btree_sync_and_close(session, false, false));
 
diff --git a/src/evict/evict_lru.c b/src/evict/evict_lru.c
index 47917289503..dc64a55091d 100644
--- a/src/evict/evict_lru.c
+++ b/src/evict/evict_lru.c
@@ -729,31 +729,35 @@ __wt_evict_file_exclusive_on(WT_SESSION_IMPL *session, bool *evict_resetp)
 {
 	WT_BTREE *btree;
 	WT_CACHE *cache;
+	WT_DECL_RET;
 	WT_EVICT_ENTRY *evict;
 	u_int i, elem;
 
 	btree = S2BT(session);
 	cache = S2C(session)->cache;
 
-	/*
-	 * If the file isn't evictable, there's no work to do.
-	 */
-	if (F_ISSET(btree, WT_BTREE_NO_EVICTION)) {
-		*evict_resetp = false;
+	*evict_resetp = false;
+	/* If the file was never evictable, there's no work to do. */
+	if (F_ISSET(btree, WT_BTREE_NO_EVICTION))
 		return (0);
-	}
-	*evict_resetp = true;
 
 	/*
 	 * Hold the walk lock to set the "no eviction" flag: no new pages from
 	 * the file will be queued for eviction after this point.
 	 */
 	__wt_spin_lock(session, &cache->evict_walk_lock);
-	F_SET(btree, WT_BTREE_NO_EVICTION);
+	if (!F_ISSET(btree, WT_BTREE_NO_EVICTION)) {
+		F_SET(btree, WT_BTREE_NO_EVICTION);
+		*evict_resetp = true;
+	}
 	__wt_spin_unlock(session, &cache->evict_walk_lock);
 
+	/* If some other operation has disabled eviction, we're done. */
+	if (!*evict_resetp)
+		return (0);
+
 	/* Clear any existing LRU eviction walk for the file. */
-	WT_RET(__evict_request_walk_clear(session));
+	WT_ERR(__evict_request_walk_clear(session));
 
 	/* Hold the evict lock to remove any queued pages from this file. */
 	__wt_spin_lock(session, &cache->evict_lock);
@@ -776,6 +780,10 @@ __wt_evict_file_exclusive_on(WT_SESSION_IMPL *session, bool *evict_resetp)
 		__wt_yield();
 
 	return (0);
+
+err:	F_CLR(btree, WT_BTREE_NO_EVICTION);
+	*evict_resetp = false;
+	return (ret);
 }
 
 /*
@@ -789,7 +797,14 @@ __wt_evict_file_exclusive_off(WT_SESSION_IMPL *session)
 
 	btree = S2BT(session);
 
-	WT_ASSERT(session, btree->evict_ref == NULL);
+	/*
+	 * We have seen subtle bugs with multiple threads racing to turn
+	 * eviction on/off.  Make races more likely in diagnostic builds.
+	 */
+	WT_DIAGNOSTIC_YIELD;
+
+	WT_ASSERT(session, btree->evict_ref == NULL &&
+	    F_ISSET(btree, WT_BTREE_NO_EVICTION));
 
 	F_CLR(btree, WT_BTREE_NO_EVICTION);
 }
@@ -952,6 +967,7 @@ __evict_walk(WT_SESSION_IMPL *session, uint32_t flags)
 
 	conn = S2C(session);
 	cache = S2C(session)->cache;
+	btree = NULL;
 	dhandle = NULL;
 	dhandle_locked = incr = false;
 	retries = 0;
@@ -1011,6 +1027,7 @@ retry:	while (slot < max_entries && ret == 0) {
 				(void)__wt_atomic_subi32(
 				    &dhandle->session_inuse, 1);
 				incr = false;
+				cache->evict_file_next = NULL;
 			}
 			dhandle = TAILQ_NEXT(dhandle, q);
 		}
@@ -1065,6 +1082,9 @@ retry:	while (slot < max_entries && ret == 0) {
 		 * exclusive access when a handle is being closed.
 		 */
 		if (!F_ISSET(btree, WT_BTREE_NO_EVICTION)) {
+			/* Remember the file to visit first, next loop. */
+			cache->evict_file_next = dhandle;
+
 			WT_WITH_DHANDLE(session, dhandle,
 			    ret = __evict_walk_file(session, &slot, flags));
 			WT_ASSERT(session, session->split_gen == 0);
@@ -1084,9 +1104,6 @@ retry:	while (slot < max_entries && ret == 0) {
 	}
 
 	if (incr) {
-		/* Remember the file we should visit first, next loop. */
-		cache->evict_file_next = dhandle;
-
 		WT_ASSERT(session, dhandle->session_inuse > 0);
 		(void)__wt_atomic_subi32(&dhandle->session_inuse, 1);
 		incr = false;