From c5a27fa32e9cccdd68900f633a279cce6fb0be82 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Mon, 26 Apr 2010 15:38:11 -0500 Subject: [PATCH 1/2] Rework epicsThreadOnce() using ideas from Michael Davidsaver. An epicsThreadOnceId is now an epicsThreadId. During initialization, it is set to the thread running the init routine which can now detect a recursive initialization attempt and suspend. EPICS_THREAD_ONCE_INIT is still zero, the implementations now define a new private value for EPICS_THREAD_ONCE_DONE. This is deliberately not made public. --- src/libCom/osi/epicsThread.h | 22 ++++++--------- src/libCom/osi/os/RTEMS/osdThread.c | 21 +++++++++++---- src/libCom/osi/os/WIN32/osdThread.c | 23 +++++++++++----- src/libCom/osi/os/posix/osdThread.c | 39 ++++++++++++++------------- src/libCom/osi/os/vxWorks/osdThread.c | 32 +++++++++++++++++----- 5 files changed, 86 insertions(+), 51 deletions(-) diff --git a/src/libCom/osi/epicsThread.h b/src/libCom/osi/epicsThread.h index d86583c6d..fa0a6bd70 100644 --- a/src/libCom/osi/epicsThread.h +++ b/src/libCom/osi/epicsThread.h @@ -48,23 +48,17 @@ typedef enum { epicsShareFunc unsigned int epicsShareAPI epicsThreadGetStackSize( epicsThreadStackSizeClass size); -typedef int epicsThreadOnceId; -#define EPICS_THREAD_ONCE_INIT 0 - -/* void epicsThreadOnce(epicsThreadOnceId *id, EPICSTHREADFUNC, void *arg); */ -/* epicsThreadOnce is implemented as a macro */ -/* epicsThreadOnceOsd should not be called by user code */ -epicsShareFunc void epicsShareAPI epicsThreadOnceOsd( - epicsThreadOnceId *id, EPICSTHREADFUNC, void *arg); - -#define epicsThreadOnce(id,func,arg) \ -epicsThreadOnceOsd((id),(func),(arg)) - -epicsShareFunc void epicsShareAPI epicsThreadExitMain(void); - /* (epicsThreadId)0 is guaranteed to be an invalid thread id */ typedef struct epicsThreadOSD *epicsThreadId; +typedef epicsThreadId epicsThreadOnceId; +#define EPICS_THREAD_ONCE_INIT 0 + +epicsShareFunc void epicsShareAPI epicsThreadOnce( + epicsThreadOnceId *id, EPICSTHREADFUNC, void *arg); + +epicsShareFunc void epicsShareAPI epicsThreadExitMain(void); + epicsShareFunc epicsThreadId epicsShareAPI epicsThreadCreate ( const char * name, unsigned int priority, unsigned int stackSize, EPICSTHREADFUNC funptr,void * parm ); diff --git a/src/libCom/osi/os/RTEMS/osdThread.c b/src/libCom/osi/os/RTEMS/osdThread.c index 29feacb99..a5cf629d4 100644 --- a/src/libCom/osi/os/RTEMS/osdThread.c +++ b/src/libCom/osi/os/RTEMS/osdThread.c @@ -473,18 +473,29 @@ epicsThreadId epicsThreadGetId (const char *name) /* * Ensure func() is run only once. */ -void epicsThreadOnceOsd(epicsThreadOnceId *id, void(*func)(void *), void *arg) +void epicsThreadOnce(epicsThreadOnceId *id, void(*func)(void *), void *arg) { + static struct epicsThreadOSD threadOnceComplete; + #define EPICS_THREAD_ONCE_DONE &threadOnceComplete + if (!initialized) epicsThreadInit(); epicsMutexMustLock(onceMutex); - if (*id == 0) { - *id = -1; + if (*id == EPICS_THREAD_ONCE_INIT) { /* first call */ + *id = epicsThreadGetIdSelf(); /* mark active */ epicsMutexUnlock(onceMutex); func(arg); epicsMutexMustLock(onceMutex); - *id = 1; + *id = EPICS_THREAD_ONCE_DONE; /* mark done */ + } else if (*id == epicsThreadGetIdSelf()) { + epicsMutexUnlock(onceMutex); + cantProceed("Recursive epicsThreadOnce() initialization\n"); } else - assert(*id > 0 /* func() called epicsThreadOnce() with same id */); + while (*id != EPICS_THREAD_ONCE_DONE) { + /* Another thread is in the above func(arg) call. */ + epicsMutexUnlock(onceMutex); + epicsThreadSleep(epicsThreadSleepQuantum()); + epicsMutexMustLock(onceMutex); + } epicsMutexUnlock(onceMutex); } diff --git a/src/libCom/osi/os/WIN32/osdThread.c b/src/libCom/osi/os/WIN32/osdThread.c index 1d9cdd4b1..50002b4e9 100644 --- a/src/libCom/osi/os/WIN32/osdThread.c +++ b/src/libCom/osi/os/WIN32/osdThread.c @@ -997,24 +997,33 @@ epicsShareFunc void epicsShareAPI epicsThreadShow ( epicsThreadId id, unsigned l /* * epicsThreadOnce () */ -epicsShareFunc void epicsShareAPI epicsThreadOnceOsd ( +epicsShareFunc void epicsShareAPI epicsThreadOnce ( epicsThreadOnceId *id, void (*func)(void *), void *arg ) { + static struct epicsThreadOSD threadOnceComplete; + #define EPICS_THREAD_ONCE_DONE & threadOnceComplete win32ThreadGlobal * pGbl = fetchWin32ThreadGlobal (); assert ( pGbl ); EnterCriticalSection ( & pGbl->mutex ); - if ( *id == 0 ) { - *id = -1; + if ( *id == EPICS_THREAD_ONCE_INIT ) { /* first call */ + *id = epicsThreadGetIdSelf(); /* mark active */ LeaveCriticalSection ( & pGbl->mutex ); - ( *func ) ( arg ); + func ( arg ); EnterCriticalSection ( & pGbl->mutex ); - *id = 1; + *id = EPICS_THREAD_ONCE_DONE; /* mark done */ + } else if ( *id == epicsThreadGetIdSelf() ) { + LeaveCriticalSection ( & pGbl->mutex ); + cantProceed( "Recursive epicsThreadOnce() initialization\n" ); } else - assert(*id > 0 /* func() called epicsThreadOnce() with same id */); - + while ( *id != EPICS_THREAD_ONCE_DONE ) { + /* Another thread is in the above func(arg) call. */ + LeaveCriticalSection ( & pGbl->mutex ); + epicsThreadSleep ( epicsThreadSleepQuantum() ); + EnterCriticalSection ( & pGbl->mutex ); + } LeaveCriticalSection ( & pGbl->mutex ); } diff --git a/src/libCom/osi/os/posix/osdThread.c b/src/libCom/osi/os/posix/osdThread.c index 5d101f9a1..073bdb626 100644 --- a/src/libCom/osi/os/posix/osdThread.c +++ b/src/libCom/osi/os/posix/osdThread.c @@ -321,40 +321,43 @@ epicsShareFunc unsigned int epicsShareAPI epicsThreadGetStackSize (epicsThreadSt #endif /*_POSIX_THREAD_ATTR_STACKSIZE*/ } -/* epicsThreadOnce is a macro that calls epicsThreadOnceOsd */ -epicsShareFunc void epicsShareAPI epicsThreadOnceOsd(epicsThreadOnceId *id, void (*func)(void *), void *arg) +epicsShareFunc void epicsShareAPI epicsThreadOnce(epicsThreadOnceId *id, void (*func)(void *), void *arg) { + static struct epicsThreadOSD threadOnceComplete; + #define EPICS_THREAD_ONCE_DONE &threadOnceComplete int status; + epicsThreadInit(); status = mutexLock(&onceLock); if(status) { - fprintf(stderr,"epicsThreadOnceOsd: pthread_mutex_lock returned %s.\n", + fprintf(stderr,"epicsThreadOnce: pthread_mutex_lock returned %s.\n", strerror(status)); exit(-1); } - if (*id == 0) { /* 0 => first call */ - *id = -1; /* -1 => func() active */ - /* avoid recursive locking */ + + if (*id == EPICS_THREAD_ONCE_INIT) { /* first call */ + *id = epicsThreadGetIdSelf(); /* mark active */ status = pthread_mutex_unlock(&onceLock); - checkStatusQuit(status,"pthread_mutex_unlock","epicsThreadOnceOsd"); + checkStatusQuit(status,"pthread_mutex_unlock", "epicsThreadOnce"); func(arg); status = mutexLock(&onceLock); - checkStatusQuit(status,"pthread_mutex_lock","epicsThreadOnceOsd"); - *id = +1; /* +1 => func() done */ + checkStatusQuit(status,"pthread_mutex_lock", "epicsThreadOnce"); + *id = EPICS_THREAD_ONCE_DONE; /* mark done */ + } else if (*id == epicsThreadGetIdSelf()) { + status = pthread_mutex_unlock(&onceLock); + checkStatusQuit(status,"pthread_mutex_unlock", "epicsThreadOnce"); + cantProceed("Recursive epicsThreadOnce() initialization\n"); } else - while (*id < 0) { - /* Someone is in the above func(arg) call. If that someone is - * actually us, we're screwed, but the other OS implementations - * will fire an assert() that should detect this condition. - */ + while (*id != EPICS_THREAD_ONCE_DONE) { + /* Another thread is in the above func(arg) call. */ status = pthread_mutex_unlock(&onceLock); - checkStatusQuit(status,"pthread_mutex_unlock","epicsThreadOnceOsd"); - epicsThreadSleep(0.01); + checkStatusQuit(status,"pthread_mutex_unlock", "epicsThreadOnce"); + epicsThreadSleep(epicsThreadSleepQuantum()); status = mutexLock(&onceLock); - checkStatusQuit(status,"pthread_mutex_lock","epicsThreadOnceOsd"); + checkStatusQuit(status,"pthread_mutex_lock", "epicsThreadOnce"); } status = pthread_mutex_unlock(&onceLock); - checkStatusQuit(status,"pthread_mutex_unlock","epicsThreadOnceOsd"); + checkStatusQuit(status,"pthread_mutex_unlock","epicsThreadOnce"); } epicsShareFunc epicsThreadId epicsShareAPI epicsThreadCreate(const char *name, diff --git a/src/libCom/osi/os/vxWorks/osdThread.c b/src/libCom/osi/os/vxWorks/osdThread.c index 4bda3cb52..9542778e2 100644 --- a/src/libCom/osi/os/vxWorks/osdThread.c +++ b/src/libCom/osi/os/vxWorks/osdThread.c @@ -110,21 +110,39 @@ unsigned int epicsThreadGetStackSize (epicsThreadStackSizeClass stackSizeClass) return stackSizeTable[stackSizeClass]; } -void epicsThreadOnceOsd(epicsThreadOnceId *id, void (*func)(void *), void *arg) +struct epicsThreadOSD {}; + /* Strictly speaking this should be a WIND_TCB, but we only need it to + * be able to create an epicsThreadId that is guaranteed never to be + * the same as any current TID, and since TIDs are pointers this works. + */ + +void epicsThreadOnce(epicsThreadOnceId *id, void (*func)(void *), void *arg) { + static struct epicsThreadOSD threadOnceComplete; + #define EPICS_THREAD_ONCE_DONE &threadOnceComplete int result; + epicsThreadInit(); result = semTake(epicsThreadOnceMutex, WAIT_FOREVER); assert(result == OK); - if (*id == 0) { /* 0 => first call */ - *id = -1; /* -1 => func() active */ + if (*id == EPICS_THREAD_ONCE_INIT) { /* first call */ + *id = epicsThreadGetIdSelf(); /* mark active */ semGive(epicsThreadOnceMutex); func(arg); result = semTake(epicsThreadOnceMutex, WAIT_FOREVER); assert(result == OK); - *id = +1; /* +1 => func() done */ + *id = EPICS_THREAD_ONCE_DONE; /* mark done */ + } else if (*id == epicsThreadGetIdSelf()) { + semGive(epicsThreadOnceMutex); + cantProceed("Recursive epicsThreadOnce() initialization\n"); } else - assert(*id > 0 /* func() called epicsThreadOnce() with same id */); + while (*id != EPICS_THREAD_ONCE_DONE) { + /* Another thread is in the above func(arg) call. */ + semGive(epicsThreadOnceMutex); + epicsThreadSleep(epicsThreadSleepQuantum()); + result = semTake(epicsThreadOnceMutex, WAIT_FOREVER); + assert(result == OK); + } semGive(epicsThreadOnceMutex); } @@ -132,13 +150,13 @@ static void createFunction(EPICSTHREADFUNC func, void *parm) { int tid = taskIdSelf(); - taskVarAdd(tid,(int *)&papTSD); + taskVarAdd(tid,(int *)(char *)&papTSD); /*Make sure that papTSD is still 0 after that call to taskVarAdd*/ papTSD = 0; (*func)(parm); epicsExitCallAtThreadExits (); free(papTSD); - taskVarDelete(tid,(int *)&papTSD); + taskVarDelete(tid,(int *)(char *)&papTSD); } #ifdef ALTIVEC From 6ece3235c96315334d9cfe493be94e3941fa4e61 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Mon, 26 Apr 2010 15:48:42 -0500 Subject: [PATCH 2/2] Added libCom/test code for epicsThreadOnce() implementations. Also fixed subsystems that were not using epicsThreadOnce correctly. --- src/libCom/error/errlog.c | 2 +- src/libCom/osi/os/vxWorks/osdPoolStatus.c | 26 ++--- src/libCom/test/Makefile | 5 + src/libCom/test/epicsRunLibComTests.c | 6 +- src/libCom/test/epicsThreadOnceTest.c | 110 ++++++++++++++++++++++ 5 files changed, 127 insertions(+), 22 deletions(-) create mode 100644 src/libCom/test/epicsThreadOnceTest.c diff --git a/src/libCom/error/errlog.c b/src/libCom/error/errlog.c index bfae98fc4..40cbb7bc1 100644 --- a/src/libCom/error/errlog.c +++ b/src/libCom/error/errlog.c @@ -445,7 +445,7 @@ epicsShareFunc int epicsShareAPI errlogInit2(int bufsize, int maxMsgSize) static epicsThreadOnceId errlogOnceFlag = EPICS_THREAD_ONCE_INIT; struct initArgs config; - if (errlogOnceFlag > 0 && pvtData.atExit) + if (pvtData.atExit) return 0; if (bufsize < BUFFER_SIZE) bufsize = BUFFER_SIZE; diff --git a/src/libCom/osi/os/vxWorks/osdPoolStatus.c b/src/libCom/osi/os/vxWorks/osdPoolStatus.c index f60aedb07..c8418a236 100644 --- a/src/libCom/osi/os/vxWorks/osdPoolStatus.c +++ b/src/libCom/osi/os/vxWorks/osdPoolStatus.c @@ -3,8 +3,7 @@ * National Laboratory. * Copyright (c) 2002 The Regents of the University of California, as * Operator of Los Alamos National Laboratory. -* EPICS BASE Versions 3.13.7 -* and higher are distributed subject to a Software License Agreement found +* EPICS BASE is distributed subject to a Software License Agreement found * in file LICENSE that is included with this distribution. \*************************************************************************/ @@ -16,7 +15,7 @@ #include "osiPoolStatus.h" /* - * It turns out that memPartInfoGet() nad memFindMax() are very CPU intensive on vxWorks + * It turns out that memPartInfoGet() and memFindMax() are very CPU intensive on vxWorks * so we must spawn off a thread that periodically polls. Although this isnt 100% safe, I * dont see what else to do. * @@ -32,19 +31,15 @@ static size_t osdMaxBlockSize = 0; static void osdSufficentSpaceInPoolQuery () { int temp = memFindMax (); - if ( temp > 0 ) { - osdMaxBlockSize = (size_t) temp; - } - else { - osdMaxBlockSize = 0; - } + + osdMaxBlockSize = ( temp > 0 ) ? (size_t) temp : 0; } static void osdSufficentSpaceInPoolPoll ( void *pArgIn ) { while ( 1 ) { - osdSufficentSpaceInPoolQuery (); epicsThreadSleep ( 1.0 ); + osdSufficentSpaceInPoolQuery (); } } @@ -54,14 +49,9 @@ static void osdSufficentSpaceInPoolInit ( void *pArgIn ) osdSufficentSpaceInPoolQuery (); - id = epicsShareAPI epicsThreadCreate ( "poolPoll", epicsThreadPriorityMedium, - epicsThreadGetStackSize ( epicsThreadStackSmall ), osdSufficentSpaceInPoolPoll, 0 ); - if ( id ) { - osdMaxBlockOnceler = 1; - } - else { - epicsThreadSleep ( 0.1 ); - } + id = epicsThreadCreate ( "poolPoll", epicsThreadPriorityMedium, + epicsThreadGetStackSize ( epicsThreadStackSmall ), + osdSufficentSpaceInPoolPoll, 0 ); } /* diff --git a/src/libCom/test/Makefile b/src/libCom/test/Makefile index d348b8ae8..255d07fd1 100644 --- a/src/libCom/test/Makefile +++ b/src/libCom/test/Makefile @@ -57,6 +57,11 @@ epicsThreadTest_SRCS += epicsThreadTest.cpp testHarness_SRCS += epicsThreadTest.cpp TESTS += epicsThreadTest +TESTPROD_HOST += epicsThreadOnceTest +epicsThreadOnceTest_SRCS += epicsThreadOnceTest.c +testHarness_SRCS += epicsThreadOnceTest.c +TESTS += epicsThreadOnceTest + TESTPROD_HOST += epicsThreadPriorityTest epicsThreadPriorityTest_SRCS += epicsThreadPriorityTest.cpp testHarness_SRCS += epicsThreadPriorityTest.cpp diff --git a/src/libCom/test/epicsRunLibComTests.c b/src/libCom/test/epicsRunLibComTests.c index a27dd6bc0..9445c99cb 100644 --- a/src/libCom/test/epicsRunLibComTests.c +++ b/src/libCom/test/epicsRunLibComTests.c @@ -7,9 +7,6 @@ /* * Run libCom tests as a batch - * - * This is part of the work being done to provide a unified set of automated - * tests for EPICS. Many more changes will be forthcoming. */ #include #include @@ -27,6 +24,7 @@ int epicsMessageQueueTest(void); int epicsMutexTest(void); int epicsStdioTest(void); int epicsStringTest(void); +int epicsThreadOnceTest(void); int epicsThreadPriorityTest(void); int epicsThreadPrivateTest(void); int epicsTimeTest(void); @@ -73,6 +71,8 @@ void epicsRunLibComTests(void) runTest(epicsStringTest); + runTest(epicsThreadOnceTest); + runTest(epicsThreadPriorityTest); runTest(epicsThreadPrivateTest); diff --git a/src/libCom/test/epicsThreadOnceTest.c b/src/libCom/test/epicsThreadOnceTest.c new file mode 100644 index 000000000..0b1398a34 --- /dev/null +++ b/src/libCom/test/epicsThreadOnceTest.c @@ -0,0 +1,110 @@ +/*************************************************************************\ +* Copyright (c) 2010 UChicago Argonne LLC, as Operator of Argonne +* National Laboratory. +* EPICS BASE is distributed subject to a Software License Agreement found +* in file LICENSE that is included with this distribution. +\*************************************************************************/ +/* $Id$ */ + +#include +#include + +#include "epicsEvent.h" +#include "epicsExit.h" +#include "epicsMutex.h" +#include "epicsThread.h" +#include "epicsUnitTest.h" +#include "testMain.h" + +#define NUM_ONCE_THREADS 8 + +epicsThreadOnceId onceFlag = EPICS_THREAD_ONCE_INIT; +epicsThreadOnceId twiceFlag = EPICS_THREAD_ONCE_INIT; +epicsMutexId lock; +epicsEventId go; + +int runCount = 0; +int initCount = 0; +char initBy[20]; +int doneCount = 0; + +void onceInit(void *ctx) +{ + initCount++; + strcpy(initBy, epicsThreadGetNameSelf()); +} + +void onceThread(void *ctx) +{ + epicsMutexMustLock(lock); + runCount++; + epicsMutexUnlock(lock); + + epicsEventMustWait(go); + epicsEventSignal(go); + + epicsThreadOnce(&onceFlag, onceInit, ctx); + testOk(initCount == 1, "%s: initCount = %d", + epicsThreadGetNameSelf(), initCount); + + epicsMutexMustLock(lock); + doneCount++; + epicsMutexUnlock(lock); +} + + +void recurseInit(void); +void onceRecurse(void *ctx) +{ + recurseInit(); +} + +void recurseInit(void) +{ + epicsThreadOnce(&twiceFlag, onceRecurse, 0); +} + +void recurseThread(void *ctx) +{ + recurseInit(); + testFail("Recursive epicsThreadOnce() not detected"); +} + + +MAIN(epicsThreadOnceTest) +{ + int i; + epicsThreadId tid; + + testPlan(3 + NUM_ONCE_THREADS); + + go = epicsEventMustCreate(epicsEventEmpty); + lock = epicsMutexMustCreate(); + + for (i = 0; i < NUM_ONCE_THREADS; i++) { + char name[20]; + + sprintf(name, "once-%d", i); + epicsThreadCreate(name, epicsThreadPriorityMedium, + epicsThreadGetStackSize(epicsThreadStackSmall), + onceThread, 0); + } + epicsThreadSleep(0.1); + + testOk(runCount == NUM_ONCE_THREADS, "runCount = %d", runCount); + epicsEventSignal(go); + epicsThreadSleep(0.1); + + testOk(doneCount == NUM_ONCE_THREADS, "doneCount = %d", doneCount); + testDiag("init was run by %s", initBy); + + tid = epicsThreadCreate("recurse", epicsThreadPriorityMedium, + epicsThreadGetStackSize(epicsThreadStackSmall), + recurseThread, 0); + do { + epicsThreadSleep(0.1); + } while (!epicsThreadIsSuspended(tid)); + testPass("Recursive epicsThreadOnce() detected"); + + return testDone(); +}