Experimental Physics and
Industrial Control System

Hi Michael,

 

Thanks for tracking down the problem so quickly. 

 

Should I make a patch for libcom/test/epicsMessageQueueTest.cpp that would generate errors due to this bug?

 

I searched for all places in base where epicsMessageQueueReceiveWithTimeout is used.  There are none. 

 

I also searched for all occurrences of the C++ version, i.e. epicsMessageQueue::receive() with timeout.  The only ones seem to be in epicsMessageQueueTest.cpp.

 

I also searched the synApps epics-modules and areaDetector repositories for the same things.

 

This is where epicsMessageQueueReceiveWithTimeout is called:

 

corvette:~/devel>find . -name '*.c*' -type f -exec grep -H ReceiveWithTimeout {} \;

./mca-7-8/mcaApp/CanberraSrc/nmc_comm_subs_1.c:        len = epicsMessageQueueReceiveWithTimeout(m->responseQ, pkt, sizeof(*pkt), (double)(i->timeout_time/1000.));

./mca-7-8/mcaApp/CanberraSrc/nmc_comm_subs_1.c:        do len = epicsMessageQueueReceiveWithTimeout(nmc_module_info[module].responseQ, temp, sizeof(temp), 0.);

./asyn-4-39/asyn/drvAsynUSBTMC/drvAsynUSBTMC.c:            s = epicsMessageQueueReceiveWithTimeout(

./asyn-4-39/testAsynPortDriverApp/src/testMessageQueue.c:    recvSize = epicsMessageQueueReceiveWithTimeout(msgQId, &value, sizeof(value), TIMEOUT);

./ipUnidig-2-11/ipUnidigApp/src/drvIpUnidig.cpp:    status = epicsMessageQueueReceiveWithTimeout(msgQId_,

./autosave-5-10/asApp/src/save_restore.c:               while (epicsMessageQueueReceiveWithTimeout(opMsgQueue, (void*) &msg, OP_MSG_SIZE, (double)MIN_DELAY) >= 0) {

./caputRecorder-1-7-2/caputRecorderApp/src/caputRecorder.c:             msg_size = epicsMessageQueueReceiveWithTimeout(caputRecorderMsgQueue, (void *)pmsg, MSG_SIZE, 5.0);

./softGlue-2-8-2/softGlueApp/src/drvIP_EP201.c:         if (epicsMessageQueueReceiveWithTimeout(pPvt->msgQId,

./areaDetector-3-9/aravisGigE/aravisGigEApp/src/aravisCamera.cpp:        if (epicsMessageQueueReceiveWithTimeout(this->msgQId, &buffer, sizeof(&buffer), 0.005) == -1) {

./areaDetector-3-9/ADAravis/aravisApp/src/ADAravis.cpp:        if (epicsMessageQueueReceiveWithTimeout(this->msgQId, &buffer, sizeof(&buffer), 0.005) == -1) {

 

This is where I believe epicsMessageQueue::receive() with a timeout is used:

 

corvette:~/devel>find . -name '*.c*' -type f -exec grep -H 'receive(' {} \;

./dante-1-0/danteApp/danteSrc/dante.cpp:    numRecv = msgQ_->receive(&message, sizeof(message), MESSAGE_TIMEOUT);

./areaDetector-3-9/ADCore/ADApp/pluginSrc/NDPluginDriver.cpp:        numBytes = pFromThreadMsgQ_->receive(&fromMsg, sizeof(fromMsg), 2.0);

./areaDetector-3-9/ADCore/ADApp/pluginSrc/NDPluginDriver.cpp:            numBytes = pFromThreadMsgQ_->receive(&fromMsg, sizeof(fromMsg), 2.0);

 

So there are very few places where either the C or C++ version of this function is used, which is probably why a very old bug has not been found previously.

 

> Rather than a timeout, I would use a special message to signal the receiver to (maybe) break from its loop.  In this case, a NULL should do nicely.

 

Excellent suggestion.  I have changed ADSpinnaker to do that instead.  This avoids the bug in base and is more elegant as well.

 

Thanks,

Mark

 

 

 

-----Original Message-----
From: Michael Davidsaver <mdavidsaver at gmail.com>
Sent: Friday, March 20, 2020 9:46 PM
To: Mark Rivers <rivers at cars.uchicago.edu>
Cc: 'core-talk at aps.anl.gov' <core-talk at aps.anl.gov>
Subject: Re: C++ question

 

On 3/20/20 6:32 PM, Mark Rivers wrote:

>> It's also not clear to me why you need a timeout on the dequeue step

>> at all.  Are you using this to detect a device error?

> No, I am not using it to detect a device error.  I am using because, for example, in continuous acquisition mode the dequeue routine is being called in an infinite loop.  But when the user tells acquisition to stop that final frame will never arrive, so the dequeue function needs to time out and exit.

>

 

Rather than a timeout, I would use a special message to signal the receiver to (maybe) break from its loop.  In this case, a NULL should do nicely.