Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ADApp/ADSrc/NDArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,11 @@ int NDArray::release()
printf("%s: WARNING, no owner\n", functionName);
return(ND_ERROR);
}
/* No-op if pool was already destroyed (e.g. IOC exit: PVA tears down after driver). */
if (NDArrayPool::isPoolDestroyed(pNDArrayPool)) {
pNDArrayPool = NULL;
return(ND_ERROR);
}
return(pNDArrayPool->release(this));
}

Expand Down
7 changes: 7 additions & 0 deletions ADApp/ADSrc/NDArray.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,13 @@ class ADCORE_API NDArrayPool {
size_t getMemorySize();
int getNumFree();
void emptyFreeList();

/** Register a pool as being destroyed so late NDArray::release() calls no-op (avoids SIGSEGV
* when pvAccess tears down MonitorElements after the driver and pool are deleted). */
static void registerDestroyingPool(NDArrayPool *p);
/** Check if the given pool address was registered as destroyed (pointer not dereferenced). */
static bool isPoolDestroyed(NDArrayPool *p);

static void setDefaultFrameMemoryFunctions(MallocFunc_t newMalloc,
FreeFunc_t newFree);
virtual void* frameMalloc(size_t size);
Expand Down
28 changes: 28 additions & 0 deletions ADApp/ADSrc/NDArrayPool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <string.h>
#include <stdlib.h>
#include <set>
#include <dbDefs.h>
#include <stdint.h>

Expand Down Expand Up @@ -46,6 +47,33 @@ FreeFunc_t defaultFrameFree = free;
volatile int eraseNDAttributes=0;
extern "C" {epicsExportAddress(int, eraseNDAttributes);}

/* Registry of pool addresses being destroyed so NDArray::release() can no-op (avoids SIGSEGV
* when pvAccess tears down after the driver). Only the address is used; the pointer is never
* dereferenced in isPoolDestroyed(). */
static std::set<NDArrayPool *> *destroyedPools = NULL;
static epicsMutexId destroyedPoolsMutex = NULL;

void NDArrayPool::registerDestroyingPool(NDArrayPool *p)
{
if (!p) return;
if (!destroyedPoolsMutex)
destroyedPoolsMutex = epicsMutexCreate();
if (!destroyedPools)
destroyedPools = new std::set<NDArrayPool *>();
Comment on lines +53 to +62

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the most thread-safe static initialization. If static objects are needed, it would definitely be best to use proper C++ classes so they could take advantage of the language guarantees around static objects inside of functions.

epicsMutexLock(destroyedPoolsMutex);
destroyedPools->insert(p);
epicsMutexUnlock(destroyedPoolsMutex);
}

bool NDArrayPool::isPoolDestroyed(NDArrayPool *p)
{
if (!p || !destroyedPoolsMutex || !destroyedPools) return false;
epicsMutexLock(destroyedPoolsMutex);
bool found = (destroyedPools->find(p) != destroyedPools->end());
epicsMutexUnlock(destroyedPoolsMutex);
return found;
}

/** NDArrayPool constructor
* \param[in] pDriver Pointer to the asynNDArrayDriver that created this object.
* \param[in] maxMemory Maxiumum number of bytes of memory the the pool is allowed to use, summed over
Expand Down
15 changes: 13 additions & 2 deletions ADApp/ADSrc/asynNDArrayDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -866,8 +866,8 @@ asynNDArrayDriver::asynNDArrayDriver(const char *portName, int maxAddr, int maxB
interfaceMask | asynInt32Mask | asynFloat64Mask | asynOctetMask | asynInt32ArrayMask | asynGenericPointerMask | asynDrvUserMask,
interruptMask | asynInt32Mask | asynFloat64Mask | asynOctetMask | asynInt32ArrayMask | asynGenericPointerMask,
asynFlags, autoConnect, priority, stackSize),
pNDArrayPool(NULL), queuedArrayCountMutex_(NULL), queuedArrayCount_(0),
queuedArrayUpdateRun_(true)
pNDArrayPool(NULL), maxAddr_(maxAddr), queuedArrayCountMutex_(NULL),
queuedArrayCount_(0), queuedArrayUpdateRun_(true)
{
char versionString[20];
static const char *functionName = "asynNDArrayDriver";
Expand Down Expand Up @@ -1029,6 +1029,17 @@ asynNDArrayDriver::~asynNDArrayDriver()
epicsEventSignal(queuedArrayEvent_);
epicsEventWait(queuedArrayUpdateDone_);

/* Register pool as destroying so any late NDArray::release() (e.g. from pvAccess
* MonitorElement teardown) no-ops instead of touching the pool we are about to delete. */
NDArrayPool::registerDestroyingPool(this->pNDArrayPoolPvt_);

/* Null pNDArrayPool on driver-owned arrays as well (belt and braces). */
if (this->pArrays) {
for (int i = 0; i < this->maxAddr_; i++) {
if (this->pArrays[i])
this->pArrays[i]->pNDArrayPool = NULL;
}
}
delete this->pNDArrayPoolPvt_;
free(this->pArrays);
delete this->pAttributeList;
Expand Down
1 change: 1 addition & 0 deletions ADApp/ADSrc/asynNDArrayDriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ class ADCORE_API asynNDArrayDriver : public asynPortDriver {

private:
asynStatus preAllocateBuffers();
int maxAddr_;
NDArrayPool *pNDArrayPoolPvt_;
epicsMutex *queuedArrayCountMutex_;
epicsEventId queuedArrayEvent_;
Expand Down