From ff197f89cb6be9ad5b64b3a2435bc74d765217d7 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Thu, 19 Apr 2018 14:13:07 -0700 Subject: [PATCH] testpvalink avoid false positive races --- pdbApp/pv/qsrv.h | 4 ++++ pdbApp/pvalink.cpp | 17 ++++++++++++++++ pdbApp/pvalink.h | 1 + pdbApp/pvalink_channel.cpp | 11 ++++++++-- pdbApp/pvalink_lset.cpp | 7 ++++--- testApp/testpvalink.cpp | 41 +++++++++++++++++++++++++------------- testApp/testpvalink.db | 2 +- 7 files changed, 63 insertions(+), 20 deletions(-) diff --git a/pdbApp/pv/qsrv.h b/pdbApp/pv/qsrv.h index 620cad8..d64d1d1 100644 --- a/pdbApp/pv/qsrv.h +++ b/pdbApp/pv/qsrv.h @@ -19,12 +19,16 @@ extern "C" { #endif +struct link; /* aka. DBLINK from link.h */ + /** returns QSRV_VERSION_INT captured at compilation time */ epicsShareExtern unsigned qsrvVersion(void); /** returns QSRV_ABI_VERSION_INT captured at compilation time */ epicsShareExtern unsigned qsrvABIVersion(void); +epicsShareFunc void testqsrvWaitForLinkEvent(struct link *plink); + /** Call before testIocShutdownOk() @code testdbPrepare(); diff --git a/pdbApp/pvalink.cpp b/pdbApp/pvalink.cpp index 484ec8c..08eb4fc 100644 --- a/pdbApp/pvalink.cpp +++ b/pdbApp/pvalink.cpp @@ -138,6 +138,23 @@ void testqsrvCleanup(void) } } +void testqsrvWaitForLinkEvent(struct link *plink) +{ + std::tr1::shared_ptr lchan; + { + DBScanLocker lock(plink->precord); + + if(plink->type!=JSON_LINK || !plink->value.json.jlink || plink->value.json.jlink->pif!=&lsetPVA) { + testAbort("Not a PVA link"); + } + pvaLink *pval = static_cast(plink->value.json.jlink); + lchan = pval->lchan; + } + if(lchan) { + lchan->run_done.wait(); + } +} + static void installPVAAddLinkHook() { diff --git a/pdbApp/pvalink.h b/pdbApp/pvalink.h index 106739c..e761fa9 100644 --- a/pdbApp/pvalink.h +++ b/pdbApp/pvalink.h @@ -121,6 +121,7 @@ struct pvaLinkChannel : public pvac::ClientChannel::MonitorCallback, static size_t num_instances; pvd::Mutex lock; + epicsEvent run_done; // used by testing code pvac::ClientChannel chan; pvac::Monitor op_mon; diff --git a/pdbApp/pvalink_channel.cpp b/pdbApp/pvalink_channel.cpp index 9615bd1..2f06f81 100644 --- a/pdbApp/pvalink_channel.cpp +++ b/pdbApp/pvalink_channel.cpp @@ -270,8 +270,13 @@ void pvaLinkChannel::run() // pop next update from monitor queue. // still under lock to safeguard concurrent calls to lset functions - if(connected && !op_mon.poll()) + if(connected && !op_mon.poll()) { + TRACE(<<"empty"); + run_done.signal(); return; // monitor queue is empty, nothing more to do here + } + + TRACE(<<(connected_latched?"connected":"disconnected")); assert(!connected || !!op_mon.root); @@ -305,7 +310,7 @@ void pvaLinkChannel::run() // at this point we know we will re-queue, but not immediately // so an expected error won't get us stuck in a tight loop. - requeue = queued = true; + requeue = queued = connected_latched; if(links_changed) { // a link has been added or removed since the last update. @@ -361,6 +366,8 @@ void pvaLinkChannel::run() if(requeue) { // re-queue until monitor queue is empty pvaGlobal->queue.add(shared_from_this()); + } else { + run_done.signal(); } } diff --git a/pdbApp/pvalink_lset.cpp b/pdbApp/pvalink_lset.cpp index f29ffd2..0c88c4a 100644 --- a/pdbApp/pvalink_lset.cpp +++ b/pdbApp/pvalink_lset.cpp @@ -99,7 +99,6 @@ int pvaIsConnected(const DBLINK *plink) TRY { TRACE(<precord->name<<" "<channelName); Guard G(self->lchan->lock); - if(!self->valid()) return -1; return self->valid(); @@ -161,9 +160,10 @@ long pvaGetValue(DBLINK *plink, short dbrType, void *pbuffer, long *pnRequest) { TRY { - TRACE(<precord->name<<" "<channelName); Guard G(self->lchan->lock); + TRACE(<precord->name<<" "<channelName<<" conn="<<(self->valid()?"T":"F")); + if(!self->valid()) { // disconnected if(self->ms != pvaLink::NMS) { @@ -371,10 +371,11 @@ long pvaPutValue(DBLINK *plink, short dbrType, const void *pbuffer, long nRequest) { TRY { - TRACE(<precord->name<<" "<channelName); (void)self; Guard G(self->lchan->lock); + TRACE(<precord->name<<" "<channelName<<" nReq="<valid()) { diff --git a/testApp/testpvalink.cpp b/testApp/testpvalink.cpp index 4fee846..6d72daf 100644 --- a/testApp/testpvalink.cpp +++ b/testApp/testpvalink.cpp @@ -1,10 +1,13 @@ #include #include +#include +#include #include #include "utilities.h" #include "pvalink.h" +#include "pv/qsrv.h" namespace { @@ -12,40 +15,50 @@ void testGet() { testDiag("==== testGet ===="); + longinRecord *li1 = (longinRecord*)testdbRecordPtr("src:li1"); + + while(!dbIsLinkConnected(&li1->inp)) + testqsrvWaitForLinkEvent(&li1->inp); + testdbGetFieldEqual("target:li.VAL", DBF_LONG, 42); - testdbGetFieldEqual("src:li1.VAL", DBF_LONG, 0); + + testdbGetFieldEqual("src:li1.VAL", DBF_LONG, 0); // value before first process + testdbGetFieldEqual("src:li1.INP", DBF_STRING, "{\"pva\":\"target:li\"}"); testdbPutFieldOk("src:li1.PROC", DBF_LONG, 1); - //TODO: wait for dbEvent queue update - epicsThreadSleep(0.1); testdbGetFieldEqual("src:li1.VAL", DBF_LONG, 42); testdbPutFieldOk("src:li1.INP", DBF_STRING, "{\"pva\":\"target:ai\"}"); - testdbGetFieldEqual("src:li1.VAL", DBF_LONG, 42); + while(!dbIsLinkConnected(&li1->inp)) + testqsrvWaitForLinkEvent(&li1->inp); + + testdbGetFieldEqual("src:li1.VAL", DBF_LONG, 42); // changing link doesn't automatically process - //TODO: wait for pvalink worker update - epicsThreadSleep(0.1); testdbPutFieldOk("src:li1.PROC", DBF_LONG, 1); - //TODO: wait for dbEvent queue update - epicsThreadSleep(0.1); - testdbGetFieldEqual("src:li1.VAL", DBF_LONG, 4); + testdbGetFieldEqual("src:li1.VAL", DBF_LONG, 4); // now it's changed } void testPut() { testDiag("==== testPut ===="); - testdbGetFieldEqual("target:li2.VAL", DBF_LONG, 43); - testdbGetFieldEqual("src:li2.VAL", DBF_LONG, 0); - testdbGetFieldEqual("src:li2.OUT", DBF_STRING, "{\"pva\":\"target:li2\"}"); - testdbPutFieldOk("src:li2.VAL", DBF_LONG, 14); + longoutRecord *lo2 = (longoutRecord*)testdbRecordPtr("src:lo2"); + + while(!dbIsLinkConnected(&lo2->out)) + testqsrvWaitForLinkEvent(&lo2->out); + + testdbGetFieldEqual("target:li2.VAL", DBF_LONG, 43); + testdbGetFieldEqual("src:lo2.VAL", DBF_LONG, 0); + testdbGetFieldEqual("src:lo2.OUT", DBF_STRING, "{\"pva\":\"target:li2\"}"); + + testdbPutFieldOk("src:lo2.VAL", DBF_LONG, 14); testdbGetFieldEqual("target:li2.VAL", DBF_LONG, 14); - testdbGetFieldEqual("src:li2.VAL", DBF_LONG, 14); + testdbGetFieldEqual("src:lo2.VAL", DBF_LONG, 14); } } // namespace diff --git a/testApp/testpvalink.db b/testApp/testpvalink.db index 69d29e7..334daae 100644 --- a/testApp/testpvalink.db +++ b/testApp/testpvalink.db @@ -16,6 +16,6 @@ record(longin, "target:li2") { field(VAL, "43") } -record(longout, "src:li2") { +record(longout, "src:lo2") { field(OUT, {pva:"target:li2"}) }