Merge lp:~johill-lanl/epics-base/epicsThreadOnce-atomics-based into lp:~epics-core/epics-base/3.15

[Jeff Hill]

o added new epicsAtomics based, and OS independent, implementation of epicsThreadOnce in epicsThreadOnce.cpp
o changed type of epicsThreadOnceId from epicsThreadId to void *
o fixed once flag isnt initialized with EPICSTHREAD_ONCE_INIT in ipAddrToAsciiAsynchronous.cpp
o removed OS dependent implementations of epicsThreadOnce from {posix, RTEMS, vxWorks, win32}

[lp:~johill-lanl/epics-base/epicsThreadOnce-atomics-based]

12261. By Jeff Hill on 2011-08-31

fixed spelling in comment

[Andrew Johnson (anj)]

Hi Jeff,

The epicsThreadOnce.cpp file has an essential function epicsThreadOnceOnly() which calls cantProceed(), epicsThreadSleep() and errlogPrintf() [the cantProceed() function also calls both of the other two]. Unfortunately both the errlog and generalTime facilities rely on epicsThreadOnce during initialization, thus this implementation could be subject to circular startup problems.

When I try to boot an IOC on vxWorks 6.8 with this code, it hangs silently while executing the C++ static constructors.

If I replace the errlogPrintf() call with std::printf(), after maybe a 10 second delay waiting for the vxWorks munch file to finish loading I get the message "epicsThreadOnce: waiting for another thread to finish calling the once function" which then repeats forever. Here's the stack of the thread loading the munch file:

0x00206b2c ld +0xb4 : usrModuleLoad ()
0x002069e0 usrModuleLoad+0x1c : loadModule ()
0x001da4c0 loadModule +0x24 : loadModuleAt ()
0x001da3d4 loadModuleAt +0xe0 : 0x001da02c ()
0x001da198 loadLibInit +0x21c: cplusLoadFixup ()
0x002141a4 cplusLoadFixup+0x8c : cplusCallCtors ()
0x00143594 cplusCallCtors+0x28 : _GLOBAL__I_iocshPpdbbase ()
0x010b2bc8 _GLOBAL__I_iocshPpdbbase+0x28 : 0x010b2ae8 ()
0x010b2b44 iocshRegister+0x210: iocshRegister ()
0x010b2978 iocshRegister+0x44 : 0x010b0d40 ()
0x010b0d70 gphInitPvt +0x16c: epicsThreadOnce ()
0x010bdfc4 epicsThreadOnce+0xfc : 0x010c59bc (0x8000003c)
0x010c5a44 epicsThreadSleep+0x8c : taskDelay ()

I suspect it may not be possible to fix this in a generic, portable fashion. The current implementations of epicsThreadOnce() are all OS-specific and understand the OS-specific startup requirements. However I'd be happy to test a revised version if you manage to get it working properly on vxWorks.

- Andrew

review: Needs Fixing

Unmerged revisions

12261. By Jeff Hill on 2011-08-31

fixed spelling in comment

12260. By Jeff Hill on 2011-08-31

o moved epicsThreadOnce impl details to anonymous namespace
o fixed recursion sanity test is too agressive
o added some comments

12259. By Jeff Hill on 2011-08-31

o added new epicsAtomics based, and OS independent, implementation of epicsThreadOnce in epicsThreadOnce.cpp
o changed type of epicsThreadOnceId from epicsThreadId to void *
o fixed once flag isnt initialized with EPICSTHREAD_ONCE_INIT in ipAddrToAsciiAsynchronous.cpp
o removed OS dependent implementations of epicsThreadOnce from {posix, RTEMS, vxWorks, win32}

12258. By Jeff Hill <email address hidden> on 2011-08-30

fixed names on redefinition protection macros for vxWorks

12257. By Jeff Hill on 2011-08-30

fixed epics atomic read memory barrier name - old versions of vxWorks

12256. By Jeff Hill on 2011-08-30

fixed word missing from vxWorks specific read and write memory barrier functions

12255. By Jeff Hill <email address hidden> on 2011-08-30

fixed vxWorks name for epicsAtomicTest

12254. By Jeff Hill <email address hidden> on 2011-08-30

fixed wrong return type old vxWorks epicsAtomicUnlock

12253. By Jeff Hill on 2011-08-30

fixed test count

12252. By Jeff Hill on 2011-08-30

fixed vxWorks jumbled ifdef

=== modified file 'src/libCom/Makefile'
--- src/libCom/Makefile	2011-08-31 22:40:22 +0000
+++ src/libCom/Makefile	2011-08-31 22:40:22 +0000
@@ -256,6 +256,7 @@
 SRCS += osdEnv.c
 SRCS += epicsReadline.c
 SRCS += epicsTempFile.cpp
+SRCS += epicsThreadOnce.cpp
 SRCS += epicsStdio.c
 SRCS += osdStdio.c
 
 
=== modified file 'src/libCom/misc/ipAddrToAsciiAsynchronous.cpp'
--- src/libCom/misc/ipAddrToAsciiAsynchronous.cpp	2008-10-21 20:26:48 +0000
+++ src/libCom/misc/ipAddrToAsciiAsynchronous.cpp	2011-08-31 22:40:22 +0000
@@ -127,7 +127,7 @@
 ipAddrToAsciiEnginePrivate * ipAddrToAsciiEnginePrivate :: pEngine = 0;
 unsigned ipAddrToAsciiEnginePrivate :: numberOfReferences = 0u;
 bool ipAddrToAsciiEnginePrivate :: shutdownRequest = false;
-static epicsThreadOnceId ipAddrToAsciiEngineGlobalMutexOnceFlag = 0;
+static epicsThreadOnceId ipAddrToAsciiEngineGlobalMutexOnceFlag = EPICS_THREAD_ONCE_INIT;
 
 // the users are not required to supply a show routine
 // for there transaction callback class
 
=== modified file 'src/libCom/osi/epicsThread.h'
--- src/libCom/osi/epicsThread.h	2010-04-26 20:38:11 +0000
+++ src/libCom/osi/epicsThread.h	2011-08-31 22:40:22 +0000
@@ -51,7 +51,7 @@
 /* (epicsThreadId)0 is guaranteed to be an invalid thread id */
 typedef struct epicsThreadOSD *epicsThreadId;
 
-typedef epicsThreadId epicsThreadOnceId;
+typedef void * epicsThreadOnceId;
 #define EPICS_THREAD_ONCE_INIT 0
 
 epicsShareFunc void epicsShareAPI epicsThreadOnce(
 
=== added file 'src/libCom/osi/epicsThreadOnce.cpp'
--- src/libCom/osi/epicsThreadOnce.cpp	1970-01-01 00:00:00 +0000
+++ src/libCom/osi/epicsThreadOnce.cpp	2011-08-31 22:40:22 +0000
@@ -0,0 +1,129 @@
+
+/*************************************************************************\
+* Copyright (c) 2011 LANS LLC, as Operator of
+*     Los Alamos National Laboratory.
+* Copyright (c) 2011 UChicago Argonne LLC, as Operator of Argonne
+*     National Laboratory.
+* Copyright (c) 2002 The Regents of the University of California, as
+*     Operator of Los Alamos National Laboratory.
+* EPICS BASE is distributed subject to a Software License Agreement found
+* in file LICENSE that is included with this distribution. 
+\*************************************************************************/
+
+/*
+ *  Author Jeffrey O. Hill [e-mail]
+ *  Original Authors (before converting to operating system independent code): 
+ *                      Jeff Hill, Andrew Johnson, Marty Kraimer, Eric Norum
+ */
+
+#include <cstdlib>
+
+#define epicsExportSharedSymbols
+#include "errlog.h"
+#include "cantProceed.h"
+#include "epicsThread.h"
+#include "epicsAtomic.h"
+#include "epicsAssert.h"
+
+namespace {
+    
+struct ThreadID {
+public:
+    ThreadID ();
+    ThreadID ( epicsThreadId id );
+    bool operator == ( const ThreadID & ) const;
+private:
+    epicsThreadId m_threadId;
+    ThreadID ( const ThreadID & ); // disabled
+    ThreadID & operator = ( const ThreadID & ); // disabled
+};
+
+inline ThreadID :: ThreadID ( epicsThreadId id ) : 
+    m_threadId ( id ) 
+{
+}
+
+inline ThreadID :: ThreadID () : m_threadId ( 0 ) 
+{
+}
+
+inline bool ThreadID :: operator == ( const ThreadID & ctrl ) const
+{
+    return ctrl.m_threadId == m_threadId;
+}
+
+static ThreadID done;
+
+} // end of namespace anonymous
+
+using namespace epics;
+using namespace atomic;
+
+/*
+ * epicsThreadOnceOnly ()
+ */
+static void epicsShareAPI epicsThreadOnceOnly ( epicsThreadOnceId * pId, 
+                                                    void ( * pFunc )( void * ), 
+                                                    void * pArg )
+{
+    static const epicsThreadOnceId idStartup = EPICS_THREAD_ONCE_INIT;
+    ThreadID self ( epicsThreadGetIdSelf () );
+    epicsThreadOnceId pCurrent = compareAndSwap ( *pId, idStartup, & self );
+    if ( pCurrent != & done ) {
+        if ( pCurrent == idStartup ) {
+            ( *pFunc ) ( pArg );
+            set ( *pId, & done );
+        }
+        else {
+            {
+                ThreadID & initID = 
+                        * reinterpret_cast < ThreadID * > ( pCurrent );
+                if ( initID == self ) {
+                    cantProceed( "epicsThreadOnce() once was called "
+                                "recursively from the user's once fuction\n" );
+                }
+            }
+            static const std :: size_t spinDownInit = 1000u;
+            static const std :: size_t spinCount = 10u;
+            STATIC_ASSERT ( spinDownInit > spinCount );
+            static const std :: size_t spinThresh = spinDownInit - spinCount;
+            std :: size_t spinDown = spinDownInit;
+            do {
+                if ( spinDown <= spinThresh ) {
+                    epicsThreadSleep ( epicsThreadSleepQuantum () );
+                }
+                if ( spinDown > 0u ) {
+                    spinDown--;
+                }
+                else {
+                    errlogPrintf ( "epicsThreadOnce: waiting for another "
+                            "thread to finish calling the once function\n" );
+                    spinDown = spinThresh;
+                }
+                pCurrent = get ( *pId );
+            } while ( pCurrent != & done );
+        }
+    }
+}
+
+//
+// we implement this as a separate function so that it is easy to see
+// that the performance impact of the primary (most commonly used) path  
+// and that options for inlining this function are available (based on 
+// source code organization)
+//
+// performance might be slightly better if this were implemented as an inline
+// function, but that would pull epicsAtomic.h into epicsThread.h, and on
+// windows this includes (the lean and mean version of) windows.h, so perhaps 
+// its best to make this an out-of-line function as that type of instantiation 
+// will probably have only a small negative impact on performance
+//
+extern "C" void epicsShareAPI epicsThreadOnce ( epicsThreadOnceId * pId, 
+                                                void ( * pFunc )( void * ), 
+                                                void * pArg )
+{
+    const epicsThreadOnceId pCurrent = get ( *pId );
+    if ( pCurrent != & done ) {
+        epicsThreadOnceOnly ( pId, pFunc, pArg );
+    }
+}
 
=== modified file 'src/libCom/osi/os/RTEMS/osdThread.c'
--- src/libCom/osi/os/RTEMS/osdThread.c	2010-10-05 19:27:37 +0000
+++ src/libCom/osi/os/RTEMS/osdThread.c	2011-08-31 22:40:22 +0000
@@ -60,7 +60,6 @@
  * Support for `once-only' execution
  */
 static int initialized = 0;
-static epicsMutexId onceMutex;
 
 /*
  * Just map osi 0 to 99 into RTEMS 199 to 100
@@ -231,7 +230,6 @@
         rtems_task_priority old;
 
         rtems_task_set_priority (RTEMS_SELF, epicsThreadGetOssPriorityValue(99), &old);
-        onceMutex = epicsMutexMustCreate();
         taskVarMutex = epicsMutexMustCreate ();
         rtems_task_ident (RTEMS_SELF, 0, &tid);
         setThreadInfo (tid, "_main_", NULL, NULL);
@@ -471,36 +469,6 @@
 }
 
 /*
- * Ensure func() is run only once.
- */
-void epicsThreadOnce(epicsThreadOnceId *id, void(*func)(void *), void *arg)
-{
-    #define EPICS_THREAD_ONCE_DONE (epicsThreadId) 1
-
-    if (!initialized) epicsThreadInit();
-    epicsMutexMustLock(onceMutex);
-    if (*id != EPICS_THREAD_ONCE_DONE) {
-        if (*id == EPICS_THREAD_ONCE_INIT) { /* first call */
-            *id = epicsThreadGetIdSelf();    /* mark active */
-            epicsMutexUnlock(onceMutex);
-            func(arg);
-            epicsMutexMustLock(onceMutex);
-            *id = EPICS_THREAD_ONCE_DONE;    /* mark done */
-        } else if (*id == epicsThreadGetIdSelf()) {
-            epicsMutexUnlock(onceMutex);
-            cantProceed("Recursive epicsThreadOnce() initialization\n");
-        } else
-            while (*id != EPICS_THREAD_ONCE_DONE) {
-                /* Another thread is in the above func(arg) call. */
-                epicsMutexUnlock(onceMutex);
-                epicsThreadSleep(epicsThreadSleepQuantum());
-                epicsMutexMustLock(onceMutex);
-            }
-    }
-    epicsMutexUnlock(onceMutex);
-}
-
-/*
  * Thread private storage implementation based on the vxWorks
  * implementation by Andrew Johnson APS/ASD.
  */
 
=== modified file 'src/libCom/osi/os/WIN32/osdThread.c'
--- src/libCom/osi/os/WIN32/osdThread.c	2011-02-11 22:33:58 +0000
+++ src/libCom/osi/os/WIN32/osdThread.c	2011-08-31 22:40:22 +0000
@@ -1008,41 +1008,6 @@
 }
 
 /*
- * epicsThreadOnce ()
- */
-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 != EPICS_THREAD_ONCE_DONE ) {
-        if ( *id == EPICS_THREAD_ONCE_INIT ) { /* first call */
-            *id = epicsThreadGetIdSelf();      /* mark active */
-            LeaveCriticalSection ( & pGbl->mutex );
-            func ( arg );
-            EnterCriticalSection ( & pGbl->mutex );
-            *id = EPICS_THREAD_ONCE_DONE;      /* mark done */
-        } else if ( *id == epicsThreadGetIdSelf() ) {
-            LeaveCriticalSection ( & pGbl->mutex );
-            cantProceed( "Recursive epicsThreadOnce() initialization\n" );
-        } else
-            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 );
-}
-
-/*
  * epicsThreadPrivateCreate ()
  */
 epicsShareFunc epicsThreadPrivateId epicsShareAPI epicsThreadPrivateCreate ()
 
=== modified file 'src/libCom/osi/os/posix/osdThread.c'
--- src/libCom/osi/os/posix/osdThread.c	2010-05-14 22:26:54 +0000
+++ src/libCom/osi/os/posix/osdThread.c	2011-08-31 22:40:22 +0000
@@ -76,7 +76,6 @@
 } epicsThreadOSD;
 
 static pthread_key_t getpthreadInfo;
-static pthread_mutex_t onceLock;
 static pthread_mutex_t listLock;
 static ELLLIST pthreadList = ELLLIST_INIT;
 static commonAttr *pcommonAttr = 0;
@@ -207,8 +206,6 @@
     int status;
 
     pthread_key_create(&getpthreadInfo,0);
-    status = pthread_mutex_init(&onceLock,0);
-    checkStatusQuit(status,"pthread_mutex_init","epicsThreadInit");
     status = pthread_mutex_init(&listLock,0);
     checkStatusQuit(status,"pthread_mutex_init","epicsThreadInit");
     pcommonAttr = calloc(1,sizeof(commonAttr));
@@ -321,47 +318,6 @@
 #endif /*_POSIX_THREAD_ATTR_STACKSIZE*/
 }
 
 
-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,"epicsThreadOnce: pthread_mutex_lock returned %s.\n",
-            strerror(status));
-        exit(-1);
-    }
-
-    if (*id != EPICS_THREAD_ONCE_DONE) {
-        if (*id == EPICS_THREAD_ONCE_INIT) { /* first call */
-            *id = epicsThreadGetIdSelf();    /* mark active */
-            status = pthread_mutex_unlock(&onceLock);
-            checkStatusQuit(status,"pthread_mutex_unlock", "epicsThreadOnce");
-            func(arg);
-            status = mutexLock(&onceLock);
-            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 != EPICS_THREAD_ONCE_DONE) {
-                /* Another thread is in the above func(arg) call. */
-                status = pthread_mutex_unlock(&onceLock);
-                checkStatusQuit(status,"pthread_mutex_unlock", "epicsThreadOnce");
-                epicsThreadSleep(epicsThreadSleepQuantum());
-                status = mutexLock(&onceLock);
-                checkStatusQuit(status,"pthread_mutex_lock", "epicsThreadOnce");
-            }
-    }
-    status = pthread_mutex_unlock(&onceLock);
-    checkStatusQuit(status,"pthread_mutex_unlock","epicsThreadOnce");
-}
-
 epicsShareFunc epicsThreadId epicsShareAPI epicsThreadCreate(const char *name,
     unsigned int priority, unsigned int stackSize,
     EPICSTHREADFUNC funptr,void *parm)
 
=== modified file 'src/libCom/osi/os/vxWorks/osdThread.c'
--- src/libCom/osi/os/vxWorks/osdThread.c	2010-08-11 15:45:17 +0000
+++ src/libCom/osi/os/vxWorks/osdThread.c	2011-08-31 22:40:22 +0000
@@ -53,7 +53,6 @@
 static void **papTSD = 0;
 static int nepicsThreadPrivate = 0;
 
-static SEM_ID epicsThreadOnceMutex = 0;
 
 
 /* Just map osi 0 to 99 into vx 100 to 199 */
 /* remember that for vxWorks lower number means higher priority */
@@ -83,19 +82,6 @@
     }
 }
 
-static void epicsThreadInit(void)
-{
-    static int lock = 0;
-
-    while(!vxTas(&lock)) taskDelay(1);
-    if(epicsThreadOnceMutex==0) {
-        epicsThreadOnceMutex = semMCreate(
-                SEM_DELETE_SAFE|SEM_INVERSION_SAFE|SEM_Q_PRIORITY);
-        assert(epicsThreadOnceMutex);
-    }
-    lock = 0;
-}
-
 unsigned int epicsThreadGetStackSize (epicsThreadStackSizeClass stackSizeClass) 
 {
 
@@ -112,43 +98,6 @@
     return stackSizeTable[stackSizeClass];
 }
 
-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 != EPICS_THREAD_ONCE_DONE) {
-        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 = EPICS_THREAD_ONCE_DONE;    /* mark done */
-        } else if (*id == epicsThreadGetIdSelf()) {
-            semGive(epicsThreadOnceMutex);
-            cantProceed("Recursive epicsThreadOnce() initialization\n");
-        } else
-            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);
-}
 
 
 static void createFunction(EPICSTHREADFUNC func, void *parm)
 {
@@ -173,7 +122,6 @@
     EPICSTHREADFUNC funptr,void *parm)
 {
     int tid;
-    if(epicsThreadOnceMutex==0) epicsThreadInit();
     if(stackSize<100) {
         errlogPrintf("epicsThreadCreate %s illegal stackSize %d\n",name,stackSize);
         return(0);
@@ -343,7 +291,6 @@
     static int lock = 0;
     epicsThreadPrivateId id;
 
-    epicsThreadInit();
     /*lock is necessary because ++nepicsThreadPrivate may not be indivisible*/
     while(!vxTas(&lock)) taskDelay(1);
     id = (epicsThreadPrivateId)++nepicsThreadPrivate;