diff --git a/pvAccessApp/remote/blockingClientTCPTransport.cpp b/pvAccessApp/remote/blockingClientTCPTransport.cpp index 87e0ea4..7455b21 100644 --- a/pvAccessApp/remote/blockingClientTCPTransport.cpp +++ b/pvAccessApp/remote/blockingClientTCPTransport.cpp @@ -67,10 +67,10 @@ namespace epics { epicsTimeStamp currentTime; epicsTimeGetCurrent(¤tTime); - _ownersMutex.lock(); + _mutex.lock(); // no exception expected here double diff = epicsTimeDiffInSeconds(¤tTime, &_aliveTimestamp); - _ownersMutex.unlock(); + _mutex.unlock(); if(diff>2*_connectionTimeout) { unresponsiveTransport(); @@ -84,7 +84,7 @@ namespace epics { } void BlockingClientTCPTransport::unresponsiveTransport() { - Lock lock(_ownersMutex); + Lock lock(_mutex); if(!_unresponsiveTransport) { _unresponsiveTransport = true; @@ -107,19 +107,23 @@ namespace epics { ipAddrToDottedIP(&_socketAddress.ia, ipAddrStr, sizeof(ipAddrStr)); LOG(logLevelDebug, "Acquiring transport to %s.", ipAddrStr); - Lock lock2(_ownersMutex); -// TODO double check? if(_closed) return false; - //_owners.insert(TransportClient::weak_pointer(client)); _owners[client->getID()] = TransportClient::weak_pointer(client); + //_owners.insert(TransportClient::weak_pointer(client)); return true; } + // _mutex is held when this method is called void BlockingClientTCPTransport::internalClose(bool forced) { BlockingTCPTransport::internalClose(forced); _timerNode.cancel(); + } + void BlockingClientTCPTransport::internalPostClose(bool forced) { + BlockingTCPTransport::internalPostClose(forced); + + // _owners cannot change when transport is closed closedNotifyClients(); } @@ -127,7 +131,6 @@ namespace epics { * Notifies clients about disconnect. */ void BlockingClientTCPTransport::closedNotifyClients() { - Lock lock(_ownersMutex); // check if still acquired int refs = _owners.size(); @@ -163,7 +166,6 @@ namespace epics { LOG(logLevelDebug, "Releasing transport to %s.", ipAddrStr); - Lock lock2(_ownersMutex); _owners.erase(clientID); //_owners.erase(TransportClient::weak_pointer(client)); @@ -173,13 +175,13 @@ namespace epics { } void BlockingClientTCPTransport::aliveNotification() { - Lock guard(_ownersMutex); + Lock guard(_mutex); epicsTimeGetCurrent(&_aliveTimestamp); if(_unresponsiveTransport) responsiveTransport(); } void BlockingClientTCPTransport::responsiveTransport() { - Lock lock(_ownersMutex); + Lock lock(_mutex); if(_unresponsiveTransport) { _unresponsiveTransport = false; @@ -198,7 +200,7 @@ namespace epics { void BlockingClientTCPTransport::changedTransport() { _introspectionRegistry.reset(); - Lock lock(_ownersMutex); + Lock lock(_mutex); TransportClientMap_t::iterator it = _owners.begin(); for(; it!=_owners.end(); it++) { TransportClient::shared_pointer client = it->second.lock(); diff --git a/pvAccessApp/remote/blockingServerTCPTransport.cpp b/pvAccessApp/remote/blockingServerTCPTransport.cpp index 644e5e3..aa7ff33 100644 --- a/pvAccessApp/remote/blockingServerTCPTransport.cpp +++ b/pvAccessApp/remote/blockingServerTCPTransport.cpp @@ -67,6 +67,10 @@ namespace epics { destroyAllChannels(); } + void BlockingServerTCPTransport::internalPostClose(bool forced) { + BlockingTCPTransport::internalClose(forced); + } + pvAccessID BlockingServerTCPTransport::preallocateChannelSID() { Lock lock(_channelsMutex); // search first free (theoretically possible loop of death) diff --git a/pvAccessApp/remote/blockingTCP.h b/pvAccessApp/remote/blockingTCP.h index 7826153..f2d2e0a 100644 --- a/pvAccessApp/remote/blockingTCP.h +++ b/pvAccessApp/remote/blockingTCP.h @@ -179,6 +179,13 @@ namespace epics { */ virtual void internalClose(bool force); + /** + * Called to any resources just after closing transport and without any locks held on transport + * @param[in] force flag indicating if forced (e.g. forced + * disconnect) is required + */ + virtual void internalPostClose(bool force); + /** * Send a buffer through the transport. * NOTE: TCP sent buffer/sending has to be synchronized (not done by this method). @@ -520,6 +527,7 @@ namespace epics { IntrospectionRegistry _introspectionRegistry; virtual void internalClose(bool force); + virtual void internalPostClose(bool force); private: @@ -550,9 +558,6 @@ namespace epics { */ epicsTimeStamp _aliveTimestamp; - epics::pvData::Mutex _mutex; - epics::pvData::Mutex _ownersMutex; - bool _verifyOrEcho; /** @@ -749,6 +754,7 @@ namespace epics { IntrospectionRegistry _introspectionRegistry; virtual void internalClose(bool force); + virtual void internalPostClose(bool force); private: /** diff --git a/pvAccessApp/remote/blockingTCPTransport.cpp b/pvAccessApp/remote/blockingTCPTransport.cpp index f875dd2..6202423 100644 --- a/pvAccessApp/remote/blockingTCPTransport.cpp +++ b/pvAccessApp/remote/blockingTCPTransport.cpp @@ -235,6 +235,11 @@ namespace epics { // notify send queue _sendQueueEvent.signal(); + + lock.unlock(); + + // post close without a lock + internalPostClose(force); } void BlockingTCPTransport::internalClose(bool force) { @@ -245,6 +250,9 @@ namespace epics { } } + void BlockingTCPTransport::internalPostClose(bool force) { + } + int BlockingTCPTransport::getSocketReceiveBufferSize() const { // Get value of the SO_RCVBUF option for this DatagramSocket, // that is the buffer size used by the platform for input on diff --git a/pvAccessApp/remote/channelSearchManager.cpp b/pvAccessApp/remote/channelSearchManager.cpp index f3f4434..8510397 100644 --- a/pvAccessApp/remote/channelSearchManager.cpp +++ b/pvAccessApp/remote/channelSearchManager.cpp @@ -581,6 +581,7 @@ void ChannelSearchManager::unregisterChannel(SearchInstance* channel) void ChannelSearchManager::searchResponse(int32 cid, int32 seqNo, int8 minorRevision, osiSockAddr* serverAddress) { Lock guard(_channelMutex); + // first remove SearchInstance* si = NULL; _channelsIter = _channels.find(cid); @@ -597,12 +598,16 @@ void ChannelSearchManager::searchResponse(int32 cid, int32 seqNo, int8 minorRevi now.getCurrent(); _timers[timerIndex]->searchResponse(seqNo, seqNo != 0, now.getMilliseconds()); + guard.unlock(); + // then noftify SearchInstance si->searchResponse(minorRevision, serverAddress); //si->release(); } else { + guard.unlock(); + // minor hack to enable duplicate reports si = dynamic_cast(_context->getChannel(cid).get()); // TODO if(si != NULL) diff --git a/pvAccessApp/remoteClient/clientContextImpl.cpp b/pvAccessApp/remoteClient/clientContextImpl.cpp index 5b641bc..a6235c6 100644 --- a/pvAccessApp/remoteClient/clientContextImpl.cpp +++ b/pvAccessApp/remoteClient/clientContextImpl.cpp @@ -3834,7 +3834,10 @@ TODO iter++) { channels[count++] = iter->second; - } + } + + guard.unlock(); + ChannelImpl::shared_pointer ptr; for (int i = 0; i < count; i++)