> The following lines are very problematic: > > EnterCriticalSection( &p_timer->cb_lock ); > (p_timer->pfn_callback)( (void*)p_timer->context ); > LeaveCriticalSection( &p_timer->cb_lock ); > > Nothing specific with timer, but if you have a code that takes a lock, and > call someone else code than this is asking for trouble. The reason is that > if he has done something like this (take a lock and call you back) than > there is a deadlock. This can be avoided by documentation, but I would > rather avoid this issues...
This locking is fine. The cb_lock lock is specifically only used here and not in other calls. There is no chance of deadlock coming from the use of this lock. > One thing that I'm missing here is that start might not do anything but > still return success. I would like to have two more return code, one for > the case of timer not running because the time is too big, and one because > another callback is running. This is to allow the user to know if he should > wait for a callback or not. If start returns success, then the timer will run. > Another issue here is that start() might end waiting for the callback to > return. Again this is a very problematic behavior (for a general timer) > since two of this can cerate a deadlock. This is especially true for using > process io threads that one has no control over. That is documentation > should say, either don't call start when there is already something > running, or don't call timer.start() from a system thread. This is because > two system threads might create a deadlock. Start and trim never wait for a callback to return. That's why NULL is passed into DeleteTimerQueueTimer rather than INVALID_HANDLE_VALUE. NULL is non-blocking, INVALID_HANDLE_VALUE is blocking. > Also on start one takes the critical section and calls > > if( p_timer->h_timer ) > DeleteTimerQueueTimer( p_timer->h_timer_queue, p_timer- > >h_timer, NULL ); > If the callback has already started running but haven't yet called > __clear_timer_handle() there is a deadlock here. As one is holding the > spinlock waiting for the other to finish running , while the other is > trying to capture the spinlock. This is why there are two locks. One lock is used to synchronize the state, the other lock to serialize the callbacks. > So, the general rule should be don't call start on this timer from io > threads... Start can be called from any thread. _______________________________________________ ofw mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
