From 939d84f31a16222ba72425f73c16dfab963db204 Mon Sep 17 00:00:00 2001 From: Freddie Akeroyd Date: Sat, 11 Sep 2021 22:10:10 +0100 Subject: [PATCH 01/10] Refactor to use common epicsThreadGetParamWIN32() function --- src/libCom/osi/os/WIN32/osdThread.c | 78 +++++++---------------------- 1 file changed, 19 insertions(+), 59 deletions(-) diff --git a/src/libCom/osi/os/WIN32/osdThread.c b/src/libCom/osi/os/WIN32/osdThread.c index 092432e71..922c098ab 100644 --- a/src/libCom/osi/os/WIN32/osdThread.c +++ b/src/libCom/osi/os/WIN32/osdThread.c @@ -477,21 +477,6 @@ epicsShareFunc unsigned int epicsShareAPI return stackSizeTable[stackSizeClass]; } -void epicsThreadCleanupWIN32 () -{ - win32ThreadGlobal * pGbl = fetchWin32ThreadGlobal (); - win32ThreadParam * pParm; - - if ( ! pGbl ) { - fprintf ( stderr, "epicsThreadCleanupWIN32: unable to find ctx\n" ); - return; - } - - pParm = ( win32ThreadParam * ) - TlsGetValue ( pGbl->tlsIndexThreadLibraryEPICS ); - epicsParmCleanupWIN32 ( pParm ); -} - /* * epicsWin32ThreadEntry() */ @@ -662,13 +647,12 @@ epicsShareFunc epicsThreadId epicsShareAPI epicsThreadCreate (const char *pName, } /* - * epicsThreadSuspendSelf () + * epicsThreadGetParamWIN32 () */ -epicsShareFunc void epicsShareAPI epicsThreadSuspendSelf () +static void* epicsThreadGetParamWIN32 ( void ) { win32ThreadGlobal * pGbl = fetchWin32ThreadGlobal (); win32ThreadParam * pParm; - DWORD stat; assert ( pGbl ); @@ -677,6 +661,18 @@ epicsShareFunc void epicsShareAPI epicsThreadSuspendSelf () if ( ! pParm ) { pParm = epicsThreadImplicitCreate (); } + return pParm; +} + +/* + * epicsThreadSuspendSelf () + */ +epicsShareFunc void epicsShareAPI epicsThreadSuspendSelf () +{ + DWORD stat; + win32ThreadGlobal * pGbl = fetchWin32ThreadGlobal (); + win32ThreadParam * pParm = epicsThreadGetParamWIN32(); + if ( pParm ) { EnterCriticalSection ( & pGbl->mutex ); pParm->isSuspended = 1; @@ -721,16 +717,8 @@ epicsShareFunc unsigned epicsShareAPI epicsThreadGetPriority (epicsThreadId id) */ epicsShareFunc unsigned epicsShareAPI epicsThreadGetPrioritySelf () { - win32ThreadGlobal * pGbl = fetchWin32ThreadGlobal (); - win32ThreadParam * pParm; + win32ThreadParam * pParm = epicsThreadGetParamWIN32(); - assert ( pGbl ); - - pParm = ( win32ThreadParam * ) - TlsGetValue ( pGbl->tlsIndexThreadLibraryEPICS ); - if ( ! pParm ) { - pParm = epicsThreadImplicitCreate (); - } if ( pParm ) { return pParm->epicsPriority; } @@ -791,14 +779,7 @@ epicsShareFunc int epicsShareAPI epicsThreadIsSuspended ( epicsThreadId id ) */ HANDLE osdThreadGetTimer() { - win32ThreadGlobal * pGbl = fetchWin32ThreadGlobal (); - win32ThreadParam * pParm; - - assert ( pGbl ); - - pParm = ( win32ThreadParam * ) - TlsGetValue ( pGbl->tlsIndexThreadLibraryEPICS ); - + win32ThreadParam * pParm = epicsThreadGetParamWIN32(); return pParm->timer; } @@ -880,17 +861,8 @@ double epicsShareAPI epicsThreadSleepQuantum () */ epicsShareFunc epicsThreadId epicsShareAPI epicsThreadGetIdSelf (void) { - win32ThreadGlobal * pGbl = fetchWin32ThreadGlobal (); - win32ThreadParam * pParm; - - assert ( pGbl ); - - pParm = ( win32ThreadParam * ) TlsGetValue ( - pGbl->tlsIndexThreadLibraryEPICS ); - if ( ! pParm ) { - pParm = epicsThreadImplicitCreate (); - assert ( pParm ); /* very dangerous to allow non-unique thread id into use */ - } + win32ThreadParam * pParm = epicsThreadGetParamWIN32(); + assert ( pParm ); /* very dangerous to allow non-unique thread id into use */ return ( epicsThreadId ) pParm; } @@ -927,19 +899,7 @@ epicsShareFunc epicsThreadId epicsShareAPI epicsThreadGetId ( const char * pName */ epicsShareFunc const char * epicsShareAPI epicsThreadGetNameSelf (void) { - win32ThreadGlobal * pGbl = fetchWin32ThreadGlobal (); - win32ThreadParam * pParm; - - if ( ! pGbl ) { - return "thread library not initialized"; - } - - pParm = ( win32ThreadParam * ) - TlsGetValue ( pGbl->tlsIndexThreadLibraryEPICS ); - if ( ! pParm ) { - pParm = epicsThreadImplicitCreate (); - } - + win32ThreadParam * pParm = epicsThreadGetParamWIN32(); if ( pParm ) { if ( pParm->pName ) { return pParm->pName; From 65b34874bdd4af4e32cfb4b7a10be588abf78bab Mon Sep 17 00:00:00 2001 From: Freddie Akeroyd Date: Sat, 11 Sep 2021 22:17:09 +0100 Subject: [PATCH 02/10] Check for NULL in osdThreadGetTimer --- src/libCom/osi/os/WIN32/osdThread.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libCom/osi/os/WIN32/osdThread.c b/src/libCom/osi/os/WIN32/osdThread.c index 922c098ab..59dac832f 100644 --- a/src/libCom/osi/os/WIN32/osdThread.c +++ b/src/libCom/osi/os/WIN32/osdThread.c @@ -780,7 +780,11 @@ epicsShareFunc int epicsShareAPI epicsThreadIsSuspended ( epicsThreadId id ) HANDLE osdThreadGetTimer() { win32ThreadParam * pParm = epicsThreadGetParamWIN32(); - return pParm->timer; + if (pParam) { + return pParm->timer; + } else { + return NULL; + } } /* From 00e9ecf7b582b42bf544eaa1c4981af001ecb12a Mon Sep 17 00:00:00 2001 From: Freddie Akeroyd Date: Sat, 11 Sep 2021 22:19:40 +0100 Subject: [PATCH 03/10] Fix typo in osdThreadGetTimer --- src/libCom/osi/os/WIN32/osdThread.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libCom/osi/os/WIN32/osdThread.c b/src/libCom/osi/os/WIN32/osdThread.c index 59dac832f..e151a7790 100644 --- a/src/libCom/osi/os/WIN32/osdThread.c +++ b/src/libCom/osi/os/WIN32/osdThread.c @@ -780,10 +780,10 @@ epicsShareFunc int epicsShareAPI epicsThreadIsSuspended ( epicsThreadId id ) HANDLE osdThreadGetTimer() { win32ThreadParam * pParm = epicsThreadGetParamWIN32(); - if (pParam) { + if (pParm) { return pParm->timer; } else { - return NULL; + return NULL; } } From c528948f4518f29725873a8f4b87b0c32a41d6f4 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Tue, 14 Sep 2021 18:25:30 -0500 Subject: [PATCH 04/10] Adjust build rules for POD to HTML conversion --- configure/RULES.Db | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/configure/RULES.Db b/configure/RULES.Db index dd0aaaa28..76d46147a 100644 --- a/configure/RULES.Db +++ b/configure/RULES.Db @@ -429,26 +429,26 @@ $(foreach file, $(DBD_INSTALLS), $(eval $(call DBD_INSTALLS_template, $(file)))) ##################################################### HTML files $(COMMON_DIR)/%.html: %.dbd.pod $(TOOLS)/dbdToHtml.pl - @$(RM) $(notdir $@) + @$(RM) $@ $(notdir $@) $(PERL) $(TOOLS)/dbdToHtml.pl $(DBDFLAGS) -o $(notdir $@) $< @$(MV) $(notdir $@) $@ $(COMMON_DIR)/%.html: %.pod $(TOOLS)/podToHtml.pl - @$(RM) $(notdir $@) + @$(RM) $@ $(notdir $@) $(PERL) $(TOOLS)/podToHtml.pl -o $(notdir $@) $< @$(MV) $(notdir $@) $@ $(COMMON_DIR)/%.html: %.pm $(TOOLS)/podToHtml.pl - @$(RM) $(notdir $@) + @$(RM) $@ $(notdir $@) $(PERL) $(TOOLS)/podToHtml.pl -o $(notdir $@) $< @$(MV) $(notdir $@) $@ $(COMMON_DIR)/%.html: ../%.pm $(TOOLS)/podToHtml.pl - @$(RM) $(notdir $@) + @$(RM) $@ $(notdir $@) $(PERL) $(TOOLS)/podToHtml.pl -o $(notdir $@) $< @$(MV) $(notdir $@) $@ -.PRECIOUS: $(COMMON_DIR)/%.html %.html +.PRECIOUS: $(COMMON_DIR)/%.html ##################################################### DB files From e7ea81c7a29631179aaba92567ad62951b8dfd8b Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Tue, 14 Sep 2021 18:26:32 -0500 Subject: [PATCH 05/10] Reference doc tweaks to the dfanout record --- src/std/rec/dfanoutRecord.dbd.pod | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/std/rec/dfanoutRecord.dbd.pod b/src/std/rec/dfanoutRecord.dbd.pod index 790abdffe..7dc221677 100644 --- a/src/std/rec/dfanoutRecord.dbd.pod +++ b/src/std/rec/dfanoutRecord.dbd.pod @@ -99,22 +99,23 @@ SELM is a menu, with three choices: =menu dfanoutSELM -If SELM=="All", then all output links are used, and the values of +If SELM is C, then all output links are used, and the values of SELL and SELN are ignored. -If SELM=="Specified", then the value of SELN is used to specify a single +If SELM is C, then the value of SELN is used to specify a single link which will be used. If SELN==0, then no link will be used; if SELN==1, then OUTA will be used, and so on. -SELN can either have its value set directly, or have its values retrieved -from another EPICS PV. If SELL is a valid PV link, then SELN will be set to -the values of the linked PV. +SELN can either have its value set directly, or have it retrieved from +another EPICS PV. If SELL is a valid PV link, then SELN will be read from +the linked PV. -If SELM=="Mask", then SELN will be treated as a bit mask. If bit one of -SELN is set, then OUTA will be used, if bit two is set, OUTB will be used. -Thus if SELN==5, OUTC and OUTA will be used. +If SELM is C, then SELN will be treated as a bit mask. If bit zero +(the LSB) of SELN is set, then OUTA will be written to; if bit one is set, +OUTB will be written to, and so on. Thus when SELN==5, both OUTC and OUTA +will be written to. -=fields OUTA, OUTB, OUTC, OUTD, OUTE, OUTF, OUTG, OUTH +=fields SELL, SELM, SELN, OUTA, OUTB, OUTC, OUTD, OUTE, OUTF, OUTG, OUTH =head3 Operator Display Parameters From 9842bd1b20fa7b0f868ea21d56471d3af973ae3a Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Thu, 16 Sep 2021 13:21:59 -0500 Subject: [PATCH 06/10] Prefer to install %.html files from O.Common --- configure/RULES_BUILD | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/configure/RULES_BUILD b/configure/RULES_BUILD index 4594fa8be..2cf01a02b 100644 --- a/configure/RULES_BUILD +++ b/configure/RULES_BUILD @@ -498,6 +498,10 @@ $(INSTALL_DOC)/%: ../% $(ECHO) "Installing doc $@" @$(INSTALL) -d -m $(INSTALL_PERMISSIONS) $< $(INSTALL_DOC) +$(INSTALL_HTML)/$(HTMLS_DIR)/%: $(COMMON_DIR)/% + $(ECHO) "Installing generated html $@" + @$(INSTALL) -d -m $(INSTALL_PERMISSIONS) $< $(@D) + $(INSTALL_HTML)/$(HTMLS_DIR)/%: % $(ECHO) "Installing html $@" @$(INSTALL) -d -m $(INSTALL_PERMISSIONS) $< $(@D) @@ -506,10 +510,6 @@ $(INSTALL_HTML)/$(HTMLS_DIR)/%: ../% $(ECHO) "Installing html $@" @$(INSTALL) -d -m $(INSTALL_PERMISSIONS) $< $(@D) -$(INSTALL_HTML)/$(HTMLS_DIR)/%: $(COMMON_DIR)/% - $(ECHO) "Installing generated html $@" - @$(INSTALL) -d -m $(INSTALL_PERMISSIONS) $< $(@D) - $(INSTALL_TEMPLATES_SUBDIR)/%: ../% $(ECHO) "Installing $@" @$(INSTALL) -d -m $(INSTALL_PERMISSIONS) $< $(@D) From 33138606910a30039b7b554371354b1e43dcdd79 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Thu, 16 Sep 2021 14:45:25 -0500 Subject: [PATCH 07/10] Win32 osdThread.c polishing A little optimization of Freddie's code Rename the new non-public routine --- src/libCom/osi/os/WIN32/osdThread.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/libCom/osi/os/WIN32/osdThread.c b/src/libCom/osi/os/WIN32/osdThread.c index e151a7790..7469d360f 100644 --- a/src/libCom/osi/os/WIN32/osdThread.c +++ b/src/libCom/osi/os/WIN32/osdThread.c @@ -647,14 +647,16 @@ epicsShareFunc epicsThreadId epicsShareAPI epicsThreadCreate (const char *pName, } /* - * epicsThreadGetParamWIN32 () + * getMyWin32ThreadParam () */ -static void* epicsThreadGetParamWIN32 ( void ) +static void* getMyWin32ThreadParam ( win32ThreadGlobal * pGbl ) { - win32ThreadGlobal * pGbl = fetchWin32ThreadGlobal (); win32ThreadParam * pParm; - assert ( pGbl ); + if ( ! pGbl ) { + pGbl = fetchWin32ThreadGlobal (); + assert ( pGbl ); + } pParm = ( win32ThreadParam * ) TlsGetValue ( pGbl->tlsIndexThreadLibraryEPICS ); @@ -671,7 +673,7 @@ epicsShareFunc void epicsShareAPI epicsThreadSuspendSelf () { DWORD stat; win32ThreadGlobal * pGbl = fetchWin32ThreadGlobal (); - win32ThreadParam * pParm = epicsThreadGetParamWIN32(); + win32ThreadParam * pParm = getMyWin32ThreadParam ( pGbl ); if ( pParm ) { EnterCriticalSection ( & pGbl->mutex ); @@ -717,7 +719,7 @@ epicsShareFunc unsigned epicsShareAPI epicsThreadGetPriority (epicsThreadId id) */ epicsShareFunc unsigned epicsShareAPI epicsThreadGetPrioritySelf () { - win32ThreadParam * pParm = epicsThreadGetParamWIN32(); + win32ThreadParam * pParm = getMyWin32ThreadParam ( NULL ); if ( pParm ) { return pParm->epicsPriority; @@ -779,7 +781,7 @@ epicsShareFunc int epicsShareAPI epicsThreadIsSuspended ( epicsThreadId id ) */ HANDLE osdThreadGetTimer() { - win32ThreadParam * pParm = epicsThreadGetParamWIN32(); + win32ThreadParam * pParm = getMyWin32ThreadParam ( NULL ); if (pParm) { return pParm->timer; } else { @@ -865,7 +867,7 @@ double epicsShareAPI epicsThreadSleepQuantum () */ epicsShareFunc epicsThreadId epicsShareAPI epicsThreadGetIdSelf (void) { - win32ThreadParam * pParm = epicsThreadGetParamWIN32(); + win32ThreadParam * pParm = getMyWin32ThreadParam ( NULL ); assert ( pParm ); /* very dangerous to allow non-unique thread id into use */ return ( epicsThreadId ) pParm; } @@ -903,7 +905,7 @@ epicsShareFunc epicsThreadId epicsShareAPI epicsThreadGetId ( const char * pName */ epicsShareFunc const char * epicsShareAPI epicsThreadGetNameSelf (void) { - win32ThreadParam * pParm = epicsThreadGetParamWIN32(); + win32ThreadParam * pParm = getMyWin32ThreadParam ( NULL ); if ( pParm ) { if ( pParm->pName ) { return pParm->pName; From 422513990edc65ef9768fc61877c80d7ae4d7a6d Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Thu, 16 Sep 2021 14:54:31 -0500 Subject: [PATCH 08/10] Release notes for Win32 timer fix. --- documentation/RELEASE_NOTES.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/documentation/RELEASE_NOTES.md b/documentation/RELEASE_NOTES.md index 948a17c21..363105e87 100644 --- a/documentation/RELEASE_NOTES.md +++ b/documentation/RELEASE_NOTES.md @@ -6,6 +6,14 @@ This version of EPICS Base has not been released yet. +### Fix timers on MS Windows for non-EPICS threads + +The waitable timer changes in 3.15.9 broke calls to `epicsThreadSleep()` and +similar routines that used timers (including `ca_pend_event()`) when made from +threads that were not started using the epicsThread APIs. +[This problem](https://github.com/epics-base/epics-base/pull/200) +[has now been fixed](https://github.com/epics-base/epics-base/pull/201). + ## Changes made between 3.15.8 and 3.15.9 ### Use waitable timers on Microsoft Windows From 9b69e63a6909b99b73126eb5cd8139ab33c2b3be Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Fri, 17 Sep 2021 10:06:10 -0500 Subject: [PATCH 09/10] Update GHA config, drop Ubuntu-16.04 builds --- .github/workflows/ci-scripts-build.yml | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci-scripts-build.yml b/.github/workflows/ci-scripts-build.yml index 8d8b7ebb1..ebde05573 100644 --- a/.github/workflows/ci-scripts-build.yml +++ b/.github/workflows/ci-scripts-build.yml @@ -17,6 +17,12 @@ on: - '**/*.html' - '**/*.md' pull_request: + paths-ignore: + - 'documentation/*' + - 'startup/*' + - '.appveyor/*' + - '**/*.html' + - '**/*.md' env: SETUP_PATH: .ci-local:.ci @@ -57,11 +63,6 @@ jobs: extra: "CMD_CXXFLAGS=-std=c++11" name: "Ub-20 gcc-9 C++11, static" - - os: ubuntu-16.04 - cmp: clang - configuration: default - name: "Ub-16 clang-9" - - os: ubuntu-20.04 cmp: clang configuration: default @@ -80,18 +81,6 @@ jobs: rtems: "4.9" name: "Ub-20 gcc-9 + RT-4.9" - - os: ubuntu-16.04 - cmp: gcc-4.8 - utoolchain: true - configuration: default - name: "Ub-16 gcc-4.8" - - - os: ubuntu-16.04 - cmp: gcc-4.9 - utoolchain: true - configuration: default - name: "Ub-16 gcc-4.9" - - os: ubuntu-20.04 cmp: gcc-8 utoolchain: true @@ -143,9 +132,12 @@ jobs: - name: Run main module tests run: python .ci/cue.py test - name: Upload tapfiles Artifact + if: ${{ always() }} uses: actions/upload-artifact@v2 with: name: tapfiles ${{ matrix.name }} path: '**/O.*/*.tap' + if-no-files-found: ignore - name: Collect and show test results + if: ${{ always() }} run: python .ci/cue.py test-results From 5cddcea8292ab6418fa527c3539cb01e9a2de3b5 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Fri, 17 Sep 2021 11:16:00 -0500 Subject: [PATCH 10/10] Change the epicsTimerTest delayVerify failure condition This test verifies timer delays between 1.0 and 3.4 seconds. A test failure now means the measured delay was more than 0.25 seconds different than the request, instead of being 5% of the request. This should now pass on GHA macOS. On the 7.0 branch it should call testImpreciseTiming() and use a smaller absolute delay threshold for better targets. --- src/libCom/test/epicsTimerTest.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/libCom/test/epicsTimerTest.cpp b/src/libCom/test/epicsTimerTest.cpp index 55870ed2d..d392a5ec1 100644 --- a/src/libCom/test/epicsTimerTest.cpp +++ b/src/libCom/test/epicsTimerTest.cpp @@ -108,13 +108,14 @@ inline double delayVerify::delay () const double delayVerify::checkError () const { - const double messageThresh = 5.0; // percent - double actualDelay = this->expireStamp - this->beginStamp; - double measuredError = actualDelay - this->expectedDelay; - double percentError = 100.0 * fabs ( measuredError ) / this->expectedDelay; - testOk ( percentError < messageThresh, "%f < %f, delay = %f s, error = %f s (%.1f %%)", - percentError, messageThresh, - this->expectedDelay, measuredError, percentError ); + const double minError = 0.25; + double measuredDelay = this->expireStamp - this->beginStamp; + double measuredError = measuredDelay - this->expectedDelay; + double absoluteError = fabs(measuredError); + double percentError = 100.0 * measuredError / this->expectedDelay; + testOk(absoluteError < minError, + "Delay %.3f s, error = %+.6f ms (%+.3f %%)", + this->expectedDelay, measuredError * 1000, percentError); return measuredError; }