Skip to content

Commit

Permalink
Shut down thread before destroying asynPortDriver
Browse files Browse the repository at this point in the history
This prevents a potentially locked mutex from being destroyed
as well as use-after-free bugs on other members of asynPortDriver.
This fixes issue epics-modules#83.
  • Loading branch information
mark0n committed Apr 12, 2019
1 parent b242e47 commit 6804156
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 29 deletions.
53 changes: 25 additions & 28 deletions asyn/asynPortDriver/asynPortDriver.cpp
Expand Up @@ -809,31 +809,38 @@ paramVal* paramList::getParameter(int index)
return this->vals[index];
}

callbackThread::callbackThread(asynPortDriver *portDriver) :
thread(*this, "asynPortDriverCallback", epicsThreadGetStackSize(epicsThreadStackMedium), epicsThreadPriorityMedium),
pPortDriver(portDriver)
{
thread.start();
}

callbackThread::~callbackThread()
{
shutdown.signal();
thread.exitWait();
}

/* I thought this would be a temporary fix until EPICS supported PINI after interruptAccept, which would then be used
* for input records that need callbacks after output records that also have PINI and that could affect them. But this
* does not work with asyn device support because of the ring buffer. Records with SCAN=I/O Intr must not processed
* for any other reason, including PINI, or the ring buffer can get out of sync. */
static void callbackTaskC(void *drvPvt)
{
asynPortDriver *pPvt = (asynPortDriver *)drvPvt;

pPvt->callbackTask();
}

void asynPortDriver::callbackTask()
void callbackThread::run()
{
int addr;

while(!interruptAccept) epicsThreadSleep(0.1);
epicsMutexLock(this->mutexId);
for (addr=0; addr<this->maxAddr; addr++) {
callParamCallbacks(addr, addr);
while(!interruptAccept && !shutdown.tryWait())
epicsThreadSleep(0.001);
epicsMutexLock(pPortDriver->mutexId);
for (addr=0; addr<pPortDriver->maxAddr; addr++) {
if(shutdown.tryWait()) break;
pPortDriver->callParamCallbacks(addr, addr);
}
epicsMutexUnlock(this->mutexId);
epicsMutexUnlock(pPortDriver->mutexId);
}



/** Locks the driver to prevent multiple threads from accessing memory at the same time.
* This function is called whenever asyn clients call the functions on the asyn interfaces.
* Drivers with their own background threads must call lock() to protect conflicts with
Expand Down Expand Up @@ -3407,26 +3414,16 @@ void asynPortDriver::initialize(const char *portNameIn, int maxAddrIn, int inter
}

/* Create a thread that waits for interruptAccept and then does all the callbacks once. */
status = (asynStatus)(epicsThreadCreate("asynPortDriverCallback",
epicsThreadPriorityMedium,
epicsThreadGetStackSize(epicsThreadStackMedium),
(EPICSTHREADFUNC)callbackTaskC,
this) == NULL);
if (status) {
std::string msg = std::string(driverName) + ":" + functionName +
" ERROR: epicsThreadCreate failure for callback task: " + portName;
asynPrint(this->pasynUserSelf, ASYN_TRACE_ERROR, "%s\n", msg.c_str());
throw std::runtime_error(msg);
}
cbThread = new callbackThread(this);
}

/** Destructor for asynPortDriver class; frees resources allocated when port driver is created. */
asynPortDriver::~asynPortDriver()
{
int addr;

delete cbThread;
epicsMutexDestroy(this->mutexId);
for (addr=0; addr<this->maxAddr; addr++) {

for (int addr=0; addr<this->maxAddr; addr++) {
delete this->params[addr];
}

Expand Down
16 changes: 15 additions & 1 deletion asyn/asynPortDriver/asynPortDriver.h
Expand Up @@ -6,6 +6,7 @@

#include <epicsTypes.h>
#include <epicsMutex.h>
#include <epicsThread.h>

#include <asynStandardInterfaces.h>
#include <asynParamType.h>
Expand Down Expand Up @@ -34,7 +35,7 @@ typedef void (*userTimeStampFunction)(void *userPvt, epicsTimeStamp *pTimeStamp)
#define asynGenericPointerMask 0x00001000
#define asynEnumMask 0x00002000


class callbackThread;

/** Base class for asyn port drivers; handles most of the bookkeeping for writing an asyn port driver
* with standard asyn interfaces and a parameter library. */
Expand Down Expand Up @@ -194,11 +195,24 @@ class epicsShareClass asynPortDriver {
int inputEosLenOctet;
char *outputEosOctet;
int outputEosLenOctet;
callbackThread *cbThread;
template <typename epicsType, typename interruptType>
asynStatus doCallbacksArray(epicsType *value, size_t nElements,
int reason, int address, void *interruptPvt);

friend class paramList;
friend class callbackThread;
};

class callbackThread: public epicsThreadRunable {
public:
callbackThread(asynPortDriver *portDriver);
~callbackThread();
void run();
private:
epicsThread thread;
asynPortDriver *pPortDriver;
epicsEvent shutdown;
};

#endif /* cplusplus */
Expand Down

0 comments on commit 6804156

Please sign in to comment.