WT-2378 Always wait for the handle list lock.

That is, the "lock_wait=false" configuration to WT_SESSION::drop does
not apply to the handle list lock.  The reason is code (specifically in
LSM) that acquires the handle list lock to get an LSM tree, does some
work, then acquires the handle list lock again when discarding the tree.
If the first acquire happens to succeed and the second acquire fails,
the tree is left in the system with its reference count bumped.

14 files changed, 53 insertions, 38 deletions

diff --git a/src/conn/conn_dhandle.c b/src/conn/conn_dhandle.c
index a3cb26afb27..60136a71b99 100644
--- a/src/conn/conn_dhandle.c
+++ b/src/conn/conn_dhandle.c
@@ -644,7 +644,7 @@ __wt_conn_dhandle_discard_single(
 		F_SET(S2C(session)->cache, WT_CACHE_CLEAR_WALKS);
 
 	/* Try to remove the handle, protected by the data handle lock. */
-	WT_WITH_HANDLE_LIST_LOCK(session, tret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    tret = __conn_dhandle_remove(session, final));
 	WT_TRET(tret);
 
diff --git a/src/conn/conn_stat.c b/src/conn/conn_stat.c
index 9edc6091b10..08ad105c725 100644
--- a/src/conn/conn_stat.c
+++ b/src/conn/conn_stat.c
@@ -340,7 +340,7 @@ __statlog_log_one(WT_SESSION_IMPL *session, WT_ITEM *path, WT_ITEM *tmp)
 	 * any that match the list of object sources.
 	 */
 	if (conn->stat_sources != NULL) {
-		WT_WITH_HANDLE_LIST_LOCK(session, ret,
+		WT_WITH_HANDLE_LIST_LOCK(session,
 		    ret = __wt_conn_btree_apply(
 		    session, false, NULL, __statlog_apply, NULL));
 		WT_RET(ret);
diff --git a/src/conn/conn_sweep.c b/src/conn/conn_sweep.c
index 11fe180d7a4..7628076e605 100644
--- a/src/conn/conn_sweep.c
+++ b/src/conn/conn_sweep.c
@@ -235,7 +235,7 @@ __sweep_remove_handles(WT_SESSION_IMPL *session)
 		if (!WT_DHANDLE_CAN_DISCARD(dhandle))
 			continue;
 
-		WT_WITH_HANDLE_LIST_LOCK(session, ret,
+		WT_WITH_HANDLE_LIST_LOCK(session,
 		    ret = __sweep_remove_one(session, dhandle));
 		if (ret == 0)
 			WT_STAT_FAST_CONN_INCR(session, dh_sweep_remove);
diff --git a/src/cursor/cur_backup.c b/src/cursor/cur_backup.c
index 6d5d68000ee..d7d74da48d4 100644
--- a/src/cursor/cur_backup.c
+++ b/src/cursor/cur_backup.c
@@ -377,7 +377,7 @@ __backup_all(WT_SESSION_IMPL *session, WT_CURSOR_BACKUP *cb)
 	WT_ERR(__wt_metadata_cursor_release(session, &cursor));
 
 	/* Build a list of the file objects that need to be copied. */
-	WT_WITH_HANDLE_LIST_LOCK(session, ret, ret =
+	WT_WITH_HANDLE_LIST_LOCK(session, ret =
 	    __wt_meta_btree_apply(session, __backup_list_all_append, NULL));
 
 err:	WT_TRET(__wt_metadata_cursor_release(session, &cursor));
diff --git a/src/include/schema.h b/src/include/schema.h
index 88a3a39f8b3..c594aa928ab 100644
--- a/src/include/schema.h
+++ b/src/include/schema.h
@@ -79,27 +79,37 @@ struct __wt_table {
 #define	WT_COLGROUPS(t)	WT_MAX((t)->ncolgroups, 1)
 
 /*
+ * WT_WITH_LOCK_WAIT --
+ *	Wait for a lock, perform an operation, drop the lock.
+ */
+#define	WT_WITH_LOCK_WAIT(session, lock, flag, op) do {			\
+	if (F_ISSET(session, (flag))) {					\
+		op;							\
+	}  else {							\
+		__wt_spin_lock(session, (lock));			\
+		F_SET(session, (flag));					\
+		op;							\
+		F_CLR(session, (flag));					\
+		__wt_spin_unlock(session, (lock));			\
+	}								\
+} while (0)
+
+/*
  * WT_WITH_LOCK --
  *	Acquire a lock, perform an operation, drop the lock.
  */
 #define	WT_WITH_LOCK(session, ret, lock, flag, op) do {			\
 	ret = 0;							\
-	if (F_ISSET(session, (flag))) {					\
-		op;							\
-	} else if (F_ISSET(session, WT_SESSION_LOCK_NO_WAIT)) {		\
+	if (!F_ISSET(session, (flag)) &&				\
+	    F_ISSET(session, WT_SESSION_LOCK_NO_WAIT)) {		\
 		if ((ret = __wt_spin_trylock(session, (lock))) == 0) {	\
 			F_SET(session, (flag));				\
 			op;						\
 			F_CLR(session, (flag));				\
 			__wt_spin_unlock(session, (lock));		\
 		}							\
-	} else {							\
-		__wt_spin_lock(session, (lock));			\
-		F_SET(session, (flag));					\
-		op;							\
-		F_CLR(session, (flag));					\
-		__wt_spin_unlock(session, (lock));			\
-	}								\
+	} else								\
+		WT_WITH_LOCK_WAIT(session, lock, flag, op);			\
 } while (0)
 
 /*
@@ -113,10 +123,15 @@ struct __wt_table {
 /*
  * WT_WITH_HANDLE_LIST_LOCK --
  *	Acquire the data handle list lock, perform an operation, drop the lock.
+ *
+ *	Note: always waits because some operations need the handle list lock to
+ *	discard handles, and we only expect it to be held across short
+ *	operations.
  */
-#define	WT_WITH_HANDLE_LIST_LOCK(session, ret, op)			\
-	WT_WITH_LOCK(session, ret,					\
+#define	WT_WITH_HANDLE_LIST_LOCK(session, op)				\
+	WT_WITH_LOCK_WAIT(session, 					\
 	    &S2C(session)->dhandle_lock, WT_SESSION_LOCKED_HANDLE_LIST, op)
+
 /*
  * WT_WITH_SCHEMA_LOCK --
  *	Acquire the schema lock, perform an operation, drop the lock.
diff --git a/src/lsm/lsm_cursor.c b/src/lsm/lsm_cursor.c
index 1bb9a7238fe..f76b2bfd9ac 100644
--- a/src/lsm/lsm_cursor.c
+++ b/src/lsm/lsm_cursor.c
@@ -1543,7 +1543,7 @@ __wt_clsm_open(WT_SESSION_IMPL *session,
 	bulk = cval.val != 0;
 
 	/* Get the LSM tree. */
-	WT_WITH_HANDLE_LIST_LOCK(session, ret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    ret = __wt_lsm_tree_get(session, uri, bulk, &lsm_tree));
 	/*
 	 * Check whether the exclusive open for a bulk load succeeded, and
diff --git a/src/lsm/lsm_stat.c b/src/lsm/lsm_stat.c
index c147cf5774a..76e2ca6185e 100644
--- a/src/lsm/lsm_stat.c
+++ b/src/lsm/lsm_stat.c
@@ -33,7 +33,7 @@ __curstat_lsm_init(
 	   "checkpoint=" WT_CHECKPOINT, NULL, NULL };
 
 	locked = false;
-	WT_WITH_HANDLE_LIST_LOCK(session, ret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    ret = __wt_lsm_tree_get(session, uri, false, &lsm_tree));
 	WT_RET(ret);
 	WT_ERR(__wt_scr_alloc(session, 0, &uribuf));
diff --git a/src/lsm/lsm_tree.c b/src/lsm/lsm_tree.c
index ff6e66fd1a1..ab18e41a2f5 100644
--- a/src/lsm/lsm_tree.c
+++ b/src/lsm/lsm_tree.c
@@ -315,7 +315,7 @@ __wt_lsm_tree_create(WT_SESSION_IMPL *session,
 	char *tmpconfig;
 
 	/* If the tree is open, it already exists. */
-	WT_WITH_HANDLE_LIST_LOCK(session, ret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    ret = __wt_lsm_tree_get(session, uri, false, &lsm_tree));
 	if (ret == 0) {
 		__wt_lsm_tree_release(session, lsm_tree);
@@ -447,7 +447,7 @@ __wt_lsm_tree_create(WT_SESSION_IMPL *session,
 	 * tracking macros handle cleaning up on failure.
 	 */
 	if (ret == 0)
-		WT_WITH_HANDLE_LIST_LOCK(session, ret,
+		WT_WITH_HANDLE_LIST_LOCK(session,
 		    ret = __lsm_tree_open(session, uri, true, &lsm_tree));
 	if (ret == 0)
 		__wt_lsm_tree_release(session, lsm_tree);
@@ -961,7 +961,7 @@ __wt_lsm_tree_drop(
 	locked = false;
 
 	/* Get the LSM tree. */
-	WT_WITH_HANDLE_LIST_LOCK(session, ret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    ret = __wt_lsm_tree_get(session, name, true, &lsm_tree));
 	WT_RET(ret);
 
@@ -997,7 +997,7 @@ __wt_lsm_tree_drop(
 
 err:	if (locked)
 		WT_TRET(__wt_lsm_tree_writeunlock(session, lsm_tree));
-	WT_WITH_HANDLE_LIST_LOCK(session, tret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    tret = __lsm_tree_discard(session, lsm_tree, false));
 	WT_TRET(tret);
 	return (ret);
@@ -1023,7 +1023,7 @@ __wt_lsm_tree_rename(WT_SESSION_IMPL *session,
 	locked = false;
 
 	/* Get the LSM tree. */
-	WT_WITH_HANDLE_LIST_LOCK(session, ret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    ret = __wt_lsm_tree_get(session, olduri, true, &lsm_tree));
 	WT_RET(ret);
 
@@ -1073,7 +1073,7 @@ err:	if (locked)
 	 * Discard this LSM tree structure. The first operation on the renamed
 	 * tree will create a new one.
 	 */
-	WT_WITH_HANDLE_LIST_LOCK(session, tret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    tret = __lsm_tree_discard(session, lsm_tree, false));
 	WT_TRET(tret);
 	return (ret);
@@ -1098,7 +1098,7 @@ __wt_lsm_tree_truncate(
 	locked = false;
 
 	/* Get the LSM tree. */
-	WT_WITH_HANDLE_LIST_LOCK(session, ret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    ret = __wt_lsm_tree_get(session, name, true, &lsm_tree));
 	WT_RET(ret);
 
@@ -1137,7 +1137,7 @@ err:	if (locked)
 		 * the last good version of the metadata will be used, resulting
 		 * in a valid (not truncated) tree.
 		 */
-		WT_WITH_HANDLE_LIST_LOCK(session, tret,
+		WT_WITH_HANDLE_LIST_LOCK(session,
 		    tret = __lsm_tree_discard(session, lsm_tree, false));
 		WT_TRET(tret);
 	}
@@ -1237,7 +1237,7 @@ __wt_lsm_compact(WT_SESSION_IMPL *session, const char *name, bool *skipp)
 	/* Tell __wt_schema_worker not to look inside the LSM tree. */
 	*skipp = true;
 
-	WT_WITH_HANDLE_LIST_LOCK(session, ret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    ret = __wt_lsm_tree_get(session, name, false, &lsm_tree));
 	WT_RET(ret);
 
@@ -1435,7 +1435,7 @@ __wt_lsm_tree_worker(WT_SESSION_IMPL *session,
 
 	locked = false;
 	exclusive = FLD_ISSET(open_flags, WT_DHANDLE_EXCLUSIVE);
-	WT_WITH_HANDLE_LIST_LOCK(session, ret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    ret = __wt_lsm_tree_get(session, uri, exclusive, &lsm_tree));
 	WT_RET(ret);
 
diff --git a/src/lsm/lsm_work_unit.c b/src/lsm/lsm_work_unit.c
index 4faa25967ad..d5d81df6785 100644
--- a/src/lsm/lsm_work_unit.c
+++ b/src/lsm/lsm_work_unit.c
@@ -272,7 +272,7 @@ __wt_lsm_checkpoint_chunk(WT_SESSION_IMPL *session,
 	if (F_ISSET(chunk, WT_LSM_CHUNK_ONDISK) &&
 	    !F_ISSET(chunk, WT_LSM_CHUNK_STABLE) &&
 	    !chunk->evicted) {
-		WT_WITH_HANDLE_LIST_LOCK(session, ret,
+		WT_WITH_HANDLE_LIST_LOCK(session,
 		    ret = __lsm_discard_handle(session, chunk->uri, NULL));
 		if (ret == 0)
 			chunk->evicted = 1;
@@ -506,7 +506,7 @@ __lsm_drop_file(WT_SESSION_IMPL *session, const char *uri)
 	 *
 	 * This will fail with EBUSY if the file is still in use.
 	 */
-	WT_WITH_HANDLE_LIST_LOCK(session, ret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	   ret = __lsm_discard_handle(session, uri, WT_CHECKPOINT));
 	WT_RET(ret);
 
diff --git a/src/schema/schema_drop.c b/src/schema/schema_drop.c
index 6ac76930c9a..ead8cc45c62 100644
--- a/src/schema/schema_drop.c
+++ b/src/schema/schema_drop.c
@@ -29,7 +29,7 @@ __drop_file(
 		return (EINVAL);
 
 	/* Close all btree handles associated with this file. */
-	WT_WITH_HANDLE_LIST_LOCK(session, ret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    ret = __wt_conn_dhandle_close_all(session, uri, force));
 	WT_RET(ret);
 
diff --git a/src/schema/schema_rename.c b/src/schema/schema_rename.c
index 4ec126394dd..21402ed9332 100644
--- a/src/schema/schema_rename.c
+++ b/src/schema/schema_rename.c
@@ -30,7 +30,7 @@ __rename_file(
 		return (EINVAL);
 
 	/* Close any btree handles in the file. */
-	WT_WITH_HANDLE_LIST_LOCK(session, ret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    ret = __wt_conn_dhandle_close_all(session, uri, false));
 	WT_ERR(ret);
 
diff --git a/src/schema/schema_worker.c b/src/schema/schema_worker.c
index a2fe5244c4d..b5ee3bb7f7d 100644
--- a/src/schema/schema_worker.c
+++ b/src/schema/schema_worker.c
@@ -49,7 +49,7 @@ __wt_schema_worker(WT_SESSION_IMPL *session,
 			 * any open file handles, including checkpoints.
 			 */
 			if (FLD_ISSET(open_flags, WT_DHANDLE_EXCLUSIVE)) {
-				WT_WITH_HANDLE_LIST_LOCK(session, ret,
+				WT_WITH_HANDLE_LIST_LOCK(session,
 				    ret = __wt_conn_dhandle_close_all(
 				    session, uri, false));
 				WT_ERR(ret);
@@ -63,7 +63,7 @@ __wt_schema_worker(WT_SESSION_IMPL *session,
 			} else if (ret == EBUSY) {
 				WT_ASSERT(session, !FLD_ISSET(
 				    open_flags, WT_DHANDLE_EXCLUSIVE));
-				WT_WITH_HANDLE_LIST_LOCK(session, ret,
+				WT_WITH_HANDLE_LIST_LOCK(session,
 				    ret = __wt_conn_btree_apply_single_ckpt(
 				    session, uri, file_func, cfg));
 			}
diff --git a/src/session/session_dhandle.c b/src/session/session_dhandle.c
index d89bb8d133c..1ee3342442c 100644
--- a/src/session/session_dhandle.c
+++ b/src/session/session_dhandle.c
@@ -454,7 +454,7 @@ __session_get_dhandle(
 	 * We didn't find a match in the session cache, search the shared
 	 * handle list and cache the handle we find.
 	 */
-	WT_WITH_HANDLE_LIST_LOCK(session, ret,
+	WT_WITH_HANDLE_LIST_LOCK(session,
 	    ret = __session_find_shared_dhandle(session, uri, checkpoint));
 	if (ret == 0)
 		ret = __session_add_dhandle(session, NULL);
@@ -511,7 +511,7 @@ __wt_session_get_btree(WT_SESSION_IMPL *session,
 			WT_RET(__wt_writeunlock(session, dhandle->rwlock));
 
 			WT_WITH_SCHEMA_LOCK(session, ret,
-			    WT_WITH_HANDLE_LIST_LOCK(session, ret,
+			    WT_WITH_HANDLE_LIST_LOCK(session,
 				ret = __wt_session_get_btree(
 				session, uri, checkpoint, cfg, flags)));
 
diff --git a/src/txn/txn_ckpt.c b/src/txn/txn_ckpt.c
index 725e46acb0c..6a2c1eef826 100644
--- a/src/txn/txn_ckpt.c
+++ b/src/txn/txn_ckpt.c
@@ -184,7 +184,7 @@ __checkpoint_apply(WT_SESSION_IMPL *session, const char *cfg[],
 			    session->ckpt_handle[i].dhandle,
 			    ret = (*op)(session, cfg));
 		else
-			WT_WITH_HANDLE_LIST_LOCK(session, ret,
+			WT_WITH_HANDLE_LIST_LOCK(session,
 			    ret = __wt_conn_btree_apply_single(session,
 			    session->ckpt_handle[i].name, NULL, op, cfg));
 		WT_RET(ret);
@@ -387,7 +387,7 @@ __txn_checkpoint(WT_SESSION_IMPL *session, const char *cfg[])
 	 */
 	WT_WITH_SCHEMA_LOCK(session, ret,
 	    WT_WITH_TABLE_LOCK(session, ret,
-		WT_WITH_HANDLE_LIST_LOCK(session, ret,
+		WT_WITH_HANDLE_LIST_LOCK(session,
 		    ret = __checkpoint_apply_all(
 		    session, cfg, __wt_checkpoint_list, NULL))));
 	WT_ERR(ret);