SERVER-25846 Coverity analysis defect 99861: Dereference after null check (#2993)

* SERVER-25846 Coverity analysis defect 99861: Dereference after null check

Now the __wt_cond_alloc, __wt_cond_wait_signal and __wt_cond_signal
functions can panic on failure, they potentially indirect through the
WT_SESSION handle to panic the WT_CONNECTION structure. Fortunately,
they should not be called with a NULL WT_SESSION handle, remove the
tests for that condition, making Coverity happy.

* Missed a call to __wt_cond_wait that passed an explicit NULL WT_SESSION;
replace the NULL with the existing session handle.

* minor whitespace cleanup during condition variable review.

5 files changed, 12 insertions, 47 deletions

diff --git a/src/async/async_api.c b/src/async/async_api.c
index d55732abfb5..0a855514a07 100644
--- a/src/async/async_api.c
+++ b/src/async/async_api.c
@@ -541,7 +541,7 @@ retry:
 	async->flush_op.state = WT_ASYNCOP_READY;
 	WT_RET(__wt_async_op_enqueue(session, &async->flush_op));
 	while (async->flush_state != WT_ASYNC_FLUSH_COMPLETE)
-		__wt_cond_wait(NULL, async->flush_cond, 100000);
+		__wt_cond_wait(session, async->flush_cond, 100000);
 	/*
 	 * Flush is done.  Clear the flags.
 	 */
diff --git a/src/include/extern.h b/src/include/extern.h
index 2992a97b727..e3fbeb7aa28 100644
--- a/src/include/extern.h
+++ b/src/include/extern.h
@@ -713,7 +713,7 @@ extern void __wt_stat_join_aggregate( WT_JOIN_STATS **from, WT_JOIN_STATS *to);
 extern WT_THREAD_RET __wt_thread_run(void *arg);
 extern int __wt_thread_group_resize( WT_SESSION_IMPL *session, WT_THREAD_GROUP *group, uint32_t new_min, uint32_t new_max, uint32_t flags) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result));
 extern int __wt_thread_group_create( WT_SESSION_IMPL *session, WT_THREAD_GROUP *group, const char *name, uint32_t min, uint32_t max, uint32_t flags, int (*run_func)(WT_SESSION_IMPL *session, WT_THREAD *context)) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result));
-extern int __wt_thread_group_destroy( WT_SESSION_IMPL *session, WT_THREAD_GROUP *group) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result));
+extern int __wt_thread_group_destroy(WT_SESSION_IMPL *session, WT_THREAD_GROUP *group) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result));
 extern int __wt_thread_group_start_one( WT_SESSION_IMPL *session, WT_THREAD_GROUP *group, bool wait) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result));
 extern void __wt_txn_release_snapshot(WT_SESSION_IMPL *session);
 extern int __wt_txn_get_snapshot(WT_SESSION_IMPL *session) WT_GCC_FUNC_DECL_ATTRIBUTE((warn_unused_result));
diff --git a/src/os_posix/os_mtx_cond.c b/src/os_posix/os_mtx_cond.c
index fb5a0f018cf..9f17d9ff381 100644
--- a/src/os_posix/os_mtx_cond.c
+++ b/src/os_posix/os_mtx_cond.c
@@ -19,10 +19,6 @@ __wt_cond_alloc(WT_SESSION_IMPL *session,
 	WT_CONDVAR *cond;
 	WT_DECL_RET;
 
-	/*
-	 * !!!
-	 * This function MUST handle a NULL session handle.
-	 */
 	WT_RET(__wt_calloc_one(session, &cond));
 
 	WT_ERR(pthread_mutex_init(&cond->mtx, NULL));
@@ -60,15 +56,8 @@ __wt_cond_wait_signal(
 	if (__wt_atomic_addi32(&cond->waiters, 1) == 0)
 		return;
 
-	/*
-	 * !!!
-	 * This function MUST handle a NULL session handle.
-	 */
-	if (session != NULL) {
-		__wt_verbose(
-		    session, WT_VERB_MUTEX, "wait %s", cond->name, cond);
-		WT_STAT_FAST_CONN_INCR(session, cond_wait);
-	}
+	__wt_verbose(session, WT_VERB_MUTEX, "wait %s", cond->name);
+	WT_STAT_FAST_CONN_INCR(session, cond_wait);
 
 	WT_ERR(pthread_mutex_lock(&cond->mtx));
 	locked = true;
@@ -118,12 +107,7 @@ __wt_cond_signal(WT_SESSION_IMPL *session, WT_CONDVAR *cond)
 
 	locked = false;
 
-	/*
-	 * !!!
-	 * This function MUST handle a NULL session handle.
-	 */
-	if (session != NULL)
-		__wt_verbose(session, WT_VERB_MUTEX, "signal %s", cond->name);
+	__wt_verbose(session, WT_VERB_MUTEX, "signal %s", cond->name);
 
 	/* Fast path if already signalled. */
 	if (cond->waiters == -1)
diff --git a/src/os_win/os_mtx_cond.c b/src/os_win/os_mtx_cond.c
index da61904443f..27207d289a6 100644
--- a/src/os_win/os_mtx_cond.c
+++ b/src/os_win/os_mtx_cond.c
@@ -18,10 +18,6 @@ __wt_cond_alloc(WT_SESSION_IMPL *session,
 {
 	WT_CONDVAR *cond;
 
-	/*
-	 * !!!
-	 * This function MUST handle a NULL session handle.
-	 */
 	WT_RET(__wt_calloc_one(session, &cond));
 
 	InitializeCriticalSection(&cond->mtx);
@@ -57,14 +53,8 @@ __wt_cond_wait_signal(
 	if (__wt_atomic_addi32(&cond->waiters, 1) == 0)
 		return;
 
-	/*
-	 * !!!
-	 * This function MUST handle a NULL session handle.
-	 */
-	if (session != NULL) {
-		__wt_verbose(session, WT_VERB_MUTEX, "wait %s", cond->name);
-		WT_STAT_FAST_CONN_INCR(session, cond_wait);
-	}
+	__wt_verbose(session, WT_VERB_MUTEX, "wait %s", cond->name);
+	WT_STAT_FAST_CONN_INCR(session, cond_wait);
 
 	EnterCriticalSection(&cond->mtx);
 	locked = true;
@@ -131,12 +121,7 @@ __wt_cond_signal(WT_SESSION_IMPL *session, WT_CONDVAR *cond)
 
 	locked = false;
 
-	/*
-	 * !!!
-	 * This function MUST handle a NULL session handle.
-	 */
-	if (session != NULL)
-		__wt_verbose(session, WT_VERB_MUTEX, "signal %s", cond->name);
+	__wt_verbose(session, WT_VERB_MUTEX, "signal %s", cond->name);
 
 	/* Fast path if already signalled. */
 	if (cond->waiters == -1)
diff --git a/src/support/thread_group.c b/src/support/thread_group.c
index 016956c527f..0def59fa29f 100644
--- a/src/support/thread_group.c
+++ b/src/support/thread_group.c
@@ -206,8 +206,7 @@ err:	/*
 	 */
 	if (ret != 0) {
 		WT_TRET(__wt_thread_group_destroy(session, group));
-		WT_PANIC_RET(session, ret,
-		    "Error while resizing thread group");
+		WT_PANIC_RET(session, ret, "Error while resizing thread group");
 	}
 	return (ret);
 }
@@ -229,8 +228,7 @@ __wt_thread_group_resize(
 	    group, group->min, new_min, group->max, new_max);
 
 	__wt_writelock(session, group->lock);
-	WT_TRET(__thread_group_resize(
-	    session, group, new_min, new_max, flags));
+	WT_TRET(__thread_group_resize(session, group, new_min, new_max, flags));
 	__wt_writeunlock(session, group->lock);
 	return (ret);
 }
@@ -283,16 +281,14 @@ err:	if (ret != 0) {
  *	Shut down a thread group.  Our caller must hold the lock.
  */
 int
-__wt_thread_group_destroy(
-    WT_SESSION_IMPL *session, WT_THREAD_GROUP *group)
+__wt_thread_group_destroy(WT_SESSION_IMPL *session, WT_THREAD_GROUP *group)
 {
 	WT_DECL_RET;
 
 	__wt_verbose(session, WT_VERB_THREAD_GROUP,
 	    "Destroying thread group: %p\n", group);
 
-	WT_ASSERT(session,
-	    __wt_rwlock_islocked(session, group->lock));
+	WT_ASSERT(session, __wt_rwlock_islocked(session, group->lock));
 
 	/* Shut down all threads and free associated resources. */
 	WT_TRET(__thread_group_shrink(session, group, 0));