Skip to content

Commit

Permalink
SERVER-25846 Coverity analysis defect 99861: Dereference after null c…
Browse files Browse the repository at this point in the history 
…heck (#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.
  • Loading branch information
@keithbostic
keithbostic committed Aug 29, 2016
1 parent 8b40b2f commit e80ef3c
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 47 deletions.
2 changes: 1 addition & 1 deletion src/async/async_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ __wt_async_flush(WT_SESSION_IMPL *session)
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.
*/
Expand Down
2 changes: 1 addition & 1 deletion src/include/extern.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
22 changes: 3 additions & 19 deletions src/os_posix/os_mtx_cond.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
21 changes: 3 additions & 18 deletions src/os_win/os_mtx_cond.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 4 additions & 8 deletions src/support/thread_group.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,7 @@ __thread_group_resize(
*/
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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit e80ef3c

Please sign in to comment.