diff --git a/ADApp/ADSrc/asynNDArrayDriver.cpp b/ADApp/ADSrc/asynNDArrayDriver.cpp index ce1fdb8c4..3a5b479ad 100644 --- a/ADApp/ADSrc/asynNDArrayDriver.cpp +++ b/ADApp/ADSrc/asynNDArrayDriver.cpp @@ -1022,13 +1022,30 @@ asynNDArrayDriver::asynNDArrayDriver(const char *portName, int maxAddr, int maxB } +// When the driver subclass is destructible, this function will be called at IOC +// shutdown. +void asynNDArrayDriver::shutdownPortDriver() { + asynPrint(pasynUserSelf, ASYN_TRACE_FLOW, "%s shutting down\n", driverName); -asynNDArrayDriver::~asynNDArrayDriver() -{ queuedArrayUpdateRun_ = false; epicsEventSignal(queuedArrayEvent_); epicsEventWait(queuedArrayUpdateDone_); +#ifdef ASYN_DESTRUCTIBLE + asynPortDriver::shutdownPortDriver(); +#endif +} + +asynNDArrayDriver::~asynNDArrayDriver() +{ + // If the driver subclass is not destructible, or asyn is old, or we are not + // in an IOC (e.g. unit tests), we need to call shutdown ourselves. + // On newer versions of asyn, we could check with needsShutdown() to see if + // shutdown has already been done, be we don't want to rely on that. + if (queuedArrayUpdateRun_) { + shutdownPortDriver(); + } + delete this->pNDArrayPoolPvt_; free(this->pArrays); delete this->pAttributeList; diff --git a/ADApp/ADSrc/asynNDArrayDriver.h b/ADApp/ADSrc/asynNDArrayDriver.h index 5d12ad971..6ae6a9567 100644 --- a/ADApp/ADSrc/asynNDArrayDriver.h +++ b/ADApp/ADSrc/asynNDArrayDriver.h @@ -148,6 +148,7 @@ class ADCORE_API asynNDArrayDriver : public asynPortDriver { virtual asynStatus setIntegerParam(int index, int value); virtual asynStatus setIntegerParam(int list, int index, int value); virtual void report(FILE *fp, int details); + virtual void shutdownPortDriver(); /* These are the methods that are new to this class */ virtual asynStatus createFilePath(const char *path, int pathDepth); diff --git a/ADApp/pluginSrc/NDPluginDriver.cpp b/ADApp/pluginSrc/NDPluginDriver.cpp index 49d3c336f..211315c84 100644 --- a/ADApp/pluginSrc/NDPluginDriver.cpp +++ b/ADApp/pluginSrc/NDPluginDriver.cpp @@ -169,17 +169,33 @@ NDPluginDriver::NDPluginDriver(const char *portName, int queueSize, int blocking unlock(); } +// When the plugin subclass is destructible, this function will be called at IOC +// shutdown. +void NDPluginDriver::shutdownPortDriver() { + asynPrint(pasynUserSelf, ASYN_TRACE_FLOW, "%s shutting down\n", driverName); + + // Most methods in NDPluginDriver expect to be called with the asynPortDriver mutex locked. + // Shutdown does not, the mutex should be unlocked before it is called. + // We lock the mutex because deleteCallbackThreads expects it to be held, but then + // unlocked it because the mutex is deleted in the asynPortDriver destructor and the + // mutex must be unlocked before deleting it. + this->lock(); + deleteCallbackThreads(); + this->unlock(); + + asynNDArrayDriver::shutdownPortDriver(); +} + NDPluginDriver::~NDPluginDriver() { - // Most methods in NDPluginDriver expect to be called with the asynPortDriver mutex locked. - // The destructor does not, the mutex should be unlocked before calling the destructor. - // We lock the mutex because deleteCallbackThreads expects it to be held, but then - // unlocked it because the mutex is deleted in the asynPortDriver destructor and the - // mutex must be unlocked before deleting it. - delete throttler_; - this->lock(); - deleteCallbackThreads(); - this->unlock(); + // If the driver subclass is not destructible, or asyn is old, or we are not + // in an IOC (e.g. unit tests), we need to call shutdown ourselves. + // On newer versions of asyn, we could check with needsShutdown() to see if + // shutdown has already been done, be we don't want to rely on that. + if (pToThreadMsgQ_) + shutdownPortDriver(); + + delete throttler_; } /** Method that is normally called at the beginning of the processCallbacks @@ -1011,7 +1027,7 @@ asynStatus NDPluginDriver::createCallbackThreads() } /** Deletes the plugin threads. - * This method is called from the destructor and whenever QueueSize or NumThreads is changed. */ + * This method is called on shutdown and whenever QueueSize or NumThreads is changed. */ asynStatus NDPluginDriver::deleteCallbackThreads() { ToThreadMessage_t toMsg = {ToThreadMessageExit, 0}; diff --git a/ADApp/pluginSrc/NDPluginDriver.h b/ADApp/pluginSrc/NDPluginDriver.h index ec33b3f30..935f4d61a 100644 --- a/ADApp/pluginSrc/NDPluginDriver.h +++ b/ADApp/pluginSrc/NDPluginDriver.h @@ -70,6 +70,7 @@ class NDPLUGIN_API NDPluginDriver : public asynNDArrayDriver, public epicsThread size_t *nActual); virtual asynStatus readInt32Array(asynUser *pasynUser, epicsInt32 *value, size_t nElements, size_t *nIn); + virtual void shutdownPortDriver(); /* These are the methods that are new to this class */ virtual void driverCallback(asynUser *pasynUser, void *genericPointer); diff --git a/ADApp/pluginSrc/NDPluginROI.cpp b/ADApp/pluginSrc/NDPluginROI.cpp index bba4ce0db..6bd150936 100644 --- a/ADApp/pluginSrc/NDPluginROI.cpp +++ b/ADApp/pluginSrc/NDPluginROI.cpp @@ -283,7 +283,6 @@ asynStatus NDPluginROI::writeInt32(asynUser *pasynUser, epicsInt32 value) return status; } - /** Constructor for NDPluginROI; most parameters are simply passed to NDPluginDriver::NDPluginDriver. * After calling the base class constructor this method sets reasonable default values for all of the * ROI parameters. @@ -307,13 +306,13 @@ asynStatus NDPluginROI::writeInt32(asynUser *pasynUser, epicsInt32 value) NDPluginROI::NDPluginROI(const char *portName, int queueSize, int blockingCallbacks, const char *NDArrayPort, int NDArrayAddr, int maxBuffers, size_t maxMemory, - int priority, int stackSize, int maxThreads) + int priority, int stackSize, int maxThreads, int asynFlags) /* Invoke the base class constructor */ : NDPluginDriver(portName, queueSize, blockingCallbacks, NDArrayPort, NDArrayAddr, 1, maxBuffers, maxMemory, asynInt32ArrayMask | asynFloat64ArrayMask | asynGenericPointerMask, asynInt32ArrayMask | asynFloat64ArrayMask | asynGenericPointerMask, - ASYN_MULTIDEVICE, 1, priority, stackSize, maxThreads) + asynFlags | ASYN_MULTIDEVICE, 1, priority, stackSize, maxThreads) { //static const char *functionName = "NDPluginROI"; @@ -360,8 +359,13 @@ extern "C" int NDROIConfigure(const char *portName, int queueSize, int blockingC int maxBuffers, size_t maxMemory, int priority, int stackSize, int maxThreads) { + int flags = 0; +#ifdef ASYN_DESTRUCTIBLE + flags |= ASYN_DESTRUCTIBLE; +#endif + NDPluginROI *pPlugin = new NDPluginROI(portName, queueSize, blockingCallbacks, NDArrayPort, NDArrayAddr, - maxBuffers, maxMemory, priority, stackSize, maxThreads); + maxBuffers, maxMemory, priority, stackSize, maxThreads, flags); return pPlugin->start(); } diff --git a/ADApp/pluginSrc/NDPluginROI.h b/ADApp/pluginSrc/NDPluginROI.h index 29012bf2d..3020d583b 100644 --- a/ADApp/pluginSrc/NDPluginROI.h +++ b/ADApp/pluginSrc/NDPluginROI.h @@ -41,7 +41,7 @@ class NDPLUGIN_API NDPluginROI : public NDPluginDriver { NDPluginROI(const char *portName, int queueSize, int blockingCallbacks, const char *NDArrayPort, int NDArrayAddr, int maxBuffers, size_t maxMemory, - int priority, int stackSize, int maxThreads); + int priority, int stackSize, int maxThreads, int asynFlags = 0); /* These methods override the virtual methods in the base class */ void processCallbacks(NDArray *pArray); asynStatus writeInt32(asynUser *pasynUser, epicsInt32 value); diff --git a/RELEASE.md b/RELEASE.md index 92250a20e..9b34151ab 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -32,6 +32,14 @@ files respectively, in the configure/ directory of the appropriate release of th * Added ADHamammatsuDCAM and BlackflyS PGE 23S6C. * Reformatted into 2 columns. +### Destructible drivers and cleanup on shutdown + +Base classes were extended with support for asyn port shutdown and driver +destruction that is available since asyn R4-45. This functionality is optional: +existing drivers do not change behavior unless they opt in. See the plugin and +driver guidelines in the documentation for instructions on how to write a +destructible driver. + ## __R3-14 (December 1, 2024)__ ### asynNDArrayDriver and NDPluginBase.template diff --git a/docs/ADCore/destructible.rst b/docs/ADCore/destructible.rst new file mode 100644 index 000000000..6ede63b85 --- /dev/null +++ b/docs/ADCore/destructible.rst @@ -0,0 +1,64 @@ +Destructible drivers +==================== + +Since asyn R4-45, subclasses of ``asynPortDriver`` can declare themselves as +destructible. This makes asyn call ``asynPortDriver::shudownPortDriver()`` on +IOC shutdown, after which, the driver is deleted. This means that on recent +versions of asyn, destructors are run, whereas on older versions, they are not. +Performing shutdown correctly is important so that the driver disconnects from +the device properly, releasing device resources and allowing clean reconnection. + +Here are the guidelines for making a destructible driver. + +When backwards compatibility is not needed +------------------------------------------ + +If the driver only supports asyn R4-45 or newer, the recipe is as follows: + +#. Pass the ``ASYN_DESTRUCTIBLE`` flag to the base constructor (i.e., + ``NDPluginDriver`` or ``ADDriver``). + +#. Override ``shutdownPortDriver()`` and put there code that needs to be + executed with the driver intact. This is a good place to stop threads, for + example. ``shutdownPortDriver()`` is a virtual function, so don't forget to + call the base implementation. + +#. Implement the destructor. Do the cleanup as best you can. Note that + ``shutdownPortDriver()`` will only be called when the IOC shuts down, so, if + the driver could be used outside an IOC (e.g. in unit tests), you should call + ``shutdownPortDriver()`` from the destructor. To determine if it has already + been run, call ``shutdownNeeded()`` which will return ``false`` if the + shutdown has already happened. + +When you need backwards compatibility +------------------------------------- + +If the driver needs to support older asyn versions, or if you are adding +destructability to a base class whose subclasses will not be upgraded at the +same time, the recipe is as follows: + +#. The constructor should not add ``ASYN_DESTRUCTIBLE`` to the flags. Instead, + it should accept flags as an argument, and ``ASYN_DESTRUCTIBLE`` should be + put there by the iocsh command that instantiates the driver. This allows the + driver to be subclassed when the derived class is not destructible. Use of + ``ASYN_DESTRUCTIBLE`` needs to be gated with an ``#ifdef``. + +#. Override ``shutdownPortDriver()`` and put there code that needs to run on IOC + shutdown. ``shutdownPortDriver()`` is a virtual function, so don't forget to + call the base implementation. + +#. Implement the destructor. Note that newer versions of asyn will call it, but + older versions **will not**. So, use it to release memory and such, but + anything that needs to happen in order to disconnect from the device must go + into ``shutdownPortDriver()``. + + Note that ``shutdownPortDriver()`` will only be called when the IOC shuts + down, so, if the driver could be used outside an IOC (e.g. in unit tests), + you should call ``shutdownPortDriver()`` from the destructor. To determine if + it has already been run, you will need to set a variable in + ``shutdownPortDriver()`` yourself because the ``shutdownNeeded()`` function is + only available in newer asyn versions. + +#. Add a wrapper function to the IOC exit hook using ``epicsAtExit()`` which + calls ``shutdownPortDriver()`` on your driver. This must happen only if + ``ASYN_DESTRUCTIBLE`` is not defined. diff --git a/docs/ADCore/guidelines.rst b/docs/ADCore/guidelines.rst index 6a10575cb..0cc8fbbcf 100644 --- a/docs/ADCore/guidelines.rst +++ b/docs/ADCore/guidelines.rst @@ -39,3 +39,4 @@ The following are guidelines and rules for writing areaDetector drivers defined for this driver to the current array. - Call doCallbacksGenericPointer() so that registered clients can get the values of the new arrays. +- The driver shall :doc:`be destructible `. diff --git a/docs/ADCore/plugin_guidelines.rst b/docs/ADCore/plugin_guidelines.rst index 11cff9f5f..6be714c87 100644 --- a/docs/ADCore/plugin_guidelines.rst +++ b/docs/ADCore/plugin_guidelines.rst @@ -49,3 +49,4 @@ The following are guidelines and rules for writing plugins near the beginning of their ``processCallbacks()`` method, and will call ``NDPluginDriver::endProcessCallbacks()`` near the end of their ``processCallbacks()`` function. +- Plugins shall :doc:`be destructible `. diff --git a/docs/ADCore/plugins.rst b/docs/ADCore/plugins.rst index 53535d5cb..8c90bc88b 100644 --- a/docs/ADCore/plugins.rst +++ b/docs/ADCore/plugins.rst @@ -8,6 +8,7 @@ areaDetector Plugins plugin_overview NDPluginDriver plugin_guidelines + destructible plugin_medm common_plugins plugin_performance