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 <sean.hefty@intel.com>
git-svn-id: svn://openib.tc.cornell.edu/gen1@2840 ad392aa1-c5ef-ae45-8dd8-e69d62a5ef86

diff --git a/trunk/core/complib/user/cl_timer.c b/trunk/core/complib/user/cl_timer.c
index 4f2ce0fcbc67c5766b92d563cbc9fc6cd94f13ee..5cab378aa37041cfb65fc36254b6c9f071b1ef02 100644 (file)
--- a/trunk/core/complib/user/cl_timer.c
+++ b/trunk/core/complib/user/cl_timer.c
@@ -32,23 +32,31 @@
 \r
 \r
 #include "complib/cl_timer.h"\r
+ \r
 \r
+#define CL_MAX_TIME 0xFFFFFFFF\r
 \r
+\r
+/*\r
+ * If timeout_time is 0, the timer has been stopped.\r
+ */\r
 static void CALLBACK\r
 __timer_callback( \r
        IN cl_timer_t* const p_timer,\r
        IN BOOLEAN timer_signalled )\r
 {\r
-       /* timer_signalled is always TRUE, and has no value. */\r
-       CL_ASSERT( timer_signalled );\r
+       uint64_t timeout;\r
        UNUSED_PARAM( timer_signalled );\r
 \r
+       EnterCriticalSection( &p_timer->cb_lock );\r
+       EnterCriticalSection( &p_timer->lock );\r
+       timeout = p_timer->timeout_time;\r
        p_timer->timeout_time = 0;\r
-       p_timer->thread_id = GetCurrentThreadId();\r
+       LeaveCriticalSection( &p_timer->lock );\r
 \r
-       (p_timer->pfn_callback)( (void*)p_timer->context );\r
-\r
-       p_timer->thread_id = 0;\r
+       if( timeout )\r
+               (p_timer->pfn_callback)( (void*)p_timer->context );\r
+       LeaveCriticalSection( &p_timer->cb_lock );\r
 }\r
 \r
 \r
@@ -56,9 +64,7 @@ void
 cl_timer_construct(\r
        IN      cl_timer_t* const       p_timer )\r
 {\r
-       p_timer->h_timer = NULL;\r
-       p_timer->timeout_time = 0;\r
-       p_timer->thread_id = 0;\r
+       memset(p_timer, 0, sizeof *p_timer);\r
 }\r
 \r
 \r
@@ -76,6 +82,11 @@ cl_timer_init(
 \r
        p_timer->pfn_callback = pfn_callback;\r
        p_timer->context = context;\r
+       InitializeCriticalSection( &p_timer->lock );\r
+       InitializeCriticalSection( &p_timer->cb_lock );\r
+       if( !CreateTimerQueueTimer( &p_timer->h_timer, NULL, __timer_callback,\r
+               p_timer, CL_MAX_TIME, CL_MAX_TIME, WT_EXECUTEINIOTHREAD ) )\r
+               return CL_ERROR;\r
        return( CL_SUCCESS );\r
 }\r
 \r
@@ -86,7 +97,9 @@ cl_timer_destroy(
 {\r
        CL_ASSERT( p_timer );\r
        \r
-       cl_timer_stop( p_timer );\r
+       DeleteTimerQueueTimer( NULL, p_timer->h_timer, INVALID_HANDLE_VALUE );\r
+       DeleteCriticalSection( &p_timer->lock );\r
+       DeleteCriticalSection( &p_timer->cb_lock );\r
 }\r
 \r
 \r
@@ -95,19 +108,7 @@ cl_timer_start(
        IN      cl_timer_t* const       p_timer,\r
        IN      const uint32_t          time_ms )\r
 {\r
-       CL_ASSERT( p_timer );\r
-\r
-       cl_timer_stop( p_timer );\r
-\r
-       p_timer->timeout_time = cl_get_time_stamp() + (((uint64_t)time_ms) * 1000);\r
-\r
-       if( !CreateTimerQueueTimer( &p_timer->h_timer, NULL, __timer_callback,\r
-               p_timer, time_ms, 0, WT_EXECUTEINIOTHREAD ) )\r
-       {\r
-               return( CL_ERROR );\r
-       }\r
-       \r
-       return( CL_SUCCESS );\r
+       return cl_timer_trim( p_timer, time_ms );\r
 }\r
 \r
 \r
@@ -116,35 +117,41 @@ cl_timer_trim(
        IN      cl_timer_t* const       p_timer,\r
        IN      const uint32_t          time_ms )\r
 {\r
-       uint64_t                timeout_time;\r
+       uint64_t        timeout;\r
+       cl_status_t     status = CL_SUCCESS;\r
 \r
        CL_ASSERT( p_timer );\r
        CL_ASSERT( p_timer->pfn_callback );\r
 \r
-       /* Calculate the timeout time in the timer object. */\r
-       timeout_time = cl_get_time_stamp() + (((uint64_t)time_ms) * 1000);\r
-\r
-       /* Only pull in the timeout time. */\r
-       if( p_timer->timeout_time && p_timer->timeout_time < timeout_time )\r
-               return( CL_SUCCESS );\r
-\r
-       return cl_timer_start( p_timer, time_ms );\r
+       EnterCriticalSection( &p_timer->lock );\r
+       timeout = cl_get_time_stamp() + (((uint64_t)time_ms) * 1000);\r
+       if ( !p_timer->timeout_time || timeout < p_timer->timeout_time )\r
+       {\r
+               if( ChangeTimerQueueTimer( NULL, p_timer->h_timer, time_ms, CL_MAX_TIME ) )\r
+                       p_timer->timeout_time = timeout;\r
+               else\r
+                       status = CL_ERROR;\r
+       }\r
+       LeaveCriticalSection( &p_timer->lock );\r
+       return status;\r
 }\r
 \r
 \r
+/*\r
+ * Acquire cb_lock to ensure that all callbacks have completed.\r
+ */\r
 void\r
 cl_timer_stop(\r
        IN      cl_timer_t* const       p_timer )\r
 {\r
        CL_ASSERT( p_timer );\r
 \r
-       if( p_timer->h_timer && p_timer->thread_id != GetCurrentThreadId() )\r
-       {\r
-               /* Make sure we block until the timer is cancelled. */\r
-               DeleteTimerQueueTimer( NULL, p_timer->h_timer, INVALID_HANDLE_VALUE );\r
-               p_timer->h_timer = NULL;\r
-       }\r
+       EnterCriticalSection( &p_timer->cb_lock );\r
+       EnterCriticalSection( &p_timer->lock );\r
        p_timer->timeout_time = 0;\r
+       ChangeTimerQueueTimer( NULL, p_timer->h_timer, CL_MAX_TIME, CL_MAX_TIME );\r
+       LeaveCriticalSection( &p_timer->lock );\r
+       LeaveCriticalSection( &p_timer->cb_lock );\r
 }\r
 \r
 \r

diff --git a/trunk/inc/user/complib/cl_timer_osd.h b/trunk/inc/user/complib/cl_timer_osd.h
index 0a023ba60992bd11b5058f95ab61ba01aad87bdd..a4a119580bcb1bd94daebc5fce975be9dcf97c76 100644 (file)
--- a/trunk/inc/user/complib/cl_timer_osd.h
+++ b/trunk/inc/user/complib/cl_timer_osd.h
@@ -38,15 +38,14 @@
 \r
 #include "cl_types.h"\r
 \r
-\r
-/* Timer object definition. */\r
 typedef struct _cl_timer\r
 {\r
        HANDLE                                  h_timer;\r
        cl_pfn_timer_callback_t pfn_callback;\r
        const void                              *context;\r
        uint64_t                                timeout_time;\r
-       DWORD                                   thread_id;\r
+       CRITICAL_SECTION                lock;\r
+       CRITICAL_SECTION                cb_lock;\r
 \r
 } cl_timer_t;\r
 \r