From a80bd1a63095eb866baa712795f62230475ce498 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 8 Jul 2011 11:18:00 -0500 Subject: [PATCH] libCom: Avoid race in errlog shutdown. A rare race during shutdown. The contenders are the log thread coming out of its loop and calling errlogCleanup(), and the exitHandler signaling waitForWork. This solution is to move cleanup completely into exitHandler, which already waits for the log thread to exit. --- documentation/RELEASE_NOTES.html | 55 +++++++++++++++++++------------- src/libCom/error/errlog.c | 23 +++++-------- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/documentation/RELEASE_NOTES.html b/documentation/RELEASE_NOTES.html index 37224ed9f..a527844fb 100644 --- a/documentation/RELEASE_NOTES.html +++ b/documentation/RELEASE_NOTES.html @@ -13,51 +13,58 @@ +

Another race condition in errlog cleaned up

+ +

If it was still busy when the IOC was closed down, the errlog thread could +have preempted the exit handler and freed the various internal pvtData mutex and +event objects too soon.

+

Top-level make target changes

-

Several make targets have been changed. Note that these can only be used from an -application's <top> directory.

+

Several make targets have been changed. Note that these can only be used from +an application's <top> directory.

make uninstall.<arch>
-
Deletes the bin/<arch> and lib/<arch> directories for <arch> - only. Note that <arch> does not have to be an architecture that this host is - configured to build, it works for any arch.
+
Deletes the bin/<arch> and lib/<arch> directories for + <arch> only. Note that <arch> does not have to be an + architecture that this host is configured to build, it works for any + arch.
make archuninstall
Deletes the bin/<arch> and lib/<arch> directories for all - architectures that this host is configured to build. Should not affect files used - for multiple architectures, or for host or target architectures that this host is - not configured to build.
+ architectures that this host is configured to build. Should not affect files + used for multiple architectures, or for host or target architectures that + this host is not configured to build.
make uninstall
-
Does archuninstall and also deletes the other install directories include, db, - dbd, doc, html, templates and java. This will affect subsequent builds for other - architectures, but it doesn't delete their bin/<arch> or lib/<arch> - contents.
+
Does archuninstall and also deletes the other install directories include, + db, dbd, doc, html, templates and java. This will affect subsequent builds + for other architectures, but it doesn't delete their bin/<arch> or + lib/<arch> contents.
make realuninstall
Deletes all install directories for all architectures.
make distclean
-
Does realclean realuninstall as before, and also now does a cvsclean, which - removes file remnants from CVS operations named .#* and editor - backups named *~ throughout the source tree.
+
Does realclean realuninstall as before, and also now does a cvsclean, + which removes file remnants from CVS operations named .#* and + editor backups named *~ throughout the source tree.

Compress record type

-

This record now posts monitors on its NUSE field whenever its value changes. A new -field OUSE was added to support this.

+

This record now posts monitors on its NUSE field whenever its value changes. +A new field OUSE was added to support this.

Remove C++ build rule for .C files

An early convention on Unix systems was to name C++ files with an upper-case extention, .C. This does not work on Windows or MacOS where the -filesystems are case-insensitive, and the C++ build rule was causing problems so has -been eliminated. Any remaining C++ source files that are still using this convention -will have to be renamed, preferably to .cpp

+filesystems are case-insensitive, and the C++ build rule was causing problems so +has been eliminated. Any remaining C++ source files that are still using this +convention will have to be renamed, preferably to .cpp

Support make -s on Windows

@@ -66,9 +73,11 @@ combinations. This has now been fixed.

iocLogServer now supports logrotate

-

The feature in the iocLogServer that closed and reopened the logfile used to ignore -the SIGHUP signal when the log filename did not change. This has now been changed so -these logfiles can be used with the standard Linux logrotate package.

+

The feature in the iocLogServer that closed and reopened the logfile used to +ignore the SIGHUP signal when the log filename did not change. This has now been +changed so these logfiles can be used with the standard Linux logrotate +package.

+

Changes between 3.14.12 and 3.14.12.1

diff --git a/src/libCom/error/errlog.c b/src/libCom/error/errlog.c index 1cf1575bf..c5834057b 100644 --- a/src/libCom/error/errlog.c +++ b/src/libCom/error/errlog.c @@ -41,7 +41,6 @@ /*Declare storage for errVerbose */ epicsShareDef int errVerbose = 0; -static void errlogCleanup(void); static void exitHandler(void *); static void errlogThread(void); @@ -391,8 +390,15 @@ static void exitHandler(void *pvt) pvtData.atExit = 1; epicsEventSignal(pvtData.waitForWork); epicsEventMustWait(pvtData.waitForExit); + + free(pvtData.pbuffer); + epicsMutexDestroy(pvtData.flushLock); + epicsEventDestroy(pvtData.flush); + epicsEventDestroy(pvtData.waitForFlush); + epicsMutexDestroy(pvtData.listenerLock); + epicsMutexDestroy(pvtData.msgQueueLock); + epicsEventDestroy(pvtData.waitForWork); epicsEventDestroy(pvtData.waitForExit); - return; } struct initArgs { @@ -432,18 +438,6 @@ static void errlogInitPvt(void *arg) pvtData.errlogInitFailed = FALSE; } } - -static void errlogCleanup(void) -{ - free(pvtData.pbuffer); - epicsMutexDestroy(pvtData.flushLock); - epicsEventDestroy(pvtData.flush); - epicsEventDestroy(pvtData.waitForFlush); - epicsMutexDestroy(pvtData.listenerLock); - epicsMutexDestroy(pvtData.msgQueueLock); - epicsEventDestroy(pvtData.waitForWork); - /*Note that exitHandler must destroy waitForExit*/ -} epicsShareFunc int epicsShareAPI errlogInit2(int bufsize, int maxMsgSize) { @@ -514,7 +508,6 @@ static void errlogThread(void) epicsThreadSleep(.2); /*just wait an extra .2 seconds*/ epicsEventSignal(pvtData.waitForFlush); } - errlogCleanup(); epicsEventSignal(pvtData.waitForExit); }