From 228d7fe279ebf946e7bdf24ae88e7cd60f169061 Mon Sep 17 00:00:00 2001 From: Matej Sekoranja Date: Wed, 7 Jan 2015 12:58:28 +0100 Subject: [PATCH] queue.take possible race, send thread does not wait for read anymore --- src/remote/codec.cpp | 7 ++++--- src/remote/codec.h | 5 ++++- src/remoteClient/clientContextImpl.cpp | 9 +++++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/remote/codec.cpp b/src/remote/codec.cpp index 134a3d4..cce2b3a 100644 --- a/src/remote/codec.cpp +++ b/src/remote/codec.cpp @@ -1214,7 +1214,7 @@ namespace epics { void BlockingAbstractCodec::sendThread(void *param) { - BlockingAbstractCodec *bac = static_cast(param); + BlockingAbstractCodec *bac = static_cast(param); Transport::shared_pointer ptr = bac->shared_from_this(); bac->setSenderThread(); @@ -1241,16 +1241,17 @@ namespace epics { } } + /* // wait read thread to die - // TODO rewise + // TODO rewise if this is really needed // this timeout is needed where close() is initiated from the send thread, // and not from the read thread as usualy - recv() does not exit until socket is not destroyed, // which is done the internalDestroy() call below bac->_shutdownEvent.wait(3.0); + */ // call internal destroy bac->internalDestroy(); - } diff --git a/src/remote/codec.h b/src/remote/codec.h index 9bfa24f..988c982 100644 --- a/src/remote/codec.h +++ b/src/remote/codec.h @@ -71,6 +71,7 @@ namespace epics { }; + // TODO replace this queue with lock-free implementation template class queue { public: @@ -119,6 +120,7 @@ namespace epics { } + // TODO very sub-optimal (locks and empty() - pop() sequence; at least 2 locks!) T take(int timeOut) { while (true) @@ -161,6 +163,8 @@ namespace epics { else { epics::pvData::Lock lock(_queueMutex); + if (_queue.empty()) + return T(); T sender = _queue.front(); _queue.pop_front(); return sender; @@ -179,7 +183,6 @@ namespace epics { epics::pvData::Event _queueEvent; epics::pvData::Mutex _queueMutex; AtomicValue _wakeup; - epics::pvData::Mutex _stdMutex; }; diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index fbe4d8d..d47db17 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -4515,6 +4515,15 @@ TODO // stop UDPs m_searchTransport->close(); m_broadcastTransport->close(); + + // wait for all transports to cleanly exit + int tries = 40; + epics::pvData::int32 transportCount; + while ((transportCount = m_transportRegistry->numberOfActiveTransports()) && tries--) + epicsThreadSleep(0.025); + + if (transportCount) + LOG(logLevelDebug, "PVA client context destroyed with %d transport(s) active.", transportCount); } void destroyAllChannels() {