Rewrite of epicsThreadOnce(); detects recursion on all archs.

documentation/RELEASE_NOTES.html

@@ -12,6 +12,13 @@
 <h2 align="center">Changes between 3.14.11 and 3.14.12</h2>
 <!-- Insert new items immediately below here ... -->
 
+<h4>Rewrite epicsThreadOnce()</h4>
+
+<p>Michael Davidsaver suggested a better implementation of epicsThreadOnce()
+which makes it possible for all architectures to detect recursive initialization
+functions.  This change also renames the routines to remove the original macro
+and fixes some questionable usage in places.</p>
+
 <h4>Many Record Types</h4>
 
 <p>Record types were not setting many 'last value' fields at initialization,

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;

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 );

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);
 }
 

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 );
 }
 

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,

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 );
 }
 
 /*

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

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

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 <stdio.h>
 #include <epicsThread.h>
@@ -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);

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 <stdio.h>
+#include <string.h>
+
+#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();
+}