From fefe6fd1fcf4863ec917d6c8248822d634ef35d6 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Thu, 16 Jul 2015 11:48:29 -0500 Subject: [PATCH] Fix shutdown issues with scan and callback. The main reason for this merge proposal is the change to "public" API functions. Use atomic counter to resolve data race on threadsRunning in callback. Split up callbackShutdown() and scanShutdown() into two phases *Stop() and *Cleanup(). The *Stop() functions signal worker threads, and wait for them to exit. The *Cleanup() functions actually reclaim global resources. These two mechanisms have couplings which are quite complex. I/O Intr scans involve both scan lists and callbacks. --- src/ioc/db/callback.c | 54 ++++++++++++++++---------- src/ioc/db/callback.h | 3 +- src/ioc/db/dbScan.c | 6 ++- src/ioc/db/dbScan.h | 3 +- src/ioc/db/test/callbackParallelTest.c | 3 +- src/ioc/db/test/callbackTest.c | 3 +- src/ioc/misc/iocInit.c | 11 ++++-- 7 files changed, 54 insertions(+), 29 deletions(-) diff --git a/src/ioc/db/callback.c b/src/ioc/db/callback.c index 56a399ff6..897279ff9 100644 --- a/src/ioc/db/callback.c +++ b/src/ioc/db/callback.c @@ -54,6 +54,7 @@ typedef struct cbQueueSet { epicsEventId semWakeUp; epicsRingPointerId queue; int queueOverflow; + int shutdown; int threadsConfigured; int threadsRunning; } cbQueueSet; @@ -73,7 +74,6 @@ static epicsTimerQueueId timerQueue; enum ctl {ctlInit, ctlRun, ctlPause, ctlExit}; static volatile enum ctl cbCtl; static epicsEventId startStopEvent; -static void *exitCallback; /* Static data */ static char *threadNamePrefix[NUM_CALLBACK_PRIORITIES] = { @@ -149,12 +149,13 @@ int callbackParallelThreads(int count, const char *prio) static void callbackTask(void *arg) { - cbQueueSet *mySet = &callbackQueue[*(int*)arg]; + int prio = *(int*)arg; + cbQueueSet *mySet = &callbackQueue[prio]; taskwdInsert(0, NULL, NULL); epicsEventSignal(startStopEvent); - while(TRUE) { + while(!mySet->shutdown) { void *ptr; if (epicsRingPointerIsEmpty(mySet->queue)) epicsEventMustWait(mySet->semWakeUp); @@ -163,39 +164,50 @@ static void callbackTask(void *arg) CALLBACK *pcallback = (CALLBACK *)ptr; if(!epicsRingPointerIsEmpty(mySet->queue)) epicsEventMustTrigger(mySet->semWakeUp); - if (ptr == &exitCallback) goto shutdown; mySet->queueOverflow = FALSE; (*pcallback->callback)(pcallback); } } -shutdown: - mySet->threadsRunning--; + if(!epicsAtomicDecrIntT(&mySet->threadsRunning)) + epicsEventSignal(startStopEvent); taskwdRemove(0); - epicsEventSignal(startStopEvent); } -void callbackShutdown(void) +void callbackStop(void) { int i; if (cbCtl == ctlExit) return; cbCtl = ctlExit; - /* sequential shutdown of workers */ for (i = 0; i < NUM_CALLBACK_PRIORITIES; i++) { - while (callbackQueue[i].threadsRunning) { - if(epicsRingPointerPush(callbackQueue[i].queue, &exitCallback)) { - epicsEventSignal(callbackQueue[i].semWakeUp); - epicsEventWait(startStopEvent); - } else { - epicsThreadSleep(0.05); - } - } - assert(callbackQueue[i].threadsRunning==0); - epicsEventDestroy(callbackQueue[i].semWakeUp); - epicsRingPointerDelete(callbackQueue[i].queue); + callbackQueue[i].shutdown = 1; + epicsEventSignal(callbackQueue[i].semWakeUp); } + + for (i = 0; i < NUM_CALLBACK_PRIORITIES; i++) { + cbQueueSet *mySet = &callbackQueue[i]; + + while (epicsAtomicGetIntT(&mySet->threadsRunning)) { + epicsEventSignal(mySet->semWakeUp); + epicsEventWaitWithTimeout(startStopEvent, 0.1); + } + } +} + +void callbackCleanup(void) +{ + int i; + + for (i = 0; i < NUM_CALLBACK_PRIORITIES; i++) { + cbQueueSet *mySet = &callbackQueue[i]; + + assert(epicsAtomicGetIntT(&mySet->threadsRunning)==0); + epicsEventDestroy(mySet->semWakeUp); + epicsRingPointerDelete(mySet->queue); + } + epicsTimerQueueRelease(timerQueue); epicsEventDestroy(startStopEvent); startStopEvent = NULL; @@ -239,7 +251,7 @@ void callbackInit(void) cantProceed("Failed to spawn callback thread %s\n", threadName); } else { epicsEventWait(startStopEvent); - callbackQueue[i].threadsRunning++; + epicsAtomicIncrIntT(&callbackQueue[i].threadsRunning); } } } diff --git a/src/ioc/db/callback.h b/src/ioc/db/callback.h index 2732f9d5e..096854fb3 100644 --- a/src/ioc/db/callback.h +++ b/src/ioc/db/callback.h @@ -57,7 +57,8 @@ typedef void (*CALLBACKFUNC)(struct callbackPvt*); ( (USER) = (void *)((CALLBACK *)(PCALLBACK))->user ) epicsShareFunc void callbackInit(void); -epicsShareFunc void callbackShutdown(void); +epicsShareFunc void callbackStop(void); +epicsShareFunc void callbackCleanup(void); epicsShareFunc int callbackRequest(CALLBACK *pCallback); epicsShareFunc void callbackSetProcess( CALLBACK *pcallback, int Priority, void *pRec); diff --git a/src/ioc/db/dbScan.c b/src/ioc/db/dbScan.c index ba927a04b..9c1a1bef9 100644 --- a/src/ioc/db/dbScan.c +++ b/src/ioc/db/dbScan.c @@ -151,7 +151,7 @@ static void buildScanLists(void); static void addToList(struct dbCommon *precord, scan_list *psl); static void deleteFromList(struct dbCommon *precord, scan_list *psl); -void scanShutdown(void) +void scanStop(void) { int i; @@ -168,6 +168,10 @@ void scanShutdown(void) scanOnce((dbCommon *)&exitOnce); epicsEventWait(startStopEvent); +} + +void scanCleanup(void) +{ deletePeriodic(); ioscanDestroy(); diff --git a/src/ioc/db/dbScan.h b/src/ioc/db/dbScan.h index cd9666348..769a22893 100644 --- a/src/ioc/db/dbScan.h +++ b/src/ioc/db/dbScan.h @@ -46,7 +46,8 @@ struct dbCommon; epicsShareFunc long scanInit(void); epicsShareFunc void scanRun(void); epicsShareFunc void scanPause(void); -epicsShareFunc void scanShutdown(void); +epicsShareFunc void scanStop(void); +epicsShareFunc void scanCleanup(void); epicsShareFunc EVENTPVT eventNameToHandle(const char* event); epicsShareFunc void postEvent(EVENTPVT epvt); diff --git a/src/ioc/db/test/callbackParallelTest.c b/src/ioc/db/test/callbackParallelTest.c index eeb6ddaee..6dda789e3 100644 --- a/src/ioc/db/test/callbackParallelTest.c +++ b/src/ioc/db/test/callbackParallelTest.c @@ -184,7 +184,8 @@ MAIN(callbackParallelTest) free(pcbt[i]); } - callbackShutdown(); + callbackStop(); + callbackCleanup(); return testDone(); } diff --git a/src/ioc/db/test/callbackTest.c b/src/ioc/db/test/callbackTest.c index c4c6b31d9..d733f99d7 100644 --- a/src/ioc/db/test/callbackTest.c +++ b/src/ioc/db/test/callbackTest.c @@ -181,7 +181,8 @@ MAIN(callbackTest) free(pcbt[i]); } - callbackShutdown(); + callbackStop(); + callbackCleanup(); return testDone(); } diff --git a/src/ioc/misc/iocInit.c b/src/ioc/misc/iocInit.c index a8dca3c9a..9759ba18f 100644 --- a/src/ioc/misc/iocInit.c +++ b/src/ioc/misc/iocInit.c @@ -607,7 +607,8 @@ static void initialProcess(void) /* - * Shutdown processing. + * set DB_LINK and CA_LINK to PV_LINK + * Delete record scans */ static void doCloseLinks(dbRecordType *pdbRecordType, dbCommon *precord, void *user) @@ -678,8 +679,12 @@ int iocShutdown(void) if (iocState == iocVirgin || iocState == iocStopped) return 0; iterateRecords(doCloseLinks, NULL); if (iocBuildMode==buildIsolated) { - scanShutdown(); - callbackShutdown(); + /* stop and "join" threads */ + scanStop(); + callbackStop(); + /* free resources */ + scanCleanup(); + callbackCleanup(); iterateRecords(doFreeRecord, NULL); dbLockCleanupRecords(pdbbase); asShutdown();