From 3b7e348a8c7f01d8a7a17a2f604819c65f89cc3c Mon Sep 17 00:00:00 2001
From: Michael Davidsaver <mdavidsaver@gmail.com>
Date: Mon, 14 May 2018 10:35:16 -0700
Subject: [PATCH] dbUnitTest.h add callback sync. and global mutex

Add testSyncCallback() to wait for in queued and
in-progress callbacks to complete.

Also add testGlobalLock() to help tests avoid
use after free when destroying sync. primitives.
---
 src/ioc/db/callback.c   | 84 +++++++++++++++++++++++++++++++++++++++++
 src/ioc/db/dbUnitTest.c | 24 ++++++++++++
 src/ioc/db/dbUnitTest.h | 59 +++++++++++++++++++++++++++++
 3 files changed, 167 insertions(+)

diff --git a/src/ioc/db/callback.c b/src/ioc/db/callback.c
index 903ca3f43..b805c1c16 100644
--- a/src/ioc/db/callback.c
+++ b/src/ioc/db/callback.c
@@ -46,6 +46,7 @@
 #include "epicsExport.h"
 #include "link.h"
 #include "recSup.h"
+#include "dbUnitTest.h" /* for testSyncCallback() */
 
 
 static int callbackQueueSize = 2000;
@@ -353,3 +354,86 @@ void callbackRequestProcessCallbackDelayed(CALLBACK *pcallback,
     callbackSetProcess(pcallback, Priority, pRec);
     callbackRequestDelayed(pcallback, seconds);
 }
+
+/* Sync. process of testSyncCallback()
+ *
+ * 1. For each priority, make a call to callbackRequest() for each worker.
+ * 2. Wait until all callbacks are concurrently being executed
+ * 3. Last worker to begin executing signals success and begins waking up other workers
+ * 4. Last worker to wake signals testSyncCallback() to complete
+ */
+typedef struct {
+    epicsEventId wait_phase2, wait_phase4;
+    int nphase2, nphase3;
+    epicsCallback cb;
+} sync_helper;
+
+static void sync_callback(epicsCallback *cb)
+{
+    sync_helper *helper;
+    callbackGetUser(helper, cb);
+
+    testGlobalLock();
+
+    assert(helper->nphase2 > 0);
+    if(--helper->nphase2!=0) {
+        /* we are _not_ the last to start. */
+        testGlobalUnlock();
+        epicsEventMustWait(helper->wait_phase2);
+        testGlobalLock();
+    }
+
+    /* we are either the last to start, or have been
+     * woken by the same and must pass the wakeup along
+     */
+    epicsEventMustTrigger(helper->wait_phase2);
+
+    assert(helper->nphase2 == 0);
+    assert(helper->nphase3 > 0);
+
+    if(--helper->nphase3==0) {
+        /* we are the last to wake up.  wake up testSyncCallback() */
+        epicsEventMustTrigger(helper->wait_phase4);
+    }
+
+    testGlobalUnlock();
+}
+
+void testSyncCallback(void)
+{
+    sync_helper helper[NUM_CALLBACK_PRIORITIES];
+    unsigned i;
+
+    testDiag("Begin testSyncCallback()");
+
+    for(i=0; i<NUM_CALLBACK_PRIORITIES; i++) {
+        helper[i].wait_phase2 = epicsEventMustCreate(epicsEventEmpty);
+        helper[i].wait_phase4 = epicsEventMustCreate(epicsEventEmpty);
+
+        /* no real need to lock here, but do so anyway so that valgrind can establish
+         * the locking requirements for sync_helper.
+         */
+        testGlobalLock();
+        helper[i].nphase2 = helper[i].nphase3 = callbackQueue[i].threadsRunning;
+        testGlobalUnlock();
+
+        callbackSetUser(&helper[i], &helper[i].cb);
+        callbackSetPriority(i, &helper[i].cb);
+        callbackSetCallback(sync_callback, &helper[i].cb);
+
+        callbackRequest(&helper[i].cb);
+    }
+
+    for(i=0; i<NUM_CALLBACK_PRIORITIES; i++) {
+        epicsEventMustWait(helper[i].wait_phase4);
+    }
+
+    for(i=0; i<NUM_CALLBACK_PRIORITIES; i++) {
+        testGlobalLock();
+        epicsEventDestroy(helper[i].wait_phase2);
+        epicsEventDestroy(helper[i].wait_phase4);
+        testGlobalUnlock();
+    }
+
+    testDiag("Complete testSyncCallback()");
+}
diff --git a/src/ioc/db/dbUnitTest.c b/src/ioc/db/dbUnitTest.c
index 249fb6032..6626a79a5 100644
--- a/src/ioc/db/dbUnitTest.c
+++ b/src/ioc/db/dbUnitTest.c
@@ -16,6 +16,7 @@
 #include "epicsUnitTest.h"
 #include "osiFileName.h"
 #include "registry.h"
+#include "epicsThread.h"
 
 #define epicsExportSharedSymbols
 #include "dbAccess.h"
@@ -268,3 +269,26 @@ dbCommon* testdbRecordPtr(const char* pv)
 
     return addr.precord;
 }
+
+static
+epicsMutexId test_global;
+
+static
+epicsThreadOnceId test_global_once = EPICS_THREAD_ONCE_INIT;
+
+static
+void test_global_init(void* ignored)
+{
+    test_global = epicsMutexMustCreate();
+}
+
+void testGlobalLock(void)
+{
+    epicsThreadOnce(&test_global_once, &test_global_init, NULL);
+    epicsMutexMustLock(test_global);
+}
+
+void testGlobalUnlock(void)
+{
+    epicsMutexUnlock(test_global);
+}
diff --git a/src/ioc/db/dbUnitTest.h b/src/ioc/db/dbUnitTest.h
index e1d04e1df..e05e39eaa 100644
--- a/src/ioc/db/dbUnitTest.h
+++ b/src/ioc/db/dbUnitTest.h
@@ -74,6 +74,65 @@ epicsShareFunc void testdbGetArrFieldEqual(const char* pv, short dbfType, long n
 
 epicsShareFunc dbCommon* testdbRecordPtr(const char* pv);
 
+/** Synchronize the shared callback queues.
+ *
+ * Block until all callback queue jobs which were queued, or running,
+ * have completed.
+ */
+epicsShareFunc void testSyncCallback(void);
+
+/** Global mutex for use by test code.
+ *
+ * This utility mutex is intended to be used to avoid races in situations
+ * where some other syncronization primitive is being destroyed (epicsEvent,
+ * epicsMutex, ...).
+ *
+ * For example.  The following has a subtle race where the event may be
+ * destroyed (free()'d) before the call to epicsEventMustSignal() has
+ * returned.  On some targets this leads to a use after free() error.
+ *
+ @code
+ epicsEventId evt;
+ void thread1() {
+   evt = epicsEventMustCreate(...);
+   // spawn thread2()
+   epicsEventMustWait(evt);
+   epicsEventDestroy(evt);
+ }
+ // ...
+ void thread2() {
+   epicsEventMustSignal(evt);
+ }
+ @endcode
+ *
+ * One way to avoid this race is to use a global mutex to ensure
+ * that epicsEventMustSignal() has returned before destroying
+ * the event.
+ *
+ @code
+ epicsEventId evt;
+ void thread1() {
+   evt = epicsEventMustCreate(...);
+   // spawn thread2()
+   epicsEventMustWait(evt);
+   testGlobalLock();   // <-- added
+   epicsEventDestroy(evt);
+   testGlobalUnlock(); // <-- added
+ }
+ // ...
+ void thread2() {
+   testGlobalLock();   // <-- added
+   epicsEventMustSignal(evt);
+   testGlobalUnlock(); // <-- added
+ }
+ @endcode
+ *
+ * This must be a global mutex to avoid simply shifting the race
+ * from the event to a locally allocated mutex.
+ */
+epicsShareFunc void testGlobalLock(void);
+epicsShareFunc void testGlobalUnlock(void);
+
 #ifdef __cplusplus
 }
 #endif