From 03d2cf6d30331ed7860e7ff0c4d5b76b46822a36 Mon Sep 17 00:00:00 2001 From: Jure Varlec Date: Wed, 25 Feb 2026 09:29:20 +0000 Subject: [PATCH 1/3] Restructure shutdown to use ASYN_DESTRUCTIBLE --- aravisApp/src/ADAravis.cpp | 50 +++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/aravisApp/src/ADAravis.cpp b/aravisApp/src/ADAravis.cpp index 6910fd3..9aef682 100644 --- a/aravisApp/src/ADAravis.cpp +++ b/aravisApp/src/ADAravis.cpp @@ -147,6 +147,7 @@ class ADAravis : public ADGenICam, epicsThreadRunable { /* Constructor */ ADAravis(const char *portName, const char *cameraName, int enableCaching, size_t maxMemory, int priority, int stackSize); + ~ADAravis(); /* These are the methods that we override from ADDriver */ virtual asynStatus writeInt32(asynUser *pasynUser, epicsInt32 value); @@ -155,6 +156,7 @@ class ADAravis : public ADGenICam, epicsThreadRunable { std::string const & featureName, GCFeatureType_t featureType); virtual asynStatus startCapture(); virtual asynStatus stopCapture(); + virtual void shutdownPortDriver(); void report(FILE *fp, int details); /* This is the method we override from epicsThreadRunable */ @@ -217,19 +219,13 @@ GenICamFeature *ADAravis::createFeature(GenICamFeatureSet *set, return pFeature; } -/** Called by epicsAtExit to shutdown camera */ +#ifndef ASYN_DESTRUCTIBLE +/** On old asyn versions, called by epicsAtExit to shutdown camera */ static void aravisShutdown(void* arg) { ADAravis *pPvt = (ADAravis *) arg; - GErrorHelper err; - ArvCamera *cam = pPvt->camera; - printf("ADAravis: Stopping %s... ", pPvt->portName); - arv_camera_stop_acquisition(cam, err.get()); - pPvt->connectionValid = 0; - epicsThreadSleep(0.1); - pPvt->camera = NULL; - g_object_unref(cam); - printf("ADAravis: OK\n"); + pPvt->shutdownPortDriver(); } +#endif /** Called by aravis when destroying a buffer with an NDArray wrapper */ static void destroyBuffer(gpointer data){ @@ -311,7 +307,13 @@ static void setIocRunningFlag(initHookState state) { ADAravis::ADAravis(const char *portName, const char *cameraName, int enableCaching, size_t maxMemory, int priority, int stackSize) - : ADGenICam(portName, maxMemory, priority, stackSize), + : ADGenICam(portName, maxMemory, priority, stackSize, +#ifdef ASYN_DESTRUCTIBLE + ASYN_DESTRUCTIBLE +#else + 0 +#endif + ), camera(NULL), connectionValid(0), stream(NULL), @@ -385,14 +387,40 @@ ADAravis::ADAravis(const char *portName, const char *cameraName, int enableCachi this->featureIndex = 0; this->connectToCamera(); +#ifndef ASYN_DESTRUCTIBLE /* Register the shutdown function for epicsAtExit */ epicsAtExit(aravisShutdown, (void*)this); +#endif /* Register the pollingLoop to start after iocInit */ initHookRegister(setIocRunningFlag); this->pollingLoop.start(); } +void ADAravis::shutdownPortDriver() { + asynPrint(pasynUserSelf, ASYN_TRACE_FLOW, "%s shutting down\n", driverName); + + GErrorHelper err; + arv_camera_stop_acquisition(camera, err.get()); + connectionValid = 0; + epicsThreadSleep(0.1); + + ADGenICam::shutdownPortDriver(); +} + +ADAravis::~ADAravis() { + // 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 (!exiting) { + shutdownPortDriver(); + } + + g_object_unref(camera); + camera = NULL; +} + asynStatus ADAravis::makeCameraObject() { const char *functionName = "makeCameraObject"; From 44029a27fab0f09cdb5cf5e6324aff6b06d2731d Mon Sep 17 00:00:00 2001 From: Jure Varlec Date: Wed, 25 Feb 2026 11:03:12 +0000 Subject: [PATCH 2/3] Exit the polling thread on shutdown. Without stopping the thread, the IOC hangs on shutdown because the destructor of the thread objects waits for the thread to exit. This was not a problem before because the destructors never ran. --- aravisApp/src/ADAravis.cpp | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/aravisApp/src/ADAravis.cpp b/aravisApp/src/ADAravis.cpp index 9aef682..0341537 100644 --- a/aravisApp/src/ADAravis.cpp +++ b/aravisApp/src/ADAravis.cpp @@ -166,9 +166,6 @@ class ADAravis : public ADGenICam, epicsThreadRunable { epicsMessageQueueId msgQId; void newBufferCallback(ArvStream *stream); - /** Used by epicsAtExit */ - ArvCamera *camera; - /** Used by connection lost callback */ int connectionValid; @@ -198,6 +195,7 @@ class ADAravis : public ADGenICam, epicsThreadRunable { asynStatus makeCameraObject(); asynStatus makeStreamObject(); + ArvCamera *camera; ArvStream *stream; ArvDevice *device; ArvGc *genicam; @@ -208,6 +206,7 @@ class ADAravis : public ADGenICam, epicsThreadRunable { int nConsecutiveBadFrames; int nBadFramesPrior; epicsThread pollingLoop; + bool exiting; std::vector featureList; }; @@ -314,8 +313,8 @@ ADAravis::ADAravis(const char *portName, const char *cameraName, int enableCachi 0 #endif ), - camera(NULL), connectionValid(0), + camera(NULL), stream(NULL), device(NULL), genicam(NULL), @@ -326,7 +325,8 @@ ADAravis::ADAravis(const char *portName, const char *cameraName, int enableCachi pollingLoop(*this, "aravisPoll", stackSize>0 ? stackSize : epicsThreadGetStackSize(epicsThreadStackMedium), - epicsThreadPriorityHigh) + epicsThreadPriorityHigh), + exiting(false) { const char *functionName = "ADAravis"; char tempString[256]; @@ -400,10 +400,13 @@ ADAravis::ADAravis(const char *portName, const char *cameraName, int enableCachi void ADAravis::shutdownPortDriver() { asynPrint(pasynUserSelf, ASYN_TRACE_FLOW, "%s shutting down\n", driverName); + lock(); GErrorHelper err; arv_camera_stop_acquisition(camera, err.get()); connectionValid = 0; - epicsThreadSleep(0.1); + exiting = true; + unlock(); + pollingLoop.exitWait(); ADGenICam::shutdownPortDriver(); } @@ -711,11 +714,18 @@ void ADAravis::run() { epicsThreadSleep(0.1); } - /* Loop forever */ + /* Loop until driver shutdown */ epicsTimeGetCurrent(&lastFeatureGet); while (1) { /* Wait 5ms for an array to arrive from the queue */ if (epicsMessageQueueReceiveWithTimeout(this->msgQId, &buffer, sizeof(&buffer), 0.005) == -1) { + lock(); + /* We could use needsShutdown() here, but it requires a recent asyn. */ + bool e = exiting; + unlock(); + if (e) { + break; + } } else { /* Got a buffer, so lock up and process it */ this->lock(); From c79b8c3d3c7bdc46ffb6925c6aeb6350648f83fb Mon Sep 17 00:00:00 2001 From: Jure Varlec Date: Wed, 25 Feb 2026 11:23:18 +0000 Subject: [PATCH 3/3] Destroy the stream before the camera Without this, even if the camera object is destroyed, control of the camera is never properly relinquished. With some cameras, this means that it is not possible to reconnect to the camera until the previous connection times out. Deletion of stream and camera is moved from the destructor into the shutdown function so that it happens even on old asyn that doesn't destroy the driver. --- aravisApp/src/ADAravis.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/aravisApp/src/ADAravis.cpp b/aravisApp/src/ADAravis.cpp index 0341537..ad76649 100644 --- a/aravisApp/src/ADAravis.cpp +++ b/aravisApp/src/ADAravis.cpp @@ -409,6 +409,19 @@ void ADAravis::shutdownPortDriver() { pollingLoop.exitWait(); ADGenICam::shutdownPortDriver(); + + // This would normally go in the destructor. But on old versions of asyn, + // the driver is not deleted after shutdown completes, and the following + // steps are needed to disconnect the camera. + arv_stream_set_emit_signals(stream, false); + g_object_unref(stream); + g_object_unref(camera); + + // This makes it easier to find use-after-free. + stream = NULL; + genicam = NULL; + device = NULL; + camera = NULL; } ADAravis::~ADAravis() { @@ -419,9 +432,6 @@ ADAravis::~ADAravis() { if (!exiting) { shutdownPortDriver(); } - - g_object_unref(camera); - camera = NULL; } asynStatus ADAravis::makeCameraObject() {