From 5cecdb1b8bfefdb335130dc64d1e14e475f999d2 Mon Sep 17 00:00:00 2001 From: Sean Hefty Date: Tue, 29 Jun 2010 19:33:05 +0000 Subject: [PATCH] complib/user: fix timer race conditions This serializes the callbacks for a timer and provides synchronization for multi-threaded timer usage. cl_timer_stop is not allowed to be called from a timer callback, and will block until any callback has completed. (Technically, this should work, but it is not supported.) cl_timer_start simply calls cl_timer_trim cl_timer_trim changes the timeout of a queued timer if the new timeout time is less than the last set timeout. Signed-off-by: Sean Hefty git-svn-id: svn://openib.tc.cornell.edu/gen1@2840 ad392aa1-c5ef-ae45-8dd8-e69d62a5ef86 --- trunk/core/complib/user/cl_timer.c | 83 +++++++++++++++------------ trunk/inc/user/complib/cl_timer_osd.h | 5 +- 2 files changed, 47 insertions(+), 41 deletions(-) diff --git a/trunk/core/complib/user/cl_timer.c b/trunk/core/complib/user/cl_timer.c index 4f2ce0fc..5cab378a 100644 --- a/trunk/core/complib/user/cl_timer.c +++ b/trunk/core/complib/user/cl_timer.c @@ -32,23 +32,31 @@ #include "complib/cl_timer.h" + +#define CL_MAX_TIME 0xFFFFFFFF + +/* + * If timeout_time is 0, the timer has been stopped. + */ static void CALLBACK __timer_callback( IN cl_timer_t* const p_timer, IN BOOLEAN timer_signalled ) { - /* timer_signalled is always TRUE, and has no value. */ - CL_ASSERT( timer_signalled ); + uint64_t timeout; UNUSED_PARAM( timer_signalled ); + EnterCriticalSection( &p_timer->cb_lock ); + EnterCriticalSection( &p_timer->lock ); + timeout = p_timer->timeout_time; p_timer->timeout_time = 0; - p_timer->thread_id = GetCurrentThreadId(); + LeaveCriticalSection( &p_timer->lock ); - (p_timer->pfn_callback)( (void*)p_timer->context ); - - p_timer->thread_id = 0; + if( timeout ) + (p_timer->pfn_callback)( (void*)p_timer->context ); + LeaveCriticalSection( &p_timer->cb_lock ); } @@ -56,9 +64,7 @@ void cl_timer_construct( IN cl_timer_t* const p_timer ) { - p_timer->h_timer = NULL; - p_timer->timeout_time = 0; - p_timer->thread_id = 0; + memset(p_timer, 0, sizeof *p_timer); } @@ -76,6 +82,11 @@ cl_timer_init( p_timer->pfn_callback = pfn_callback; p_timer->context = context; + InitializeCriticalSection( &p_timer->lock ); + InitializeCriticalSection( &p_timer->cb_lock ); + if( !CreateTimerQueueTimer( &p_timer->h_timer, NULL, __timer_callback, + p_timer, CL_MAX_TIME, CL_MAX_TIME, WT_EXECUTEINIOTHREAD ) ) + return CL_ERROR; return( CL_SUCCESS ); } @@ -86,7 +97,9 @@ cl_timer_destroy( { CL_ASSERT( p_timer ); - cl_timer_stop( p_timer ); + DeleteTimerQueueTimer( NULL, p_timer->h_timer, INVALID_HANDLE_VALUE ); + DeleteCriticalSection( &p_timer->lock ); + DeleteCriticalSection( &p_timer->cb_lock ); } @@ -95,19 +108,7 @@ cl_timer_start( IN cl_timer_t* const p_timer, IN const uint32_t time_ms ) { - CL_ASSERT( p_timer ); - - cl_timer_stop( p_timer ); - - p_timer->timeout_time = cl_get_time_stamp() + (((uint64_t)time_ms) * 1000); - - if( !CreateTimerQueueTimer( &p_timer->h_timer, NULL, __timer_callback, - p_timer, time_ms, 0, WT_EXECUTEINIOTHREAD ) ) - { - return( CL_ERROR ); - } - - return( CL_SUCCESS ); + return cl_timer_trim( p_timer, time_ms ); } @@ -116,35 +117,41 @@ cl_timer_trim( IN cl_timer_t* const p_timer, IN const uint32_t time_ms ) { - uint64_t timeout_time; + uint64_t timeout; + cl_status_t status = CL_SUCCESS; CL_ASSERT( p_timer ); CL_ASSERT( p_timer->pfn_callback ); - /* Calculate the timeout time in the timer object. */ - timeout_time = cl_get_time_stamp() + (((uint64_t)time_ms) * 1000); - - /* Only pull in the timeout time. */ - if( p_timer->timeout_time && p_timer->timeout_time < timeout_time ) - return( CL_SUCCESS ); - - return cl_timer_start( p_timer, time_ms ); + EnterCriticalSection( &p_timer->lock ); + timeout = cl_get_time_stamp() + (((uint64_t)time_ms) * 1000); + if ( !p_timer->timeout_time || timeout < p_timer->timeout_time ) + { + if( ChangeTimerQueueTimer( NULL, p_timer->h_timer, time_ms, CL_MAX_TIME ) ) + p_timer->timeout_time = timeout; + else + status = CL_ERROR; + } + LeaveCriticalSection( &p_timer->lock ); + return status; } +/* + * Acquire cb_lock to ensure that all callbacks have completed. + */ void cl_timer_stop( IN cl_timer_t* const p_timer ) { CL_ASSERT( p_timer ); - if( p_timer->h_timer && p_timer->thread_id != GetCurrentThreadId() ) - { - /* Make sure we block until the timer is cancelled. */ - DeleteTimerQueueTimer( NULL, p_timer->h_timer, INVALID_HANDLE_VALUE ); - p_timer->h_timer = NULL; - } + EnterCriticalSection( &p_timer->cb_lock ); + EnterCriticalSection( &p_timer->lock ); p_timer->timeout_time = 0; + ChangeTimerQueueTimer( NULL, p_timer->h_timer, CL_MAX_TIME, CL_MAX_TIME ); + LeaveCriticalSection( &p_timer->lock ); + LeaveCriticalSection( &p_timer->cb_lock ); } diff --git a/trunk/inc/user/complib/cl_timer_osd.h b/trunk/inc/user/complib/cl_timer_osd.h index 0a023ba6..a4a11958 100644 --- a/trunk/inc/user/complib/cl_timer_osd.h +++ b/trunk/inc/user/complib/cl_timer_osd.h @@ -38,15 +38,14 @@ #include "cl_types.h" - -/* Timer object definition. */ typedef struct _cl_timer { HANDLE h_timer; cl_pfn_timer_callback_t pfn_callback; const void *context; uint64_t timeout_time; - DWORD thread_id; + CRITICAL_SECTION lock; + CRITICAL_SECTION cb_lock; } cl_timer_t; -- 2.46.0