epics/pcas
0
0
Fork 0
Code Issues Pull Requests Projects Wiki Activity

This was defensive coding against deadlock occurring when they hold a lock

Browse Source 
in the expire callback that they also hold when starting the timer.  I dont know
how to protect them against a situation where they hold a lock in the expire
callback and also hold it when canceling the timer, but at least that is a less
common situation.
This commit is contained in:
Jeff Hill
2002-08-16 00:07:09 +00:00
parent 936053422b
commit aa54a26c8b
2 changed files with 64 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

79
src/libCom/timer/timer.cpp
View File

@@ -60,7 +60,6 @@ void timer::start ( epicsTimerNotify & notify, double delaySeconds )
void timer::start ( epicsTimerNotify & notify, const epicsTime & expire )
{
epicsGuard < epicsMutex > locker ( this->queue.mutex );
this->privateCancel ( locker );
this->privateStart ( notify, expire );
}
@@ -69,6 +68,20 @@ void timer::privateStart ( epicsTimerNotify & notify, const epicsTime & expire )
this->pNotify = & notify;
this->exp = expire;
bool reschedualNeeded = false;
if ( this->curState == stateActive ) {
// above expire time and notify will override any restart parameters
// that may be returned from the timer expire callback
return;
}
else if ( this->curState == statePending ) {
this->queue.timerList.remove ( *this );
if ( this->queue.timerList.first() == this &&
this->queue.timerList.count() > 0 ) {
reschedualNeeded = true;
}
}
# ifdef DEBUG
unsigned preemptCount=0u;
# endif
@@ -87,7 +100,7 @@ void timer::privateStart ( epicsTimerNotify & notify, const epicsTime & expire )
// add to the beginning of the list
//
this->queue.timerList.push ( *this );
this->queue.notify.reschedule ();
reschedualNeeded = true;
break;
}
if ( pTmr->exp <= this->exp ) {
@@ -104,6 +117,10 @@ void timer::privateStart ( epicsTimerNotify & notify, const epicsTime & expire )
}
this->curState = timer::statePending;
if ( reschedualNeeded ) {
this->queue.notify.reschedule ();
}
# if defined(DEBUG) && 0
this->show ( 10u );
@@ -119,34 +136,40 @@ void timer::privateStart ( epicsTimerNotify & notify, const epicsTime & expire )
void timer::cancel ()
{
if ( this->curState == statePending || this->curState == stateActive ) {
epicsGuard < epicsMutex > locker ( this->queue.mutex );
this->privateCancel ( locker );
}
}
void timer::privateCancel ( epicsGuard < epicsMutex > & locker )
{
if ( this->curState == statePending ) {
if ( this->queue.timerList.first() == this ) {
this->queue.notify.reschedule ();
}
this->queue.timerList.remove ( *this );
this->curState = stateLimbo;
this->pNotify = 0;
}
else if ( this->curState == stateActive ) {
this->queue.cancelPending = true;
if ( this->queue.processThread != epicsThreadGetIdSelf() ) {
// make certain timer expire() does not run after cancel () returns,
// but dont require that lock is applied while calling expire()
{
epicsGuardRelease < epicsMutex > autoRelease ( locker );
while ( this->queue.cancelPending &&
this->queue.pExpireTmr == this ) {
this->queue.cancelBlockingEvent.wait ();
bool reschedual = false;
bool wakeupCancelBlockingThreads = false;
{
epicsGuard < epicsMutex > locker ( this->queue.mutex );
this->pNotify = 0;
if ( this->curState == statePending ) {
this->queue.timerList.remove ( *this );
this->curState = stateLimbo;
if ( this->queue.timerList.first() == this &&
this->queue.timerList.count() > 0 ) {
reschedual = true;
}
}
// in case other threads are waiting
else if ( this->curState == stateActive ) {
this->queue.cancelPending = true;
if ( this->queue.processThread != epicsThreadGetIdSelf() ) {
// make certain timer expire() does not run after cancel () returns,
// but dont require that lock is applied while calling expire()
{
epicsGuardRelease < epicsMutex > autoRelease ( locker );
while ( this->queue.cancelPending &&
this->queue.pExpireTmr == this ) {
this->queue.cancelBlockingEvent.wait ();
}
}
// in case other threads are waiting
wakeupCancelBlockingThreads = true;
}
}
}
if ( reschedual ) {
this->queue.notify.reschedule ();
}
if ( wakeupCancelBlockingThreads ) {
this->queue.cancelBlockingEvent.signal ();
}
}

19
src/libCom/timer/timerQueue.cpp
View File

@@ -43,7 +43,7 @@ double timerQueue::process ( const epicsTime & currentTime )
if ( this->pExpireTmr ) {
// if some other thread is processing the queue
// (or if this is a recursive call)
timer *pTmr = this->timerList.first ();
timer * pTmr = this->timerList.first ();
if ( pTmr ) {
double delay = pTmr->exp - currentTime;
if ( delay < 0.0 ) {
@@ -100,14 +100,17 @@ double timerQueue::process ( const epicsTime & currentTime )
expStat = pTmpNotify->expire ( currentTime );
}
this->pExpireTmr->curState = timer::stateLimbo;
//
// only restart if they didnt cancel() the timer
// while the call back was running
//
if ( this->cancelPending ) {
this->pExpireTmr->curState = timer::stateLimbo;
this->pExpireTmr->pNotify = 0;
// 1) if another thread is canceling cancel() waits for this
// 1) if another thread is canceling then cancel() waits for
// the event below
// 2) if this thread is canceling in the timer callback then
// dont touch timer or notify here because the cancel might
// have occurred because they destroyed the timer in the
@@ -115,15 +118,19 @@ double timerQueue::process ( const epicsTime & currentTime )
this->cancelPending = false;
this->cancelBlockingEvent.signal ();
}
else if ( this->pExpireTmr->pNotify ) {
// pNotify was cleared above so if it is valid now we know that
// someone has started the timer from another thread and that
// predominates over the restart parameters from expire.
this->pExpireTmr->privateStart (
*this->pExpireTmr->pNotify, this->pExpireTmr->exp );
}
else {
// restart as nec
if ( expStat.restart() ) {
this->pExpireTmr->privateStart (
*pTmpNotify, currentTime + expStat.expirationDelay() );
}
else {
this->pExpireTmr->curState = timer::stateLimbo;
}
}
this->pExpireTmr = 0;