From 735dc22e2d5e43a3adca36aca3af91a68e151971 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 28 Mar 2018 13:37:42 -0700 Subject: [PATCH 01/42] collapse SimpleChannelSearchManagerImpl into ChannelSearchManager Virtual base with only one sub-class. So avoid some unnecessary indirection. --- src/remote/Makefile | 2 +- ...nagerImpl.cpp => channelSearchManager.cpp} | 60 +++--- src/remote/pv/channelSearchManager.h | 150 +++++++++++-- .../pv/simpleChannelSearchManagerImpl.h | 204 ------------------ src/remoteClient/clientContextImpl.cpp | 6 +- 5 files changed, 166 insertions(+), 256 deletions(-) rename src/remote/{simpleChannelSearchManagerImpl.cpp => channelSearchManager.cpp} (79%) delete mode 100644 src/remote/pv/simpleChannelSearchManagerImpl.h diff --git a/src/remote/Makefile b/src/remote/Makefile index 1e180cf..89f4361 100644 --- a/src/remote/Makefile +++ b/src/remote/Makefile @@ -9,7 +9,7 @@ pvAccess_SRCS += blockingUDPTransport.cpp pvAccess_SRCS += blockingUDPConnector.cpp pvAccess_SRCS += beaconHandler.cpp pvAccess_SRCS += blockingTCPConnector.cpp -pvAccess_SRCS += simpleChannelSearchManagerImpl.cpp +pvAccess_SRCS += channelSearchManager.cpp pvAccess_SRCS += abstractResponseHandler.cpp pvAccess_SRCS += blockingTCPAcceptor.cpp pvAccess_SRCS += transportRegistry.cpp diff --git a/src/remote/simpleChannelSearchManagerImpl.cpp b/src/remote/channelSearchManager.cpp similarity index 79% rename from src/remote/simpleChannelSearchManagerImpl.cpp rename to src/remote/channelSearchManager.cpp index 638f3b4..c6ce5ab 100644 --- a/src/remote/simpleChannelSearchManagerImpl.cpp +++ b/src/remote/channelSearchManager.cpp @@ -11,7 +11,7 @@ #include #define epicsExportSharedSymbols -#include +#include #include #include #include @@ -23,25 +23,25 @@ using namespace epics::pvData; namespace epics { namespace pvAccess { -const int SimpleChannelSearchManagerImpl::DATA_COUNT_POSITION = PVA_MESSAGE_HEADER_SIZE + 4+1+3+16+2+1+4; -const int SimpleChannelSearchManagerImpl::CAST_POSITION = PVA_MESSAGE_HEADER_SIZE + 4; -const int SimpleChannelSearchManagerImpl::PAYLOAD_POSITION = 4; +const int ChannelSearchManager::DATA_COUNT_POSITION = PVA_MESSAGE_HEADER_SIZE + 4+1+3+16+2+1+4; +const int ChannelSearchManager::CAST_POSITION = PVA_MESSAGE_HEADER_SIZE + 4; +const int ChannelSearchManager::PAYLOAD_POSITION = 4; // 225ms +/- 25ms random -const double SimpleChannelSearchManagerImpl::ATOMIC_PERIOD = 0.225; -const int SimpleChannelSearchManagerImpl::PERIOD_JITTER_MS = 25; +const double ChannelSearchManager::ATOMIC_PERIOD = 0.225; +const int ChannelSearchManager::PERIOD_JITTER_MS = 25; -const int SimpleChannelSearchManagerImpl::DEFAULT_USER_VALUE = 1; -const int SimpleChannelSearchManagerImpl::BOOST_VALUE = 1; +const int ChannelSearchManager::DEFAULT_USER_VALUE = 1; +const int ChannelSearchManager::BOOST_VALUE = 1; // must be power of two (so that search is done) -const int SimpleChannelSearchManagerImpl::MAX_COUNT_VALUE = 1 << 8; -const int SimpleChannelSearchManagerImpl::MAX_FALLBACK_COUNT_VALUE = (1 << 7) + 1; +const int ChannelSearchManager::MAX_COUNT_VALUE = 1 << 8; +const int ChannelSearchManager::MAX_FALLBACK_COUNT_VALUE = (1 << 7) + 1; -const int SimpleChannelSearchManagerImpl::MAX_FRAMES_AT_ONCE = 10; -const int SimpleChannelSearchManagerImpl::DELAY_BETWEEN_FRAMES_MS = 50; +const int ChannelSearchManager::MAX_FRAMES_AT_ONCE = 10; +const int ChannelSearchManager::DELAY_BETWEEN_FRAMES_MS = 50; -SimpleChannelSearchManagerImpl::SimpleChannelSearchManagerImpl(Context::shared_pointer const & context) : +ChannelSearchManager::ChannelSearchManager(Context::shared_pointer const & context) : m_context(context), m_responseAddress(), // initialized in activate() m_canceled(), @@ -58,7 +58,7 @@ SimpleChannelSearchManagerImpl::SimpleChannelSearchManagerImpl(Context::shared_p srand ( time(NULL) ); } -void SimpleChannelSearchManagerImpl::activate() +void ChannelSearchManager::activate() { m_responseAddress = Context::shared_pointer(m_context)->getSearchTransport()->getRemoteAddress(); @@ -73,15 +73,15 @@ void SimpleChannelSearchManagerImpl::activate() context->getTimer()->schedulePeriodic(shared_from_this(), period, period); } -SimpleChannelSearchManagerImpl::~SimpleChannelSearchManagerImpl() +ChannelSearchManager::~ChannelSearchManager() { Lock guard(m_mutex); if (!m_canceled.get()) { - LOG(logLevelWarn, "Logic error: SimpleChannelSearchManagerImpl destroyed w/o cancel()"); + LOG(logLevelWarn, "Logic error: ChannelSearchManager destroyed w/o cancel()"); } } -void SimpleChannelSearchManagerImpl::cancel() +void ChannelSearchManager::cancel() { Lock guard(m_mutex); @@ -94,13 +94,13 @@ void SimpleChannelSearchManagerImpl::cancel() context->getTimer()->cancel(shared_from_this()); } -int32_t SimpleChannelSearchManagerImpl::registeredCount() +int32_t ChannelSearchManager::registeredCount() { Lock guard(m_channelMutex); return static_cast(m_channels.size()); } -void SimpleChannelSearchManagerImpl::registerSearchInstance(SearchInstance::shared_pointer const & channel, bool penalize) +void ChannelSearchManager::registerSearchInstance(SearchInstance::shared_pointer const & channel, bool penalize) { if (m_canceled.get()) return; @@ -122,7 +122,7 @@ void SimpleChannelSearchManagerImpl::registerSearchInstance(SearchInstance::shar callback(); } -void SimpleChannelSearchManagerImpl::unregisterSearchInstance(SearchInstance::shared_pointer const & channel) +void ChannelSearchManager::unregisterSearchInstance(SearchInstance::shared_pointer const & channel) { Lock guard(m_channelMutex); pvAccessID id = channel->getSearchInstanceID(); @@ -131,7 +131,7 @@ void SimpleChannelSearchManagerImpl::unregisterSearchInstance(SearchInstance::sh m_channels.erase(id); } -void SimpleChannelSearchManagerImpl::searchResponse(const ServerGUID & guid, pvAccessID cid, int32_t /*seqNo*/, int8_t minorRevision, osiSockAddr* serverAddress) +void ChannelSearchManager::searchResponse(const ServerGUID & guid, pvAccessID cid, int32_t /*seqNo*/, int8_t minorRevision, osiSockAddr* serverAddress) { Lock guard(m_channelMutex); m_channels_t::iterator channelsIter = m_channels.find(cid); @@ -162,13 +162,13 @@ void SimpleChannelSearchManagerImpl::searchResponse(const ServerGUID & guid, pvA } } -void SimpleChannelSearchManagerImpl::newServerDetected() +void ChannelSearchManager::newServerDetected() { boost(); callback(); } -void SimpleChannelSearchManagerImpl::initializeSendBuffer() +void ChannelSearchManager::initializeSendBuffer() { // for now OK, since it is only set here m_sequenceNumber++; @@ -202,7 +202,7 @@ void SimpleChannelSearchManagerImpl::initializeSendBuffer() m_sendBuffer.putShort((int16_t)0); // count } -void SimpleChannelSearchManagerImpl::flushSendBuffer() +void ChannelSearchManager::flushSendBuffer() { Lock guard(m_mutex); @@ -219,7 +219,7 @@ void SimpleChannelSearchManagerImpl::flushSendBuffer() } -bool SimpleChannelSearchManagerImpl::generateSearchRequestMessage(SearchInstance::shared_pointer const & channel, +bool ChannelSearchManager::generateSearchRequestMessage(SearchInstance::shared_pointer const & channel, ByteBuffer* requestMessage, TransportSendControl* control) { epics::pvData::int16 dataCount = requestMessage->getShort(DATA_COUNT_POSITION); @@ -245,7 +245,7 @@ bool SimpleChannelSearchManagerImpl::generateSearchRequestMessage(SearchInstance return true; } -bool SimpleChannelSearchManagerImpl::generateSearchRequestMessage(SearchInstance::shared_pointer const & channel, +bool ChannelSearchManager::generateSearchRequestMessage(SearchInstance::shared_pointer const & channel, bool allowNewFrame, bool flush) { Lock guard(m_mutex); @@ -267,7 +267,7 @@ bool SimpleChannelSearchManagerImpl::generateSearchRequestMessage(SearchInstance return flush; } -void SimpleChannelSearchManagerImpl::boost() +void ChannelSearchManager::boost() { Lock guard(m_channelMutex); Lock guard2(m_userValueMutex); @@ -281,7 +281,7 @@ void SimpleChannelSearchManagerImpl::boost() } } -void SimpleChannelSearchManagerImpl::callback() +void ChannelSearchManager::callback() { // high-frequency beacon anomaly trigger guard { @@ -346,12 +346,12 @@ void SimpleChannelSearchManagerImpl::callback() flushSendBuffer(); } -bool SimpleChannelSearchManagerImpl::isPowerOfTwo(int32_t x) +bool ChannelSearchManager::isPowerOfTwo(int32_t x) { return ((x > 0) && (x & (x - 1)) == 0); } -void SimpleChannelSearchManagerImpl::timerStopped() +void ChannelSearchManager::timerStopped() { } diff --git a/src/remote/pv/channelSearchManager.h b/src/remote/pv/channelSearchManager.h index 65c51e2..e9f3d25 100644 --- a/src/remote/pv/channelSearchManager.h +++ b/src/remote/pv/channelSearchManager.h @@ -20,6 +20,7 @@ #endif #include +#include namespace epics { namespace pvAccess { @@ -49,34 +50,56 @@ public: virtual void searchResponse(const ServerGUID & guid, int8_t minorRevision, osiSockAddr* serverAddress) = 0; }; -class ChannelSearchManager { + +class MockTransportSendControl: public TransportSendControl +{ +public: + void endMessage() {} + void flush(bool /*lastMessageCompleted*/) {} + void setRecipient(const osiSockAddr& /*sendTo*/) {} + void startMessage(epics::pvData::int8 /*command*/, std::size_t /*ensureCapacity*/, epics::pvData::int32 /*payloadSize*/) {} + void ensureBuffer(std::size_t /*size*/) {} + void alignBuffer(std::size_t /*alignment*/) {} + void flushSerializeBuffer() {} + void cachedSerialize(const std::tr1::shared_ptr& field, epics::pvData::ByteBuffer* buffer) + { + // no cache + field->serialize(buffer, this); + } + virtual bool directSerialize(epics::pvData::ByteBuffer* /*existingBuffer*/, const char* /*toSerialize*/, + std::size_t /*elementCount*/, std::size_t /*elementSize*/) + { + return false; + } +}; + +class ChannelSearchManager : + public epics::pvData::TimerCallback, + public std::tr1::enable_shared_from_this +{ public: POINTER_DEFINITIONS(ChannelSearchManager); + virtual ~ChannelSearchManager(); /** - * Destructor + * Cancel. */ - virtual ~ChannelSearchManager() {}; - + void cancel(); /** * Get number of registered channels. * @return number of registered channels. */ - virtual int32_t registeredCount() = 0; - + int32_t registeredCount(); /** * Register channel. - * @param channel + * @param channel to register. */ - virtual void registerSearchInstance(SearchInstance::shared_pointer const & channel, bool penalize = false) = 0; - - + void registerSearchInstance(SearchInstance::shared_pointer const & channel, bool penalize = false); /** * Unregister channel. - * @param channel + * @param channel to unregister. */ - virtual void unregisterSearchInstance(SearchInstance::shared_pointer const & channel) = 0; - + void unregisterSearchInstance(SearchInstance::shared_pointer const & channel); /** * Search response from server (channel found). * @param guid server GUID. @@ -85,19 +108,110 @@ public: * @param minorRevision server minor PVA revision. * @param serverAddress server address. */ - virtual void searchResponse(const ServerGUID & guid, pvAccessID cid, int32_t seqNo, int8_t minorRevision, osiSockAddr* serverAddress) = 0; - + void searchResponse(const ServerGUID & guid, pvAccessID cid, int32_t seqNo, int8_t minorRevision, osiSockAddr* serverAddress); /** * New server detected. * Boost searching of all channels. */ - virtual void newServerDetected() = 0; + void newServerDetected(); + + /// Timer callback. + virtual void callback() OVERRIDE FINAL; + + /// Timer stooped callback. + virtual void timerStopped() OVERRIDE FINAL; /** - * Cancel. + * Private constructor. + * @param context */ - virtual void cancel() = 0; + ChannelSearchManager(Context::shared_pointer const & context); + void activate(); +private: + + bool generateSearchRequestMessage(SearchInstance::shared_pointer const & channel, bool allowNewFrame, bool flush); + + static bool generateSearchRequestMessage(SearchInstance::shared_pointer const & channel, + epics::pvData::ByteBuffer* byteBuffer, TransportSendControl* control); + + void boost(); + + void initializeSendBuffer(); + void flushSendBuffer(); + + static bool isPowerOfTwo(int32_t x); + + /** + * Context. + */ + Context::weak_pointer m_context; + + /** + * Response address. + */ + osiSockAddr m_responseAddress; + + /** + * Canceled flag. + */ + AtomicBoolean m_canceled; + + /** + * Search (datagram) sequence number. + */ + int32_t m_sequenceNumber; + + /** + * Send byte buffer (frame) + */ + epics::pvData::ByteBuffer m_sendBuffer; + + /** + * Set of registered channels. + */ + typedef std::map m_channels_t; + m_channels_t m_channels; + + /** + * Time of last frame send. + */ + int64_t m_lastTimeSent; + + /** + * Mock transport send control + */ + MockTransportSendControl m_mockTransportSendControl; + + /** + * This instance mutex. + */ + epics::pvData::Mutex m_channelMutex; + + /** + * User value lock. + */ + epics::pvData::Mutex m_userValueMutex; + + /** + * m_channels mutex. + */ + epics::pvData::Mutex m_mutex; + + static const int DATA_COUNT_POSITION; + static const int CAST_POSITION; + static const int PAYLOAD_POSITION; + + static const double ATOMIC_PERIOD; + static const int PERIOD_JITTER_MS; + + static const int DEFAULT_USER_VALUE; + static const int BOOST_VALUE; + static const int MAX_COUNT_VALUE; + static const int MAX_FALLBACK_COUNT_VALUE; + + static const int MAX_FRAMES_AT_ONCE; + static const int DELAY_BETWEEN_FRAMES_MS; }; } diff --git a/src/remote/pv/simpleChannelSearchManagerImpl.h b/src/remote/pv/simpleChannelSearchManagerImpl.h deleted file mode 100644 index 6a54818..0000000 --- a/src/remote/pv/simpleChannelSearchManagerImpl.h +++ /dev/null @@ -1,204 +0,0 @@ -/** - * Copyright - See the COPYRIGHT that is included with this distribution. - * pvAccessCPP is distributed subject to a Software License Agreement found - * in file LICENSE that is included with this distribution. - */ - -#ifndef SIMPLECHANNELSEARCHMANAGERIMPL_H -#define SIMPLECHANNELSEARCHMANAGERIMPL_H - -#ifdef epicsExportSharedSymbols -# define simpleChannelSearchManagerEpicsExportSharedSymbols -# undef epicsExportSharedSymbols -#endif - -#include -#include -#include - -#ifdef simpleChannelSearchManagerEpicsExportSharedSymbols -# define epicsExportSharedSymbols -# undef simpleChannelSearchManagerEpicsExportSharedSymbols -#endif - -#include -#include - -namespace epics { -namespace pvAccess { - - -class MockTransportSendControl: public TransportSendControl -{ -public: - void endMessage() {} - void flush(bool /*lastMessageCompleted*/) {} - void setRecipient(const osiSockAddr& /*sendTo*/) {} - void startMessage(epics::pvData::int8 /*command*/, std::size_t /*ensureCapacity*/, epics::pvData::int32 /*payloadSize*/) {} - void ensureBuffer(std::size_t /*size*/) {} - void alignBuffer(std::size_t /*alignment*/) {} - void flushSerializeBuffer() {} - void cachedSerialize(const std::tr1::shared_ptr& field, epics::pvData::ByteBuffer* buffer) - { - // no cache - field->serialize(buffer, this); - } - virtual bool directSerialize(epics::pvData::ByteBuffer* /*existingBuffer*/, const char* /*toSerialize*/, - std::size_t /*elementCount*/, std::size_t /*elementSize*/) - { - return false; - } -}; - - -class SimpleChannelSearchManagerImpl : - public ChannelSearchManager, - public epics::pvData::TimerCallback, - public std::tr1::enable_shared_from_this -{ -public: - POINTER_DEFINITIONS(SimpleChannelSearchManagerImpl); - - /** - * Constructor. - * @param context - */ - virtual ~SimpleChannelSearchManagerImpl(); - /** - * Cancel. - */ - void cancel(); - /** - * Get number of registered channels. - * @return number of registered channels. - */ - int32_t registeredCount(); - /** - * Register channel. - * @param channel to register. - */ - void registerSearchInstance(SearchInstance::shared_pointer const & channel, bool penalize = false); - /** - * Unregister channel. - * @param channel to unregister. - */ - void unregisterSearchInstance(SearchInstance::shared_pointer const & channel); - /** - * Search response from server (channel found). - * @param guid server GUID. - * @param cid client channel ID. - * @param seqNo search sequence number. - * @param minorRevision server minor PVA revision. - * @param serverAddress server address. - */ - void searchResponse(const ServerGUID & guid, pvAccessID cid, int32_t seqNo, int8_t minorRevision, osiSockAddr* serverAddress); - /** - * New server detected. - * Boost searching of all channels. - */ - void newServerDetected(); - - /// Timer callback. - void callback(); - - /// Timer stooped callback. - void timerStopped(); - - /** - * Private constructor. - * @param context - */ - SimpleChannelSearchManagerImpl(Context::shared_pointer const & context); - void activate(); - -private: - - bool generateSearchRequestMessage(SearchInstance::shared_pointer const & channel, bool allowNewFrame, bool flush); - - static bool generateSearchRequestMessage(SearchInstance::shared_pointer const & channel, - epics::pvData::ByteBuffer* byteBuffer, TransportSendControl* control); - - void boost(); - - void initializeSendBuffer(); - void flushSendBuffer(); - - static bool isPowerOfTwo(int32_t x); - - /** - * Context. - */ - Context::weak_pointer m_context; - - /** - * Response address. - */ - osiSockAddr m_responseAddress; - - /** - * Canceled flag. - */ - AtomicBoolean m_canceled; - - /** - * Search (datagram) sequence number. - */ - int32_t m_sequenceNumber; - - /** - * Send byte buffer (frame) - */ - epics::pvData::ByteBuffer m_sendBuffer; - - /** - * Set of registered channels. - */ - typedef std::map m_channels_t; - m_channels_t m_channels; - - /** - * Time of last frame send. - */ - int64_t m_lastTimeSent; - - /** - * Mock transport send control - */ - MockTransportSendControl m_mockTransportSendControl; - - /** - * This instance mutex. - */ - epics::pvData::Mutex m_channelMutex; - - /** - * User value lock. - */ - epics::pvData::Mutex m_userValueMutex; - - /** - * m_channels mutex. - */ - epics::pvData::Mutex m_mutex; - - static const int DATA_COUNT_POSITION; - static const int CAST_POSITION; - static const int PAYLOAD_POSITION; - - static const double ATOMIC_PERIOD; - static const int PERIOD_JITTER_MS; - - static const int DEFAULT_USER_VALUE; - static const int BOOST_VALUE; - static const int MAX_COUNT_VALUE; - static const int MAX_FALLBACK_COUNT_VALUE; - - static const int MAX_FRAMES_AT_ONCE; - static const int DELAY_BETWEEN_FRAMES_MS; - -}; - -} -} - -#endif /* SIMPLECHANNELSEARCHMANAGERIMPL_H */ diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index 473b12e..0afeea0 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -28,7 +28,7 @@ #include #include #include -#include +#include #include #include #include @@ -4152,7 +4152,7 @@ private: // stores many weak_ptr m_responseHandler.reset(new ClientResponseHandler(thisPointer)); - m_channelSearchManager.reset(new SimpleChannelSearchManagerImpl(thisPointer)); + m_channelSearchManager.reset(new ChannelSearchManager(thisPointer)); // preinitialize security plugins SecurityPluginRegistry::instance(); @@ -4586,7 +4586,7 @@ private: * Channel search manager. * Manages UDP search requests. */ - SimpleChannelSearchManagerImpl::shared_pointer m_channelSearchManager; + ChannelSearchManager::shared_pointer m_channelSearchManager; /** * Beacon handler map. From a6d86d2a3c021831e41fc52be27d245cfd66cafb Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 28 Mar 2018 13:49:21 -0700 Subject: [PATCH 02/42] hide MockTransportSendControl --- src/remote/channelSearchManager.cpp | 39 ++++++++++++++++++++++++---- src/remote/pv/channelSearchManager.h | 27 ------------------- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/src/remote/channelSearchManager.cpp b/src/remote/channelSearchManager.cpp index c6ce5ab..cfb342b 100644 --- a/src/remote/channelSearchManager.cpp +++ b/src/remote/channelSearchManager.cpp @@ -20,6 +20,33 @@ using namespace std; using namespace epics::pvData; +namespace { +namespace pva = epics::pvAccess; + +class MockTransportSendControl: public pva::TransportSendControl +{ +public: + void endMessage() {} + void flush(bool /*lastMessageCompleted*/) {} + void setRecipient(const osiSockAddr& /*sendTo*/) {} + void startMessage(epics::pvData::int8 /*command*/, std::size_t /*ensureCapacity*/, epics::pvData::int32 /*payloadSize*/) {} + void ensureBuffer(std::size_t /*size*/) {} + void alignBuffer(std::size_t /*alignment*/) {} + void flushSerializeBuffer() {} + void cachedSerialize(const std::tr1::shared_ptr& field, epics::pvData::ByteBuffer* buffer) + { + // no cache + field->serialize(buffer, this); + } + virtual bool directSerialize(epics::pvData::ByteBuffer* /*existingBuffer*/, const char* /*toSerialize*/, + std::size_t /*elementCount*/, std::size_t /*elementSize*/) + { + return false; + } +}; + +}// namespace + namespace epics { namespace pvAccess { @@ -49,7 +76,6 @@ ChannelSearchManager::ChannelSearchManager(Context::shared_pointer const & conte m_sendBuffer(MAX_UDP_UNFRAGMENTED_SEND), m_channels(), m_lastTimeSent(), - m_mockTransportSendControl(), m_channelMutex(), m_userValueMutex(), m_mutex() @@ -197,8 +223,9 @@ void ChannelSearchManager::initializeSendBuffer() // TODO now only TCP is supported // note: this affects DATA_COUNT_POSITION m_sendBuffer.putByte((int8_t)1); - // TODO "tcp" constant - SerializeHelper::serializeString("tcp", &m_sendBuffer, &m_mockTransportSendControl); + + MockTransportSendControl control; + SerializeHelper::serializeString("tcp", &m_sendBuffer, &control); m_sendBuffer.putShort((int16_t)0); // count } @@ -248,14 +275,16 @@ bool ChannelSearchManager::generateSearchRequestMessage(SearchInstance::shared_p bool ChannelSearchManager::generateSearchRequestMessage(SearchInstance::shared_pointer const & channel, bool allowNewFrame, bool flush) { + MockTransportSendControl control; + Lock guard(m_mutex); - bool success = generateSearchRequestMessage(channel, &m_sendBuffer, &m_mockTransportSendControl); + bool success = generateSearchRequestMessage(channel, &m_sendBuffer, &control); // buffer full, flush if(!success) { flushSendBuffer(); if(allowNewFrame) - generateSearchRequestMessage(channel, &m_sendBuffer, &m_mockTransportSendControl); + generateSearchRequestMessage(channel, &m_sendBuffer, &control); if (flush) flushSendBuffer(); return true; diff --git a/src/remote/pv/channelSearchManager.h b/src/remote/pv/channelSearchManager.h index e9f3d25..5e4d850 100644 --- a/src/remote/pv/channelSearchManager.h +++ b/src/remote/pv/channelSearchManager.h @@ -51,28 +51,6 @@ public: }; -class MockTransportSendControl: public TransportSendControl -{ -public: - void endMessage() {} - void flush(bool /*lastMessageCompleted*/) {} - void setRecipient(const osiSockAddr& /*sendTo*/) {} - void startMessage(epics::pvData::int8 /*command*/, std::size_t /*ensureCapacity*/, epics::pvData::int32 /*payloadSize*/) {} - void ensureBuffer(std::size_t /*size*/) {} - void alignBuffer(std::size_t /*alignment*/) {} - void flushSerializeBuffer() {} - void cachedSerialize(const std::tr1::shared_ptr& field, epics::pvData::ByteBuffer* buffer) - { - // no cache - field->serialize(buffer, this); - } - virtual bool directSerialize(epics::pvData::ByteBuffer* /*existingBuffer*/, const char* /*toSerialize*/, - std::size_t /*elementCount*/, std::size_t /*elementSize*/) - { - return false; - } -}; - class ChannelSearchManager : public epics::pvData::TimerCallback, public std::tr1::enable_shared_from_this @@ -178,11 +156,6 @@ private: */ int64_t m_lastTimeSent; - /** - * Mock transport send control - */ - MockTransportSendControl m_mockTransportSendControl; - /** * This instance mutex. */ From 6c2d20353a80c74c5371c40baf76b95a5a86e93b Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 28 Mar 2018 13:59:33 -0700 Subject: [PATCH 03/42] minor, avoid extra string copys --- src/remote/channelSearchManager.cpp | 2 +- src/remote/pv/channelSearchManager.h | 4 ++-- src/remoteClient/clientContextImpl.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/remote/channelSearchManager.cpp b/src/remote/channelSearchManager.cpp index cfb342b..c5aaeda 100644 --- a/src/remote/channelSearchManager.cpp +++ b/src/remote/channelSearchManager.cpp @@ -258,7 +258,7 @@ bool ChannelSearchManager::generateSearchRequestMessage(SearchInstance::shared_p return false; */ - const std::string name = channel->getSearchInstanceName(); + const std::string& name(channel->getSearchInstanceName()); // not nice... const int addedPayloadSize = sizeof(int32)/sizeof(int8) + (1 + sizeof(int32)/sizeof(int8) + name.length()); if(((int)requestMessage->getRemaining()) < addedPayloadSize) diff --git a/src/remote/pv/channelSearchManager.h b/src/remote/pv/channelSearchManager.h index 5e4d850..43b53f9 100644 --- a/src/remote/pv/channelSearchManager.h +++ b/src/remote/pv/channelSearchManager.h @@ -32,11 +32,11 @@ public: /** * Destructor */ - virtual ~SearchInstance() {}; + virtual ~SearchInstance() {} virtual pvAccessID getSearchInstanceID() = 0; - virtual std::string getSearchInstanceName() = 0; + virtual const std::string& getSearchInstanceName() = 0; virtual int32_t& getUserValue() = 0; diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index 0afeea0..9c03dc5 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -3355,7 +3355,7 @@ public: return m_channelID; } - virtual string getSearchInstanceName() OVERRIDE FINAL { + virtual const string& getSearchInstanceName() OVERRIDE FINAL { return m_name; } From 38965bb8475529a649bfb8e04a6911a4ee067c8c Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 2 Apr 2018 10:24:35 -0700 Subject: [PATCH 04/42] codec: collapse internalDestroy() into internalClose() --- src/remote/codec.cpp | 78 ++++++++++++++++++++----------------------- src/remote/pv/codec.h | 3 -- 2 files changed, 36 insertions(+), 45 deletions(-) diff --git a/src/remote/codec.cpp b/src/remote/codec.cpp index cd45e83..6e4cb4f 100644 --- a/src/remote/codec.cpp +++ b/src/remote/codec.cpp @@ -1060,7 +1060,42 @@ void BlockingTCPTransportCodec::waitJoin() void BlockingTCPTransportCodec::internalClose(bool /*force*/) { - this->internalDestroy(); + if(_channel != INVALID_SOCKET) { + + epicsSocketSystemCallInterruptMechanismQueryInfo info = + epicsSocketSystemCallInterruptMechanismQuery (); + switch ( info ) + { + case esscimqi_socketCloseRequired: + epicsSocketDestroy ( _channel ); + break; + case esscimqi_socketBothShutdownRequired: + { + /*int status =*/ ::shutdown ( _channel, SHUT_RDWR ); + /* + if ( status ) { + char sockErrBuf[64]; + epicsSocketConvertErrnoToString ( + sockErrBuf, sizeof ( sockErrBuf ) ); + LOG(logLevelDebug, + "TCP socket to %s failed to shutdown: %s.", + inetAddressToString(_socketAddress).c_str(), sockErrBuf); + } + */ + epicsSocketDestroy ( _channel ); + } + break; + case esscimqi_socketSigAlarmRequired: + // not supported anymore anyway + default: + epicsSocketDestroy(_channel); + } + + _channel = INVALID_SOCKET; //TODO: mutex to guard _channel + } + + Transport::shared_pointer thisSharedPtr = this->shared_from_this(); + _context->getTransportRegistry()->remove(thisSharedPtr); // TODO sync if (_securitySession) @@ -1204,47 +1239,6 @@ BlockingTCPTransportCodec::BlockingTCPTransportCodec(bool serverFlag, const Cont } -// must be called only once, when there will be no operation on socket (e.g. just before tx/rx thread exists) -void BlockingTCPTransportCodec::internalDestroy() { - - if(_channel != INVALID_SOCKET) { - - epicsSocketSystemCallInterruptMechanismQueryInfo info = - epicsSocketSystemCallInterruptMechanismQuery (); - switch ( info ) - { - case esscimqi_socketCloseRequired: - epicsSocketDestroy ( _channel ); - break; - case esscimqi_socketBothShutdownRequired: - { - /*int status =*/ ::shutdown ( _channel, SHUT_RDWR ); - /* - if ( status ) { - char sockErrBuf[64]; - epicsSocketConvertErrnoToString ( - sockErrBuf, sizeof ( sockErrBuf ) ); - LOG(logLevelDebug, - "TCP socket to %s failed to shutdown: %s.", - inetAddressToString(_socketAddress).c_str(), sockErrBuf); - } - */ - epicsSocketDestroy ( _channel ); - } - break; - case esscimqi_socketSigAlarmRequired: - // not supported anymore anyway - default: - epicsSocketDestroy(_channel); - } - - _channel = INVALID_SOCKET; //TODO: mutex to guard _channel - } - - Transport::shared_pointer thisSharedPtr = this->shared_from_this(); - _context->getTransportRegistry()->remove(thisSharedPtr); -} - void BlockingTCPTransportCodec::invalidDataStreamHandler() { close(); diff --git a/src/remote/pv/codec.h b/src/remote/pv/codec.h index 631f421..5b75baa 100644 --- a/src/remote/pv/codec.h +++ b/src/remote/pv/codec.h @@ -338,9 +338,6 @@ public: return std::string("tcp"); } - - void internalDestroy(); - virtual void processControlMessage() OVERRIDE FINAL { if (_command == 2) { From da85d39c0cce2aef20a8dc0c4fdaf3e87e9f5459 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 2 Apr 2018 10:40:50 -0700 Subject: [PATCH 05/42] codec: const-ify socket Avoid possible data race of changing _channel before worker threads are joined. Was just using this as a boolean flag for a method which is already guarded in BlockingTCPTransportCodec::close() from being called twice. --- src/remote/codec.cpp | 4 +--- src/remote/pv/codec.h | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/remote/codec.cpp b/src/remote/codec.cpp index 6e4cb4f..f50ffbc 100644 --- a/src/remote/codec.cpp +++ b/src/remote/codec.cpp @@ -1060,7 +1060,7 @@ void BlockingTCPTransportCodec::waitJoin() void BlockingTCPTransportCodec::internalClose(bool /*force*/) { - if(_channel != INVALID_SOCKET) { + { epicsSocketSystemCallInterruptMechanismQueryInfo info = epicsSocketSystemCallInterruptMechanismQuery (); @@ -1090,8 +1090,6 @@ void BlockingTCPTransportCodec::internalClose(bool /*force*/) default: epicsSocketDestroy(_channel); } - - _channel = INVALID_SOCKET; //TODO: mutex to guard _channel } Transport::shared_pointer thisSharedPtr = this->shared_from_this(); diff --git a/src/remote/pv/codec.h b/src/remote/pv/codec.h index 5b75baa..92c5017 100644 --- a/src/remote/pv/codec.h +++ b/src/remote/pv/codec.h @@ -466,8 +466,8 @@ private: AtomicValue _isOpen; epics::pvData::Thread _readThread, _sendThread; epics::pvData::Event _shutdownEvent; + const SOCKET _channel; protected: - SOCKET _channel; osiSockAddr _socketAddress; std::string _socketName; protected: From 434a43dfe14b005ef7afeac8eef4a096208a9178 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 2 Apr 2018 10:46:57 -0700 Subject: [PATCH 06/42] codec: less force-ful shutdown eliminate unused 'force' argument. --- src/remote/codec.cpp | 18 +++++++++--------- src/remote/pv/codec.h | 18 +++++++----------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/remote/codec.cpp b/src/remote/codec.cpp index f50ffbc..3588af9 100644 --- a/src/remote/codec.cpp +++ b/src/remote/codec.cpp @@ -1040,14 +1040,14 @@ void BlockingTCPTransportCodec::close() { // wakeup processSendQueue // clean resources (close socket) - internalClose(true); + internalClose(); // Break sender from queue wait BreakTransport::shared_pointer B(new BreakTransport); enqueueSendRequest(B); // post close - internalPostClose(true); + internalPostClose(); } } @@ -1058,7 +1058,7 @@ void BlockingTCPTransportCodec::waitJoin() _readThread.exitWait(); } -void BlockingTCPTransportCodec::internalClose(bool /*force*/) +void BlockingTCPTransportCodec::internalClose() { { @@ -1585,9 +1585,9 @@ void BlockingServerTCPTransportCodec::destroyAllChannels() { it->second->destroy(); } -void BlockingServerTCPTransportCodec::internalClose(bool force) { +void BlockingServerTCPTransportCodec::internalClose() { Transport::shared_pointer thisSharedPtr = shared_from_this(); - BlockingTCPTransportCodec::internalClose(force); + BlockingTCPTransportCodec::internalClose(); destroyAllChannels(); } @@ -1771,15 +1771,15 @@ bool BlockingClientTCPTransportCodec::acquire(ClientChannelImpl::shared_pointer } // _mutex is held when this method is called -void BlockingClientTCPTransportCodec::internalClose(bool forced) { - BlockingTCPTransportCodec::internalClose(forced); +void BlockingClientTCPTransportCodec::internalClose() { + BlockingTCPTransportCodec::internalClose(); TimerCallbackPtr tcb = std::tr1::dynamic_pointer_cast(shared_from_this()); _context->getTimer()->cancel(tcb); } -void BlockingClientTCPTransportCodec::internalPostClose(bool forced) { - BlockingTCPTransportCodec::internalPostClose(forced); +void BlockingClientTCPTransportCodec::internalPostClose() { + BlockingTCPTransportCodec::internalPostClose(); // _owners cannot change when transport is closed closedNotifyClients(); diff --git a/src/remote/pv/codec.h b/src/remote/pv/codec.h index 92c5017..f4dffa8 100644 --- a/src/remote/pv/codec.h +++ b/src/remote/pv/codec.h @@ -449,18 +449,14 @@ protected: virtual void sendBufferFull(int tries) OVERRIDE FINAL; /** - * Called to any resources just before closing transport - * @param[in] force flag indicating if forced (e.g. forced - * disconnect) is required + * Called from close(). prior to signaling worker shutdown. */ - virtual void internalClose(bool force); + virtual void internalClose(); /** - * 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 + * Called from close(). after signaling worker shutdown. */ - virtual void internalPostClose(bool force) {} + virtual void internalPostClose() {} private: AtomicValue _isOpen; @@ -586,7 +582,7 @@ public: protected: void destroyAllChannels(); - virtual void internalClose(bool force) OVERRIDE FINAL; + virtual void internalClose() OVERRIDE FINAL; private: @@ -686,8 +682,8 @@ public: protected: - virtual void internalClose(bool force) OVERRIDE FINAL; - virtual void internalPostClose(bool force) OVERRIDE FINAL; + virtual void internalClose() OVERRIDE FINAL; + virtual void internalPostClose() OVERRIDE FINAL; private: From df14a19ae054f8b62b2babfbf716b2500772df5b Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 2 Apr 2018 11:30:19 -0700 Subject: [PATCH 07/42] codec: eliminate internalPostClose() no reason to distinguish before and after send worker break is _requested_ (won't actually be stopped). --- src/remote/codec.cpp | 8 -------- src/remote/pv/codec.h | 9 ++------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/src/remote/codec.cpp b/src/remote/codec.cpp index 3588af9..31e87b2 100644 --- a/src/remote/codec.cpp +++ b/src/remote/codec.cpp @@ -1045,9 +1045,6 @@ void BlockingTCPTransportCodec::close() { // Break sender from queue wait BreakTransport::shared_pointer B(new BreakTransport); enqueueSendRequest(B); - - // post close - internalPostClose(); } } @@ -1776,15 +1773,10 @@ void BlockingClientTCPTransportCodec::internalClose() { TimerCallbackPtr tcb = std::tr1::dynamic_pointer_cast(shared_from_this()); _context->getTimer()->cancel(tcb); -} - -void BlockingClientTCPTransportCodec::internalPostClose() { - BlockingTCPTransportCodec::internalPostClose(); // _owners cannot change when transport is closed closedNotifyClients(); } - /** * Notifies clients about disconnect. */ diff --git a/src/remote/pv/codec.h b/src/remote/pv/codec.h index f4dffa8..8f61c04 100644 --- a/src/remote/pv/codec.h +++ b/src/remote/pv/codec.h @@ -449,15 +449,11 @@ protected: virtual void sendBufferFull(int tries) OVERRIDE FINAL; /** - * Called from close(). prior to signaling worker shutdown. + * Called from close(). after start of shutdown (isOpen()==false) + * but before worker thread shutdown. */ virtual void internalClose(); - /** - * Called from close(). after signaling worker shutdown. - */ - virtual void internalPostClose() {} - private: AtomicValue _isOpen; epics::pvData::Thread _readThread, _sendThread; @@ -683,7 +679,6 @@ public: protected: virtual void internalClose() OVERRIDE FINAL; - virtual void internalPostClose() OVERRIDE FINAL; private: From eed26a166e4f3894e3a152a68d83274b0ca8344f Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 2 Apr 2018 11:45:33 -0700 Subject: [PATCH 08/42] testChannelAccess: loosen timestamp tests to reduce false positive failures --- testApp/remote/channelAccessIFTest.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/testApp/remote/channelAccessIFTest.cpp b/testApp/remote/channelAccessIFTest.cpp index c95c2a2..3f09a30 100644 --- a/testApp/remote/channelAccessIFTest.cpp +++ b/testApp/remote/channelAccessIFTest.cpp @@ -628,9 +628,9 @@ void ChannelAccessIFTest::test_channelGetIntProcessInternal(Channel::shared_poin pvTimeStamp.get(timeStamp); double deltaT = TimeStamp::diff(timeStamp, previousTimestamp); - testOk((previousValue +1)/*%11*/ == value->get(), "%s: testing the counter value change", - testMethodName.c_str()); - testOk(deltaT > 0.9 && deltaT < 2.0, + testOk((previousValue +1) == value->get(), "%s: testing the counter value change %d == %d", + testMethodName.c_str(), previousValue +1, (int)value->get()); + testOk(deltaT > 0.1 && deltaT < 20.0, "%s: timestamp change was %g", testMethodName.c_str(), deltaT); } @@ -1608,9 +1608,9 @@ void ChannelAccessIFTest::test_channelPutGetIntProcess() { //cout << "Testing1:" << testValue << " == " << getValuePtr->get() << endl; //cout << "Testing2:" << timeStamp.getSecondsPastEpoch() << ">" << previousTimestampSec << endl; - testOk( testValue == getValuePtr->get(), "%s: testing the counter value change", - CURRENT_FUNCTION); - testOk(deltaT > 0.9 && deltaT < 2.0, + testOk( testValue == getValuePtr->get(), "%s: testing the counter value change %d == %d", + CURRENT_FUNCTION, testValue, (int)getValuePtr->get()); + testOk(deltaT > 0.1 && deltaT < 20.0, "%s: timestamp change is %g", CURRENT_FUNCTION, deltaT); } From 28cec947ae43bd3bbc083cad528d52ad235551d4 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 2 Apr 2018 11:47:20 -0700 Subject: [PATCH 09/42] serverContext: collapse destroyAllTransports() into shutdown() --- src/server/pv/serverContextImpl.h | 5 ----- src/server/serverContext.cpp | 8 +------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/src/server/pv/serverContextImpl.h b/src/server/pv/serverContextImpl.h index 168032f..7e03dcf 100644 --- a/src/server/pv/serverContextImpl.h +++ b/src/server/pv/serverContextImpl.h @@ -259,11 +259,6 @@ private: */ void loadConfiguration(); - /** - * Destroy all transports. - */ - void destroyAllTransports(); - Configuration::const_shared_pointer configuration; epicsTimeStamp _startTime; diff --git a/src/server/serverContext.cpp b/src/server/serverContext.cpp index 3ba9364..3b31c8d 100644 --- a/src/server/serverContext.cpp +++ b/src/server/serverContext.cpp @@ -346,7 +346,7 @@ void ServerContextImpl::shutdown() } // this will also destroy all channels - destroyAllTransports(); + _transportRegistry.clear(); // drop timer queue LEAK_CHECK(_timer, "_timer") @@ -360,12 +360,6 @@ void ServerContextImpl::shutdown() _runEvent.signal(); } -void ServerContextImpl::destroyAllTransports() -{ - // now clear all (release) - _transportRegistry.clear(); -} - void ServerContext::printInfo(int lvl) { printInfo(cout, lvl); From d6a29a2a8663a5ea8bd873974c32f82510c8e5b2 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 2 Apr 2018 11:53:00 -0700 Subject: [PATCH 10/42] beaconEmitter const-ify data members --- src/server/pv/beaconEmitter.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/server/pv/beaconEmitter.h b/src/server/pv/beaconEmitter.h index 56ead21..badcd7a 100644 --- a/src/server/pv/beaconEmitter.h +++ b/src/server/pv/beaconEmitter.h @@ -95,7 +95,7 @@ private: /** * Protocol. */ - std::string _protocol; + const std::string _protocol; /** * Transport. @@ -110,22 +110,22 @@ private: /** * Server GUID. */ - ServerGUID _guid; + const ServerGUID _guid; /** * Fast (at startup) beacon period (in sec). */ - double _fastBeaconPeriod; + const double _fastBeaconPeriod; /** * Slow (after beaconCountLimit is reached) beacon period (in sec). */ - double _slowBeaconPeriod; + const double _slowBeaconPeriod; /** * Limit on number of beacons issued. */ - epics::pvData::int16 _beaconCountLimit; + const epics::pvData::int16 _beaconCountLimit; /** * Server address. @@ -135,7 +135,7 @@ private: /** * Server port. */ - epics::pvData::int32 _serverPort; + const epics::pvData::int32 _serverPort; /** * Server status provider implementation (optional). From abed887d875ed76ae01457ee1456f43923919b4e Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 2 Apr 2018 12:38:42 -0700 Subject: [PATCH 11/42] beaconEmitter: avoid ref. loop with Timer emitter schedules itself and timer holds a ref. while queued. So the emitter isn't destroyed until the timer expires. --- src/server/beaconEmitter.cpp | 12 +++++++++--- src/server/pv/beaconEmitter.h | 7 ++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/server/beaconEmitter.cpp b/src/server/beaconEmitter.cpp index b1b0ea4..9001085 100644 --- a/src/server/beaconEmitter.cpp +++ b/src/server/beaconEmitter.cpp @@ -109,12 +109,16 @@ void BeaconEmitter::timerStopped() void BeaconEmitter::destroy() { - _timer->cancel(shared_from_this()); + Timer::shared_pointer timer(_timer.lock()); + if(timer) + timer->cancel(shared_from_this()); } void BeaconEmitter::start() { - _timer->scheduleAfterDelay(shared_from_this(), 0.0); + Timer::shared_pointer timer(_timer.lock()); + if(timer) + timer->scheduleAfterDelay(shared_from_this(), 0.0); } void BeaconEmitter::reschedule() @@ -122,7 +126,9 @@ void BeaconEmitter::reschedule() const double period = (_beaconSequenceID >= _beaconCountLimit) ? _slowBeaconPeriod : _fastBeaconPeriod; if (period > 0) { - _timer->scheduleAfterDelay(shared_from_this(), period); + Timer::shared_pointer timer(_timer.lock()); + if(timer) + timer->scheduleAfterDelay(shared_from_this(), period); } } diff --git a/src/server/pv/beaconEmitter.h b/src/server/pv/beaconEmitter.h index badcd7a..cdc68d9 100644 --- a/src/server/pv/beaconEmitter.h +++ b/src/server/pv/beaconEmitter.h @@ -142,10 +142,11 @@ private: */ BeaconServerStatusProvider::shared_pointer _serverStatusProvider; - /** - * Timer. + /** Timer is referenced by server context, which also references us. + * We will also be queuing ourselves, and be referenced by Timer. + * So keep only a weak ref to Timer to avoid possible ref. loop. */ - epics::pvData::Timer::shared_pointer _timer; + epics::pvData::Timer::weak_pointer _timer; }; } From a3d820cf5731e31dc29620e3ee859f4baecf18f9 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 2 Apr 2018 12:39:46 -0700 Subject: [PATCH 12/42] DefaultBeaconServerStatusProvider avoid unnecessary ServerContext ref breaks loop ServerContext -> beaconEmitter -> BSSP -> ServerContext --- src/server/beaconServerStatusProvider.cpp | 45 ++++++---------------- src/server/pv/beaconServerStatusProvider.h | 9 ----- 2 files changed, 12 insertions(+), 42 deletions(-) diff --git a/src/server/beaconServerStatusProvider.cpp b/src/server/beaconServerStatusProvider.cpp index ccebc05..c533c0b 100644 --- a/src/server/beaconServerStatusProvider.cpp +++ b/src/server/beaconServerStatusProvider.cpp @@ -13,40 +13,19 @@ using namespace epics::pvData; namespace epics { namespace pvAccess { -DefaultBeaconServerStatusProvider::DefaultBeaconServerStatusProvider(ServerContext::shared_pointer const & context): _context(context) -{ - initialize(); -} +DefaultBeaconServerStatusProvider::DefaultBeaconServerStatusProvider(ServerContext::shared_pointer const & context) + :_status(getPVDataCreate()->createPVStructure(getFieldCreate()->createFieldBuilder() + ->add("connections", pvInt) + ->add("connections", pvInt) + ->add("allocatedMemory", pvLong) + ->add("freeMemory", pvLong) + ->add("threads", pvInt) + ->add("deadlocks", pvInt) + ->add("averageSystemLoad", pvDouble) + ->createStructure())) +{} -DefaultBeaconServerStatusProvider::~DefaultBeaconServerStatusProvider() -{ -} - -void DefaultBeaconServerStatusProvider::initialize() -{ - FieldCreatePtr fieldCreate = getFieldCreate(); - - StringArray fieldNames; - fieldNames.resize(6); - fieldNames[0] = "connections"; - fieldNames[1] = "allocatedMemory"; - fieldNames[2] = "freeMemory"; - fieldNames[3] = "threads"; - fieldNames[4] = "deadlocks"; - fieldNames[5] = "averageSystemLoad"; - - FieldConstPtrArray fields; - fields.resize(6); - // TODO hierarchy can be used... - fields[0] = fieldCreate->createScalar(pvInt); - fields[1] = fieldCreate->createScalar(pvLong); - fields[2] = fieldCreate->createScalar(pvLong); - fields[3] = fieldCreate->createScalar(pvInt); - fields[4] = fieldCreate->createScalar(pvInt); - fields[5] = fieldCreate->createScalar(pvDouble); - - _status = getPVDataCreate()->createPVStructure(fieldCreate->createStructure(fieldNames, fields)); -} +DefaultBeaconServerStatusProvider::~DefaultBeaconServerStatusProvider() {} PVField::shared_pointer DefaultBeaconServerStatusProvider::getServerStatusData() { diff --git a/src/server/pv/beaconServerStatusProvider.h b/src/server/pv/beaconServerStatusProvider.h index 0d681f8..e79925a 100644 --- a/src/server/pv/beaconServerStatusProvider.h +++ b/src/server/pv/beaconServerStatusProvider.h @@ -63,17 +63,8 @@ public: virtual epics::pvData::PVField::shared_pointer getServerStatusData(); -private: - /** - * Initialize - */ - void initialize(); - - private: epics::pvData::PVStructure::shared_pointer _status; - std::tr1::shared_ptr _context; - //ServerContext::shared_pointer _context; }; } From 385979340550cc68548b3735164864d6d0d66953 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 2 Apr 2018 12:40:18 -0700 Subject: [PATCH 13/42] serverContext explicitly close/flush Timer queue --- src/server/serverContext.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/server/serverContext.cpp b/src/server/serverContext.cpp index 3b31c8d..7cd9b62 100644 --- a/src/server/serverContext.cpp +++ b/src/server/serverContext.cpp @@ -307,6 +307,12 @@ void ServerContextImpl::run(uint32 seconds) void ServerContextImpl::shutdown() { + if(!_timer) + return; // already shutdown + + // abort pending timers and prevent new timers from starting + _timer->close(); + // stop responding to search requests for (BlockingUDPTransportVector::const_iterator iter = _udpTransports.begin(); iter != _udpTransports.end(); iter++) From fe5780521b1d49069496d02805b1233b4e949411 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 09:09:43 -0700 Subject: [PATCH 14/42] Context::getSecurityPlugins() no return of non-const ref. Return of map as non-const ref. allows callers to modify. Probably unintended. --- src/remote/codec.cpp | 13 +++++-------- src/remote/pv/remote.h | 3 ++- src/remote/pv/security.h | 8 ++++---- src/remoteClient/clientContextImpl.cpp | 2 +- src/server/pv/serverContextImpl.h | 2 +- src/server/serverContext.cpp | 2 +- 6 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/remote/codec.cpp b/src/remote/codec.cpp index 31e87b2..ab89e31 100644 --- a/src/remote/codec.cpp +++ b/src/remote/codec.cpp @@ -1518,12 +1518,11 @@ void BlockingServerTCPTransportCodec::send(ByteBuffer* buffer, buffer->putShort(0x7FFF); // list of authNZ plugin names - map& securityPlugins = _context->getSecurityPlugins(); + const Context::securityPlugins_t& securityPlugins = _context->getSecurityPlugins(); vector validSPNames; validSPNames.reserve(securityPlugins.size()); - for (map::const_iterator iter = - securityPlugins.begin(); + for (Context::securityPlugins_t::const_iterator iter(securityPlugins.begin()); iter != securityPlugins.end(); iter++) { SecurityPlugin::shared_pointer securityPlugin = iter->second; @@ -1616,8 +1615,7 @@ void BlockingServerTCPTransportCodec::authNZInitialize(const std::string& securi // check if plug-in name is valid SecurityPlugin::shared_pointer securityPlugin; - map::iterator spIter = - _context->getSecurityPlugins().find(securityPluginName); + Context::securityPlugins_t::const_iterator spIter(_context->getSecurityPlugins().find(securityPluginName)); if (spIter != _context->getSecurityPlugins().end()) securityPlugin = spIter->second; if (!securityPlugin) @@ -1921,13 +1919,12 @@ void BlockingClientTCPTransportCodec::authNZInitialize(const std::vector& availableSecurityPlugins = - _context->getSecurityPlugins(); + const Context::securityPlugins_t& availableSecurityPlugins(_context->getSecurityPlugins()); for (vector::const_iterator offeredSP = offeredSecurityPlugins.begin(); offeredSP != offeredSecurityPlugins.end(); offeredSP++) { - map::iterator spi = availableSecurityPlugins.find(*offeredSP); + Context::securityPlugins_t::const_iterator spi(availableSecurityPlugins.find(*offeredSP)); if (spi != availableSecurityPlugins.end()) { SecurityPlugin::shared_pointer securityPlugin = spi->second; diff --git a/src/remote/pv/remote.h b/src/remote/pv/remote.h index 2640067..dcfc378 100644 --- a/src/remote/pv/remote.h +++ b/src/remote/pv/remote.h @@ -317,11 +317,12 @@ public: virtual Configuration::const_shared_pointer getConfiguration() = 0; + typedef std::map > securityPlugins_t; /** * Get map of available security plug-ins. * @return the map of available security plug-ins */ - virtual std::map >& getSecurityPlugins() = 0; + virtual const securityPlugins_t& getSecurityPlugins() = 0; /// diff --git a/src/remote/pv/security.h b/src/remote/pv/security.h index 072c955..b227de6 100644 --- a/src/remote/pv/security.h +++ b/src/remote/pv/security.h @@ -423,12 +423,12 @@ public: return thisInstance; } - std::map >& getClientSecurityPlugins() + Context::securityPlugins_t& getClientSecurityPlugins() { return m_clientSecurityPlugins; } - std::map >& getServerSecurityPlugins() + Context::securityPlugins_t& getServerSecurityPlugins() { return m_serverSecurityPlugins; } @@ -448,8 +448,8 @@ public: private: SecurityPluginRegistry(); - std::map > m_clientSecurityPlugins; - std::map > m_serverSecurityPlugins; + Context::securityPlugins_t m_clientSecurityPlugins; + Context::securityPlugins_t m_serverSecurityPlugins; }; } diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index 9c03dc5..2c64e3b 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -4472,7 +4472,7 @@ private: } } - std::map >& getSecurityPlugins() OVERRIDE FINAL + const securityPlugins_t& getSecurityPlugins() OVERRIDE FINAL { return SecurityPluginRegistry::instance().getClientSecurityPlugins(); } diff --git a/src/server/pv/serverContextImpl.h b/src/server/pv/serverContextImpl.h index 7e03dcf..b01349e 100644 --- a/src/server/pv/serverContextImpl.h +++ b/src/server/pv/serverContextImpl.h @@ -51,7 +51,7 @@ public: Transport::shared_pointer getSearchTransport() OVERRIDE FINAL; Configuration::const_shared_pointer getConfiguration() OVERRIDE FINAL; TransportRegistry* getTransportRegistry() OVERRIDE FINAL; - std::map >& getSecurityPlugins() OVERRIDE FINAL; + const securityPlugins_t& getSecurityPlugins() OVERRIDE FINAL; virtual void newServerDetected() OVERRIDE FINAL; diff --git a/src/server/serverContext.cpp b/src/server/serverContext.cpp index 7cd9b62..0812fcc 100644 --- a/src/server/serverContext.cpp +++ b/src/server/serverContext.cpp @@ -540,7 +540,7 @@ epicsTimeStamp& ServerContextImpl::getStartTime() } -std::map >& ServerContextImpl::getSecurityPlugins() +const Context::securityPlugins_t& ServerContextImpl::getSecurityPlugins() { return SecurityPluginRegistry::instance().getServerSecurityPlugins(); } From f2164ddb5a7dd97c09d0ab5a681b97ddd3bc059f Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 10:12:05 -0700 Subject: [PATCH 15/42] fix typedef in security.h Can't refer to typedef in (private) remote.h from (public) security.h --- src/remote/pv/security.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/remote/pv/security.h b/src/remote/pv/security.h index b227de6..202408e 100644 --- a/src/remote/pv/security.h +++ b/src/remote/pv/security.h @@ -423,12 +423,14 @@ public: return thisInstance; } - Context::securityPlugins_t& getClientSecurityPlugins() + typedef std::map > securityPlugins_t; + + securityPlugins_t& getClientSecurityPlugins() { return m_clientSecurityPlugins; } - Context::securityPlugins_t& getServerSecurityPlugins() + securityPlugins_t& getServerSecurityPlugins() { return m_serverSecurityPlugins; } @@ -448,8 +450,8 @@ public: private: SecurityPluginRegistry(); - Context::securityPlugins_t m_clientSecurityPlugins; - Context::securityPlugins_t m_serverSecurityPlugins; + securityPlugins_t m_clientSecurityPlugins; + securityPlugins_t m_serverSecurityPlugins; }; } From 40ecc93e9d2d6496bc0f2aeaf2b81836f268cfa3 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 09:46:25 -0700 Subject: [PATCH 16/42] client context: use mutex guards whether this locking is necessary is another question... --- src/remoteClient/clientContextImpl.cpp | 28 ++++++++++++++++---------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index 2c64e3b..bdcb36c 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -11,6 +11,8 @@ #include #include +#include + #include #include #include @@ -915,6 +917,7 @@ public: } } + // TODO: m_structure and m_bitSet guarded by m_structureMutex? (as below) if (!(*m_structure->getStructure() == *pvPutStructure->getStructure())) { EXCEPTION_GUARD3(m_callback, cb, cb->putDone(invalidPutStructureStatus, thisPtr)); @@ -933,10 +936,11 @@ public: } try { - lock(); - *m_bitSet = *pvPutBitSet; - m_structure->copyUnchecked(*pvPutStructure, *m_bitSet); - unlock(); + { + epicsGuard G(*this); + *m_bitSet = *pvPutBitSet; + m_structure->copyUnchecked(*pvPutStructure, *m_bitSet); + } m_channel->checkAndGetTransport()->enqueueSendRequest(internal_from_this()); } catch (std::runtime_error &rte) { abortRequest(); @@ -1182,10 +1186,11 @@ public: } try { - lock(); - *m_putDataBitSet = *bitSet; - m_putData->copyUnchecked(*pvPutStructure, *m_putDataBitSet); - unlock(); + { + epicsGuard G(*this); + *m_putDataBitSet = *bitSet; + m_putData->copyUnchecked(*pvPutStructure, *m_putDataBitSet); + } m_channel->checkAndGetTransport()->enqueueSendRequest(internal_from_this()); } catch (std::runtime_error &rte) { abortRequest(); @@ -1420,9 +1425,10 @@ public: } try { - m_structureMutex.lock(); - m_structure = pvArgument; - m_structureMutex.unlock(); + { + epicsGuard G(m_structureMutex); + m_structure = pvArgument; + } m_channel->checkAndGetTransport()->enqueueSendRequest(internal_from_this()); } catch (std::runtime_error &rte) { From 8068ab3454bc006610773f7ab77eb683e5c031fb Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 09:49:50 -0700 Subject: [PATCH 17/42] eliminate unnecessary m_nullMonitorElement --- src/remoteClient/clientContextImpl.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index bdcb36c..14c2547 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -1850,9 +1850,6 @@ private: MonitorElement::shared_pointer m_overrunElement; bool m_overrunInProgress; - - MonitorElement::shared_pointer m_nullMonitorElement; - PVStructure::shared_pointer m_up2datePVStructure; int32 m_releasedCount; @@ -1878,7 +1875,6 @@ public: m_monitorQueue(), m_callback(callback), m_mutex(), m_bitSet1(), m_bitSet2(), m_overrunInProgress(false), - m_nullMonitorElement(), m_releasedCount(0), m_reportQueueStateInProgress(false), m_channel(channel), m_ioid(ioid), @@ -2013,10 +2009,10 @@ public: guard.unlock(); EXCEPTION_GUARD3(m_callback, cb, cb->unlisten(shared_from_this())); } - return m_nullMonitorElement; + return MonitorElement::shared_pointer(); } - MonitorElement::shared_pointer retVal = m_monitorQueue.front(); + MonitorElement::shared_pointer retVal(m_monitorQueue.front()); m_monitorQueue.pop(); return retVal; } From 9e090fa19152d9c13f1d09cdd99b32829d4f9895 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 10:02:21 -0700 Subject: [PATCH 18/42] client context: avoid lock order violations avoid ordering violation of calling checkAndGetTransport(), which locks channel mutex, while request mutex is locked. --- src/remoteClient/clientContextImpl.cpp | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index 14c2547..4050bcc 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -2057,14 +2057,18 @@ public: if (sendAck) { + guard.unlock(); + try { m_channel->checkAndGetTransport()->enqueueSendRequest(shared_from_this()); } catch (std::runtime_error&) { // assume wrong connection state from checkAndGetTransport() + guard.lock(); m_reportQueueStateInProgress = false; } catch (std::exception& e) { LOG(logLevelWarn, "Ignore exception during MonitorStrategyQueue::release: %s", e.what()); + guard.lock(); m_reportQueueStateInProgress = false; } } @@ -2389,12 +2393,19 @@ public: if (!startRequest(QOS_PROCESS | QOS_GET)) return BaseRequestImpl::otherRequestPendingStatus; + bool restore = m_started; + m_started = true; + + guard.unlock(); + try { m_channel->checkAndGetTransport()->enqueueSendRequest(internal_from_this()); - m_started = true; return Status::Ok; } catch (std::runtime_error &rte) { + guard.lock(); + + m_started = restore; abortRequest(); return BaseRequestImpl::channelNotConnected; } @@ -2415,12 +2426,19 @@ public: if (!startRequest(QOS_PROCESS)) return BaseRequestImpl::otherRequestPendingStatus; + bool restore = m_started; + m_started = false; + + guard.unlock(); + try { m_channel->checkAndGetTransport()->enqueueSendRequest(internal_from_this()); - m_started = false; return Status::Ok; } catch (std::runtime_error &rte) { + guard.lock(); + + m_started = restore; abortRequest(); return BaseRequestImpl::channelNotConnected; } From 18491f6eb3d97d8fcf6c372593af05764c2f0c16 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 10:12:33 -0700 Subject: [PATCH 19/42] const-ify AbstractClientResponseHandler::_context --- src/remoteClient/clientContextImpl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index 4050bcc..26c4d23 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -2466,7 +2466,7 @@ public: class AbstractClientResponseHandler : public ResponseHandler { protected: - ClientContextImpl::weak_pointer _context; + const ClientContextImpl::weak_pointer _context; public: AbstractClientResponseHandler(ClientContextImpl::shared_pointer const & context, string const & description) : ResponseHandler(context.get(), description), _context(ClientContextImpl::weak_pointer(context)) { From 83a941c07090468393256c9fb22e0f047711af97 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 14:53:52 -0700 Subject: [PATCH 20/42] channelSearchManager minor --- src/remote/channelSearchManager.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/remote/channelSearchManager.cpp b/src/remote/channelSearchManager.cpp index c5aaeda..0fe9da1 100644 --- a/src/remote/channelSearchManager.cpp +++ b/src/remote/channelSearchManager.cpp @@ -152,9 +152,7 @@ void ChannelSearchManager::unregisterSearchInstance(SearchInstance::shared_point { Lock guard(m_channelMutex); pvAccessID id = channel->getSearchInstanceID(); - m_channels_t::iterator channelsIter = m_channels.find(id); - if(channelsIter != m_channels.end()) - m_channels.erase(id); + m_channels.erase(id); } void ChannelSearchManager::searchResponse(const ServerGUID & guid, pvAccessID cid, int32_t /*seqNo*/, int8_t minorRevision, osiSockAddr* serverAddress) From 192955e7b5d9a63d37ce0d09a94d915b2a3e8b70 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 14:55:41 -0700 Subject: [PATCH 21/42] client context: don't call cancel() it doesn't do anything! --- src/remoteClient/clientContextImpl.cpp | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index 26c4d23..ac89519 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -3452,10 +3452,6 @@ public: m_transport->enqueueSendRequest(internal_from_this()); } - virtual void cancel() { - // noop - } - virtual void timeout() { createChannelFailed(); } @@ -3466,9 +3462,6 @@ public: virtual void createChannelFailed() OVERRIDE FINAL { Lock guard(m_channelMutex); - - cancel(); - // release transport if active if (m_transport) { @@ -3496,7 +3489,6 @@ public: if (m_connectionState == DESTROYED) { // end connection request - cancel(); return; } @@ -3516,10 +3508,6 @@ public: LOG(logLevelError, "connectionCompleted() %d '%s' unhandled exception: %s\n", sid, m_name.c_str(), e.what()); // noop } - - // NOTE: always call cancel - // end connection request - cancel(); } // should be called without any lock hold @@ -3561,7 +3549,6 @@ public: // stop searching... shared_pointer thisChannelPointer = internal_from_this(); m_context->getChannelSearchManager()->unregisterSearchInstance(thisChannelPointer); - cancel(); disconnectPendingIO(true); @@ -3604,7 +3591,6 @@ public: if (!initiateSearch) { // stop searching... m_context->getChannelSearchManager()->unregisterSearchInstance(internal_from_this()); - cancel(); } setConnectionState(DISCONNECTED); From 0eabf10005422b6e64bce2c920f2fe6471eda14e Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 15:41:38 -0700 Subject: [PATCH 22/42] codec: drop unused _shutdownEvent using epicsThread::exitWait() instead --- src/remote/codec.cpp | 2 -- src/remote/pv/codec.h | 1 - 2 files changed, 3 deletions(-) diff --git a/src/remote/codec.cpp b/src/remote/codec.cpp index ab89e31..793c480 100644 --- a/src/remote/codec.cpp +++ b/src/remote/codec.cpp @@ -1142,8 +1142,6 @@ void BlockingTCPTransportCodec::receiveThread() __FILE__, __LINE__); } } - - this->_shutdownEvent.signal(); } diff --git a/src/remote/pv/codec.h b/src/remote/pv/codec.h index 8f61c04..c1563af 100644 --- a/src/remote/pv/codec.h +++ b/src/remote/pv/codec.h @@ -457,7 +457,6 @@ protected: private: AtomicValue _isOpen; epics::pvData::Thread _readThread, _sendThread; - epics::pvData::Event _shutdownEvent; const SOCKET _channel; protected: osiSockAddr _socketAddress; From 7f5eb1de4d5373c89444aba0619b176cf458b269 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 15:55:23 -0700 Subject: [PATCH 23/42] BlockingClientTCPTransportCodec collapse closedNotifyClients() into internalClose() --- src/remote/codec.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/remote/codec.cpp b/src/remote/codec.cpp index 793c480..6656d9f 100644 --- a/src/remote/codec.cpp +++ b/src/remote/codec.cpp @@ -1771,12 +1771,8 @@ void BlockingClientTCPTransportCodec::internalClose() { _context->getTimer()->cancel(tcb); // _owners cannot change when transport is closed - closedNotifyClients(); -} -/** - * Notifies clients about disconnect. - */ -void BlockingClientTCPTransportCodec::closedNotifyClients() { + + // Notifies clients about disconnect. // check if still acquired size_t refs = _owners.size(); From a8e26123c7f6bc8bef2c17979b9a939324b65e2d Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 16:01:08 -0700 Subject: [PATCH 24/42] minor doc --- src/remote/pv/security.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/remote/pv/security.h b/src/remote/pv/security.h index 202408e..9e95e39 100644 --- a/src/remote/pv/security.h +++ b/src/remote/pv/security.h @@ -188,6 +188,8 @@ public: * @param remoteAddress * @return a new session. * @throws SecurityException + * + * @warning a Ref. loop is created if the SecuritySession stores a pointer to 'control' */ // authentication must be done immediately when connection is established (timeout 3seconds), // later on authentication process can be repeated From 54bb30fd86e3acd94022c15f5c866b82cbacc6b9 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 16:52:21 -0700 Subject: [PATCH 25/42] reftrack BlockingUDPTransport --- src/factory/ChannelAccessFactory.cpp | 2 ++ src/remote/blockingUDPTransport.cpp | 6 ++++++ src/remote/pv/blockingUDP.h | 2 ++ 3 files changed, 10 insertions(+) diff --git a/src/factory/ChannelAccessFactory.cpp b/src/factory/ChannelAccessFactory.cpp index 90af176..9f5a705 100644 --- a/src/factory/ChannelAccessFactory.cpp +++ b/src/factory/ChannelAccessFactory.cpp @@ -24,6 +24,7 @@ #include "pv/codec.h" #include #include +#include using namespace epics::pvData; using std::string; @@ -173,6 +174,7 @@ void providerRegInit(void*) registerRefCounter("ServerChannel", &ServerChannel::num_instances); registerRefCounter("Transport (ABC)", &Transport::num_instances); registerRefCounter("BlockingTCPTransportCodec", &detail::BlockingTCPTransportCodec::num_instances); + registerRefCounter("BlockingUDPTransport", &BlockingUDPTransport::num_instances); registerRefCounter("ChannelProvider (ABC)", &ChannelProvider::num_instances); registerRefCounter("Channel (ABC)", &Channel::num_instances); registerRefCounter("ChannelRequester (ABC)", &ChannelRequester::num_instances); diff --git a/src/remote/blockingUDPTransport.cpp b/src/remote/blockingUDPTransport.cpp index 844ed9e..67ffe8d 100644 --- a/src/remote/blockingUDPTransport.cpp +++ b/src/remote/blockingUDPTransport.cpp @@ -17,6 +17,7 @@ #include #include +#include #define epicsExportSharedSymbols #include @@ -42,6 +43,8 @@ inline int sendto(int s, const char *buf, size_t len, int flags, const struct so // reserve some space for CMD_ORIGIN_TAG message #define RECEIVE_BUFFER_PRE_RESERVE (PVA_MESSAGE_HEADER_SIZE + 16) +size_t BlockingUDPTransport::num_instances; + BlockingUDPTransport::BlockingUDPTransport(bool serverFlag, ResponseHandler::shared_pointer const & responseHandler, SOCKET channel, osiSockAddr& bindAddress, @@ -79,9 +82,12 @@ BlockingUDPTransport::BlockingUDPTransport(bool serverFlag, sockAddrToDottedIP(&_remoteAddress.sa, strBuffer, sizeof(strBuffer)); _remoteName = strBuffer; } + + REFTRACE_INCREMENT(num_instances); } BlockingUDPTransport::~BlockingUDPTransport() { + REFTRACE_DECREMENT(num_instances); close(true); // close the socket and stop the thread. } diff --git a/src/remote/pv/blockingUDP.h b/src/remote/pv/blockingUDP.h index 143e1c7..6cec9ac 100644 --- a/src/remote/pv/blockingUDP.h +++ b/src/remote/pv/blockingUDP.h @@ -49,6 +49,8 @@ class BlockingUDPTransport : public epics::pvData::NoDefaultMethods, public: POINTER_DEFINITIONS(BlockingUDPTransport); + static size_t num_instances; + private: std::tr1::weak_ptr internal_this; friend class BlockingUDPConnector; From bab32c332a316b07d4578fb80c04ce5595ebecfa Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 18:40:46 -0700 Subject: [PATCH 26/42] codec: notes on thread self dtor --- src/remote/codec.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/remote/codec.cpp b/src/remote/codec.cpp index 6656d9f..8efa03c 100644 --- a/src/remote/codec.cpp +++ b/src/remote/codec.cpp @@ -1126,13 +1126,23 @@ void BlockingTCPTransportCodec::start() { void BlockingTCPTransportCodec::receiveThread() { - Transport::shared_pointer ptr = this->shared_from_this(); + /* This innocuous ref. is an important hack. + * The code behind Transport::close() will cause + * channels and operations to drop references + * to this transport. This ref. keeps it from + * being destroyed way down the call stack, from + * which it is apparently not possible to return + * safely. Rather than try to untangle this + * knot, just keep this ref... + */ + Transport::shared_pointer ptr(this->shared_from_this()); while (this->isOpen()) { try { this->processRead(); } catch (std::exception &e) { + PRINT_EXCEPTION(e); LOG(logLevelError, "an exception caught while in receiveThread at %s:%d: %s", __FILE__, __LINE__, e.what()); @@ -1147,7 +1157,8 @@ void BlockingTCPTransportCodec::receiveThread() void BlockingTCPTransportCodec::sendThread() { - Transport::shared_pointer ptr = this->shared_from_this(); + // cf. the comment in receiveThread() + Transport::shared_pointer ptr(this->shared_from_this()); this->setSenderThread(); @@ -1158,6 +1169,7 @@ void BlockingTCPTransportCodec::sendThread() } catch (connection_closed_exception &cce) { // noop } catch (std::exception &e) { + PRINT_EXCEPTION(e); LOG(logLevelWarn, "an exception caught while in sendThread at %s:%d: %s", __FILE__, __LINE__, e.what()); From e97354db613e83fbe2c024bee0bf794851c61450 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 18:44:11 -0700 Subject: [PATCH 27/42] client context collapse channel createChannel() into searchResponse() --- src/remoteClient/clientContextImpl.cpp | 58 +++++++++++--------------- 1 file changed, 25 insertions(+), 33 deletions(-) diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index ac89519..1c9d392 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -3420,38 +3420,6 @@ public: reportChannelStateChange(); } - /** - * Create a channel, i.e. submit create channel request to the server. - * This method is called after search is complete. - * @param transport - */ - void createChannel(Transport::shared_pointer const & transport) - { - Lock guard(m_channelMutex); - - // do not allow duplicate creation to the same transport - if (!m_allowCreation) - return; - m_allowCreation = false; - - // check existing transport - if (m_transport.get() && m_transport.get() != transport.get()) - { - disconnectPendingIO(false); - - m_transport->release(getID()); - } - else if (m_transport.get() == transport.get()) - { - // request to sent create request to same transport, ignore - // this happens when server is slower (processing search requests) than client generating it - return; - } - - m_transport = transport; - m_transport->enqueueSendRequest(internal_from_this()); - } - virtual void timeout() { createChannelFailed(); } @@ -3696,7 +3664,31 @@ public: std::copy(guid.value, guid.value + 12, m_guid.value); // create channel - createChannel(transport); + { + Lock guard(m_channelMutex); + + // do not allow duplicate creation to the same transport + if (!m_allowCreation) + return; + m_allowCreation = false; + + // check existing transport + if (m_transport.get() && m_transport.get() != transport.get()) + { + disconnectPendingIO(false); + + m_transport->release(getID()); + } + else if (m_transport.get() == transport.get()) + { + // request to sent create request to same transport, ignore + // this happens when server is slower (processing search requests) than client generating it + return; + } + + m_transport = transport; + m_transport->enqueueSendRequest(internal_from_this()); + } } virtual void transportClosed() OVERRIDE FINAL { From 128d4b67f9b5229eeac9002d3118ec27fae67fe7 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 18:48:48 -0700 Subject: [PATCH 28/42] simplify InternalChannelImpl::destroy() Collapse channel destroy(bool) and destroyChannel() into destroy() --- src/remoteClient/clientContextImpl.cpp | 95 +++++++++---------------- src/remoteClient/pv/clientContextImpl.h | 1 - 2 files changed, 34 insertions(+), 62 deletions(-) diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index 1c9d392..71cccfa 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -3305,7 +3305,40 @@ private: virtual void destroy() OVERRIDE FINAL { - destroy(false); + { + Lock guard(m_channelMutex); + if (m_connectionState == DESTROYED) + return; + REFTRACE_DECREMENT(num_active); + + m_getfield.reset(); + + // stop searching... + shared_pointer thisChannelPointer = internal_from_this(); + m_context->getChannelSearchManager()->unregisterSearchInstance(thisChannelPointer); + + disconnectPendingIO(true); + + if (m_connectionState == CONNECTED) + { + disconnect(false, true); + } + else if (m_transport) + { + // unresponsive state, do not forget to release transport + m_transport->release(getID()); + m_transport.reset(); + } + + + setConnectionState(DESTROYED); + + // unregister + m_context->unregisterChannel(thisChannelPointer); + } + + // should be called without any lock hold + reportChannelStateChange(); } virtual string getRequesterName() OVERRIDE FINAL @@ -3482,66 +3515,6 @@ public: reportChannelStateChange(); } - /** - * @param force force destruction regardless of reference count (not used now) - */ - void destroy(bool force) { - { - Lock guard(m_channelMutex); - if (m_connectionState == DESTROYED) - return; - //throw std::runtime_error("Channel already destroyed."); - } - REFTRACE_DECREMENT(num_active); - - destroyChannel(force); - } - - - /** - * Actual destroy method, to be called CAJContext. - * @param force force destruction regardless of reference count - * @throws PVAException - * @throws std::runtime_error - * @throws IOException - */ - void destroyChannel(bool /*force*/) OVERRIDE FINAL { - { - Lock guard(m_channelMutex); - - if (m_connectionState == DESTROYED) - throw std::runtime_error("Channel already destroyed."); - - m_getfield.reset(); - - // stop searching... - shared_pointer thisChannelPointer = internal_from_this(); - m_context->getChannelSearchManager()->unregisterSearchInstance(thisChannelPointer); - - disconnectPendingIO(true); - - if (m_connectionState == CONNECTED) - { - disconnect(false, true); - } - else if (m_transport) - { - // unresponsive state, do not forget to release transport - m_transport->release(getID()); - m_transport.reset(); - } - - - setConnectionState(DESTROYED); - - // unregister - m_context->unregisterChannel(thisChannelPointer); - } - - // should be called without any lock hold - reportChannelStateChange(); - } - /** * Disconnected notification. * @param initiateSearch flag to indicate if searching (connect) procedure should be initiated diff --git a/src/remoteClient/pv/clientContextImpl.h b/src/remoteClient/pv/clientContextImpl.h index 0ee0884..a003d65 100644 --- a/src/remoteClient/pv/clientContextImpl.h +++ b/src/remoteClient/pv/clientContextImpl.h @@ -41,7 +41,6 @@ public: POINTER_DEFINITIONS(ClientChannelImpl); virtual pvAccessID getChannelID() = 0; - virtual void destroyChannel(bool force) = 0; virtual void connectionCompleted(pvAccessID sid/*, rights*/) = 0; virtual void createChannelFailed() = 0; virtual ClientContextImpl* getContext() = 0; From d4822f141c39e1a27423c2f97de169068096fd84 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 18:50:58 -0700 Subject: [PATCH 29/42] simplify InternalClientContextImpl::destroy() Collapse internalDestroy() into destroy() --- src/remoteClient/clientContextImpl.cpp | 65 ++++++++++++-------------- 1 file changed, 30 insertions(+), 35 deletions(-) diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index 71cccfa..d5741e5 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -4080,7 +4080,36 @@ public: m_contextState = CONTEXT_DESTROYED; } - internalDestroy(); + // + // cleanup + // + + // this will also close all PVA transports + destroyAllChannels(); + + // stop UDPs + for (BlockingUDPTransportVector::const_iterator iter = m_udpTransports.begin(); + iter != m_udpTransports.end(); iter++) + (*iter)->close(); + m_udpTransports.clear(); + + // stop UDPs + if (m_searchTransport) + m_searchTransport->close(); + + // wait for all transports to cleanly exit + int tries = 40; + epics::pvData::int32 transportCount; + while ((transportCount = m_transportRegistry.size()) && tries--) + epicsThreadSleep(0.025); + + { + Lock guard(m_beaconMapMutex); + m_beaconHandlers.clear(); + } + + if (transportCount) + LOG(logLevelDebug, "PVA client context destroyed with %u transport(s) active.", (unsigned)transportCount); } virtual ~InternalClientContextImpl() @@ -4159,40 +4188,6 @@ private: // TODO what if initialization failed!!! } - void internalDestroy() { - - // - // cleanup - // - - // this will also close all PVA transports - destroyAllChannels(); - - // stop UDPs - for (BlockingUDPTransportVector::const_iterator iter = m_udpTransports.begin(); - iter != m_udpTransports.end(); iter++) - (*iter)->close(); - m_udpTransports.clear(); - - // stop UDPs - if (m_searchTransport) - m_searchTransport->close(); - - // wait for all transports to cleanly exit - int tries = 40; - epics::pvData::int32 transportCount; - while ((transportCount = m_transportRegistry.size()) && tries--) - epicsThreadSleep(0.025); - - { - Lock guard(m_beaconMapMutex); - m_beaconHandlers.clear(); - } - - if (transportCount) - LOG(logLevelDebug, "PVA client context destroyed with %u transport(s) active.", (unsigned)transportCount); - } - void destroyAllChannels() { Lock guard(m_cidMapMutex); From c9c8fb369ac01855bae77d3f863d782b2e887c55 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 18:53:22 -0700 Subject: [PATCH 30/42] client context explicitly close() timer queue --- src/remoteClient/clientContextImpl.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index d5741e5..1f291dc 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -4084,6 +4084,8 @@ public: // cleanup // + m_timer->close(); + // this will also close all PVA transports destroyAllChannels(); From 1e0523b7b999452b6881e6b46d9f2f7a0bf1e13d Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 19:25:04 -0700 Subject: [PATCH 31/42] client context. Prevent transport dtor under channel mutex. m_channelMutex is locked recursively, so places like disconnect(bool,bool) which purport to unlock for the Transport detory actually don't as they are called with m_channelMutex already locked. Make a rather ugly hack of saving a ref to m_channel at the various entry point. --- src/remoteClient/clientContextImpl.cpp | 28 ++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index 1f291dc..53ca04f 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -3305,12 +3305,16 @@ private: virtual void destroy() OVERRIDE FINAL { + // Hack. Prevent Transport from being dtor'd while m_channelMutex is held + Transport::shared_pointer old_transport; { Lock guard(m_channelMutex); if (m_connectionState == DESTROYED) return; REFTRACE_DECREMENT(num_active); + old_transport = m_transport; + m_getfield.reset(); // stop searching... @@ -3441,7 +3445,11 @@ public: void disconnect() { { + // Hack. Prevent Transport from being dtor'd while m_channelMutex is held + Transport::shared_pointer old_transport; Lock guard(m_channelMutex); + old_transport = m_transport; + // if not destroyed... if (m_connectionState == DESTROYED) throw std::runtime_error("Channel destroyed."); @@ -3462,12 +3470,14 @@ public: */ virtual void createChannelFailed() OVERRIDE FINAL { + // Hack. Prevent Transport from being dtor'd while m_channelMutex is held + Transport::shared_pointer old_transport; Lock guard(m_channelMutex); // release transport if active if (m_transport) { m_transport->release(getID()); - m_transport.reset(); + old_transport.swap(m_transport); } // ... and search again, with penalty @@ -3607,9 +3617,12 @@ public: } virtual void searchResponse(const ServerGUID & guid, int8 minorRevision, osiSockAddr* serverAddress) OVERRIDE FINAL { + // Hack. Prevent Transport from being dtor'd while m_channelMutex is held + Transport::shared_pointer old_transport; + Lock guard(m_channelMutex); - Transport::shared_pointer transport = m_transport; - if (transport.get()) + Transport::shared_pointer transport(m_transport); + if (transport) { // GUID check case: same server listening on different NIF @@ -3626,7 +3639,7 @@ public: // NOTE: this creates a new or acquires an existing transport (implies increases usage count) transport = m_context->getTransport(internal_from_this(), serverAddress, minorRevision, m_priority); - if (!transport.get()) + if (!transport) { createChannelFailed(); return; @@ -3646,7 +3659,7 @@ public: m_allowCreation = false; // check existing transport - if (m_transport.get() && m_transport.get() != transport.get()) + if (m_transport && m_transport.get() != transport.get()) { disconnectPendingIO(false); @@ -3659,7 +3672,10 @@ public: return; } - m_transport = transport; + // rotate: transport -> m_transport -> old_transport -> + old_transport.swap(m_transport); + m_transport.swap(transport); + m_transport->enqueueSendRequest(internal_from_this()); } } From ff5d51b2218cd4472a3b583e03aa0a26b564f5b1 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 4 Apr 2018 20:55:57 -0700 Subject: [PATCH 32/42] server: shorter name for Timer thread --- src/server/serverContext.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/serverContext.cpp b/src/server/serverContext.cpp index 0812fcc..aa72bbe 100644 --- a/src/server/serverContext.cpp +++ b/src/server/serverContext.cpp @@ -39,7 +39,7 @@ ServerContextImpl::ServerContextImpl(): _broadcastPort(PVA_BROADCAST_PORT), _serverPort(PVA_SERVER_PORT), _receiveBufferSize(MAX_TCP_RECV), - _timer(new Timer("pvAccess-server timer", lowerPriority)), + _timer(new Timer("PVAS timers", lowerPriority)), _beaconEmitter(), _acceptor(), _transportRegistry(), From 0c6fe2c1c627f4f43c82bf1a3d5654ae38f6d290 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Sun, 8 Apr 2018 16:04:36 -0700 Subject: [PATCH 33/42] client.h: add validity test for all handles --- src/client/pva/client.h | 44 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/client/pva/client.h b/src/client/pva/client.h index 21f6b3a..e594ad3 100644 --- a/src/client/pva/client.h +++ b/src/client/pva/client.h @@ -64,6 +64,17 @@ struct epicsShareClass Operation //! Does not wait for remote confirmation. void cancel(); + bool valid() const { return !!impl; } + +#if __cplusplus>=201103L + explicit operator bool() const { return valid(); } +#else +private: + typedef bool (Operation::*bool_type)() const; +public: + operator bool_type() const { return valid() ? &Operation::valid : 0; } +#endif + protected: std::tr1::shared_ptr impl; }; @@ -133,6 +144,17 @@ struct epicsShareClass Monitor epics::pvData::BitSet changed, overrun; + bool valid() const { return !!impl; } + +#if __cplusplus>=201103L + explicit operator bool() const { return valid(); } +#else +private: + typedef bool (Monitor::*bool_type)() const; +public: + operator bool_type() const { return valid() ? &Monitor::valid : 0; } +#endif + private: std::tr1::shared_ptr impl; friend struct MonitorSync; @@ -247,6 +269,17 @@ public: //! Channel name or an empty string std::string name() const; + bool valid() const { return !!impl; } + +#if __cplusplus>=201103L + explicit operator bool() const { return valid(); } +#else +private: + typedef bool (ClientChannel::*bool_type)() const; +public: + operator bool_type() const { return valid() ? &ClientChannel::valid : 0; } +#endif + //! callback for get() and rpc() struct GetCallback { virtual ~GetCallback() {} @@ -450,6 +483,17 @@ public: //! Clear channel cache void disconnect(); + + bool valid() const { return !!impl; } + +#if __cplusplus>=201103L + explicit operator bool() const { return valid(); } +#else +private: + typedef bool (ClientProvider::*bool_type)() const; +public: + operator bool_type() const { return valid() ? &ClientProvider::valid : 0; } +#endif }; From 0d4046454162c8d54d359893761909226458b12b Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Tue, 10 Apr 2018 15:17:18 -0700 Subject: [PATCH 34/42] client.h: expose reset() can also be accomplished with assignment from empty. op = pvac::Operation() --- src/client/pva/client.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/client/pva/client.h b/src/client/pva/client.h index e594ad3..f4a40a7 100644 --- a/src/client/pva/client.h +++ b/src/client/pva/client.h @@ -75,6 +75,8 @@ public: operator bool_type() const { return valid() ? &Operation::valid : 0; } #endif + void reset() { impl.reset(); } + protected: std::tr1::shared_ptr impl; }; @@ -155,6 +157,8 @@ public: operator bool_type() const { return valid() ? &Monitor::valid : 0; } #endif + void reset() { impl.reset(); } + private: std::tr1::shared_ptr impl; friend struct MonitorSync; @@ -280,6 +284,8 @@ public: operator bool_type() const { return valid() ? &ClientChannel::valid : 0; } #endif + void reset() { impl.reset(); } + //! callback for get() and rpc() struct GetCallback { virtual ~GetCallback() {} @@ -494,6 +500,8 @@ private: public: operator bool_type() const { return valid() ? &ClientProvider::valid : 0; } #endif + + void reset() { impl.reset(); } }; From 61d43e498d417f52cf09f22f67fc18aa72b32f81 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Tue, 10 Apr 2018 15:44:18 -0700 Subject: [PATCH 35/42] client.h std::ostream printing --- src/client/client.cpp | 50 ++++++++++++++++++++++++++++++++++++ src/client/clientGet.cpp | 7 +++++ src/client/clientMonitor.cpp | 14 ++++++++++ src/client/clientRPC.cpp | 7 +++++ src/client/pva/client.h | 12 +++++++++ 5 files changed, 90 insertions(+) diff --git a/src/client/client.cpp b/src/client/client.cpp index e797f1d..75d95cb 100644 --- a/src/client/client.cpp +++ b/src/client/client.cpp @@ -3,6 +3,8 @@ * found in the file LICENSE that is included with the distribution */ +#include + #include #include #include @@ -197,6 +199,17 @@ void ClientChannel::removeConnectListener(ConnectCallback* cb) } } + +void ClientChannel::show(std::ostream& strm) const +{ + if(impl) { + strm<channel.get()).name()<<" : "; + impl->channel->printInfo(strm); + } else { + strm<<"NULL Channel"; + } +} + static void register_reftrack() { @@ -305,6 +318,43 @@ void ClientProvider::disconnect() impl->channels.clear(); } +::std::ostream& operator<<(::std::ostream& strm, const Operation& op) +{ + if(op.impl) { + op.impl->show(strm); + } else { + strm << "Operation()"; + } + return strm; +} + +::std::ostream& operator<<(::std::ostream& strm, const ClientChannel& op) +{ + if(op.impl) { + strm << "ClientChannel(" + << typeid(*op.impl->channel.get()).name()<<", " + "\"" << op.impl->channel->getChannelName() <<"\", " + "\"" << op.impl->channel->getProvider()->getProviderName() <<"\", " + "connected="<<(op.impl->channel->isConnected()?"true":"false") + <<"\")"; + } else { + strm << "ClientChannel()"; + } + return strm; +} + +::std::ostream& operator<<(::std::ostream& strm, const ClientProvider& op) +{ + if(op.impl) { + strm << "ClientProvider(" + << typeid(*op.impl->provider.get()).name()<<", " + "\""<provider->getProviderName()<<"\")"; + } else { + strm << "ClientProvider()"; + } + return strm; +} + namespace detail { void registerRefTrack() diff --git a/src/client/clientGet.cpp b/src/client/clientGet.cpp index 0790a02..54051fd 100644 --- a/src/client/clientGet.cpp +++ b/src/client/clientGet.cpp @@ -177,6 +177,13 @@ struct GetPutter : public pva::ChannelPutRequester, callEvent(G, status.isSuccess()? pvac::GetEvent::Success : pvac::GetEvent::Fail); } + + virtual void show(std::ostream &strm) const + { + strm << "Operation(Get/Put" + "\"" << name() <<"\"" + ")"; + } }; size_t GetPutter::num_instances; diff --git a/src/client/clientMonitor.cpp b/src/client/clientMonitor.cpp index 8b7debd..6e19abe 100644 --- a/src/client/clientMonitor.cpp +++ b/src/client/clientMonitor.cpp @@ -248,6 +248,20 @@ ClientChannel::monitor(MonitorCallback *cb, return Monitor(ret); } +::std::ostream& operator<<(::std::ostream& strm, const Monitor& op) +{ + if(op.impl) { + strm << "Monitor(" + "\"" << op.impl->chan->getChannelName() <<"\", " + "\"" << op.impl->chan->getProvider()->getProviderName() <<"\", " + "connected="<<(op.impl->chan->isConnected()?"true":"false") + <<"\")"; + } else { + strm << "Monitor()"; + } + return strm; +} + namespace detail { void registerRefTrackMonitor() diff --git a/src/client/clientRPC.cpp b/src/client/clientRPC.cpp index 9400961..5028586 100644 --- a/src/client/clientRPC.cpp +++ b/src/client/clientRPC.cpp @@ -140,6 +140,13 @@ struct RPCer : public pva::ChannelRPCRequester, callEvent(G, status.isSuccess()? pvac::GetEvent::Success : pvac::GetEvent::Fail); } + + virtual void show(std::ostream &strm) const + { + strm << "Operation(RPC" + "\"" << name() <<"\"" + ")"; + } }; size_t RPCer::num_instances; diff --git a/src/client/pva/client.h b/src/client/pva/client.h index f4a40a7..bc2d88d 100644 --- a/src/client/pva/client.h +++ b/src/client/pva/client.h @@ -5,6 +5,7 @@ #ifndef PVATESTCLIENT_H #define PVATESTCLIENT_H +#include #include #include @@ -53,6 +54,7 @@ struct epicsShareClass Operation virtual ~Impl() {} virtual std::string name() const =0; virtual void cancel() =0; + virtual void show(std::ostream&) const =0; }; Operation() {} @@ -78,6 +80,7 @@ public: void reset() { impl.reset(); } protected: + friend ::std::ostream& operator<<(::std::ostream& strm, const Operation& op); std::tr1::shared_ptr impl; }; @@ -161,6 +164,7 @@ public: private: std::tr1::shared_ptr impl; + friend ::std::ostream& operator<<(::std::ostream& strm, const Monitor& op); friend struct MonitorSync; }; @@ -246,6 +250,7 @@ private: std::tr1::shared_ptr impl; friend class ClientProvider; friend void detail::registerRefTrack(); + friend ::std::ostream& operator<<(::std::ostream& strm, const ClientChannel& op); ClientChannel(const std::tr1::shared_ptr& i) :impl(i) {} public: @@ -397,6 +402,7 @@ public: //! Remove from list of listeners void removeConnectListener(ConnectCallback*); + void show(std::ostream& strm) const; private: std::tr1::shared_ptr getChannel(); }; @@ -460,6 +466,7 @@ class epicsShareClass ClientProvider struct Impl; std::tr1::shared_ptr impl; friend void detail::registerRefTrack(); + friend ::std::ostream& operator<<(::std::ostream& strm, const ClientProvider& op); public: /** Use named provider. @@ -512,6 +519,11 @@ ClientChannel::put(const epics::pvData::PVStructure::const_shared_pointer& pvReq return detail::PutBuilder(*this, pvRequest); } +epicsShareExtern ::std::ostream& operator<<(::std::ostream& strm, const Operation& op); +epicsShareExtern ::std::ostream& operator<<(::std::ostream& strm, const Monitor& op); +epicsShareExtern ::std::ostream& operator<<(::std::ostream& strm, const ClientChannel& op); +epicsShareExtern ::std::ostream& operator<<(::std::ostream& strm, const ClientProvider& op); + //! @} }//namespace pvac From 3d85852c93f840f197ff18737770fbb2fb693fff Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 18 Apr 2018 09:01:11 -0700 Subject: [PATCH 36/42] attempt to fix dllimport issue --- src/client/pva/client.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/client/pva/client.h b/src/client/pva/client.h index bc2d88d..0c6b364 100644 --- a/src/client/pva/client.h +++ b/src/client/pva/client.h @@ -80,7 +80,7 @@ public: void reset() { impl.reset(); } protected: - friend ::std::ostream& operator<<(::std::ostream& strm, const Operation& op); + friend epicsShareFunc ::std::ostream& operator<<(::std::ostream& strm, const Operation& op); std::tr1::shared_ptr impl; }; @@ -164,7 +164,7 @@ public: private: std::tr1::shared_ptr impl; - friend ::std::ostream& operator<<(::std::ostream& strm, const Monitor& op); + friend epicsShareFunc ::std::ostream& operator<<(::std::ostream& strm, const Monitor& op); friend struct MonitorSync; }; @@ -250,7 +250,7 @@ private: std::tr1::shared_ptr impl; friend class ClientProvider; friend void detail::registerRefTrack(); - friend ::std::ostream& operator<<(::std::ostream& strm, const ClientChannel& op); + friend epicsShareFunc ::std::ostream& operator<<(::std::ostream& strm, const ClientChannel& op); ClientChannel(const std::tr1::shared_ptr& i) :impl(i) {} public: @@ -466,7 +466,7 @@ class epicsShareClass ClientProvider struct Impl; std::tr1::shared_ptr impl; friend void detail::registerRefTrack(); - friend ::std::ostream& operator<<(::std::ostream& strm, const ClientProvider& op); + friend epicsShareFunc ::std::ostream& operator<<(::std::ostream& strm, const ClientProvider& op); public: /** Use named provider. @@ -519,10 +519,10 @@ ClientChannel::put(const epics::pvData::PVStructure::const_shared_pointer& pvReq return detail::PutBuilder(*this, pvRequest); } -epicsShareExtern ::std::ostream& operator<<(::std::ostream& strm, const Operation& op); -epicsShareExtern ::std::ostream& operator<<(::std::ostream& strm, const Monitor& op); -epicsShareExtern ::std::ostream& operator<<(::std::ostream& strm, const ClientChannel& op); -epicsShareExtern ::std::ostream& operator<<(::std::ostream& strm, const ClientProvider& op); +epicsShareFunc ::std::ostream& operator<<(::std::ostream& strm, const Operation& op); +epicsShareFunc ::std::ostream& operator<<(::std::ostream& strm, const Monitor& op); +epicsShareFunc ::std::ostream& operator<<(::std::ostream& strm, const ClientChannel& op); +epicsShareFunc ::std::ostream& operator<<(::std::ostream& strm, const ClientProvider& op); //! @} From 7cf78d5b9b83f28888d870c3239668772fbd11dc Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 20 Apr 2018 15:22:57 -0700 Subject: [PATCH 37/42] client.h add ClientProvider::name() --- src/client/client.cpp | 6 ++++++ src/client/pva/client.h | 2 ++ 2 files changed, 8 insertions(+) diff --git a/src/client/client.cpp b/src/client/client.cpp index 75d95cb..0770f84 100644 --- a/src/client/client.cpp +++ b/src/client/client.cpp @@ -279,6 +279,12 @@ ClientProvider::ClientProvider(const std::tr1::shared_ptrprovider->getProviderName(); +} + ClientChannel ClientProvider::connect(const std::string& name, const ClientChannel::Options& conf) diff --git a/src/client/pva/client.h b/src/client/pva/client.h index 0c6b364..2da8e67 100644 --- a/src/client/pva/client.h +++ b/src/client/pva/client.h @@ -481,6 +481,8 @@ public: explicit ClientProvider(const std::tr1::shared_ptr& provider); ~ClientProvider(); + std::string name() const; + /** Get a new Channel * * Does not block. From a2b60771c21eff4999a982b83b7da0b03d0e3ccc Mon Sep 17 00:00:00 2001 From: "Andrew C. Starritt" Date: Tue, 24 Apr 2018 07:30:47 -0700 Subject: [PATCH 38/42] pvget -m show time/alarm MD: cleanups --- pvtoolsSrc/pvget.cpp | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/pvtoolsSrc/pvget.cpp b/pvtoolsSrc/pvget.cpp index 1757a8b..843955b 100644 --- a/pvtoolsSrc/pvget.cpp +++ b/pvtoolsSrc/pvget.cpp @@ -305,14 +305,7 @@ struct MonitorRequesterImpl : public MonitorRequester, public Tracker } else { - if (fieldSeparator == ' ' && value->getField()->getType() == scalar) - std::cout << std::setw(30) << std::left << m_channelName; - else - std::cout << m_channelName; - - std::cout << fieldSeparator; - - terse(std::cout, value) << '\n'; + printMonitoredValue (value, element); } } } @@ -344,6 +337,35 @@ struct MonitorRequesterImpl : public MonitorRequester, public Tracker std::cerr << "unlisten" << m_channelName << '\n'; done(); } + +private: + // For value type scalar or scalarArray when mode is ValueOnlyMode + void printMonitoredValue (PVField::shared_pointer value, MonitorElement* element) + { + PVStructure::shared_pointer timeStamp(element->pvStructurePtr->getSubField("timeStamp")); + PVStructure::shared_pointer alarm(element->pvStructurePtr->getSubField("alarm")); + + if (fieldSeparator == ' ' && value->getField()->getType() == scalar) + std::cout << std::setw(30) << std::left; + + std::cout << m_channelName << fieldSeparator; + + if (timeStamp) + terseStructure(std::cout, timeStamp) << " "; + + terse(std::cout, value) << " "; + + if (alarm) + { + PVScalar::shared_pointer pvSeverity(alarm->getSubField("severity")); + + bool inAlarm = !pvSeverity ? false : (pvSeverity->getAs()!=0); + if (inAlarm) + terseStructure(std::cout, alarm); + } + + std::cout<< '\n'; + } }; } // namespace @@ -565,8 +587,6 @@ int main (int argc, char *argv[]) std::cerr<<"Provider "<second; From 9c25057d80eacbc73e459585fb0d0f5b7e3f3f43 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Tue, 24 Apr 2018 08:29:45 -0700 Subject: [PATCH 39/42] pvtools: drop unnecessary accessors for global flags These don't provide any encapsulation or validation... --- pvtoolsSrc/eget.cpp | 6 +++--- pvtoolsSrc/pvget.cpp | 4 ++-- pvtoolsSrc/pvput.cpp | 1 - pvtoolsSrc/pvutils.cpp | 17 ----------------- pvtoolsSrc/pvutils.h | 5 ----- 5 files changed, 5 insertions(+), 28 deletions(-) diff --git a/pvtoolsSrc/eget.cpp b/pvtoolsSrc/eget.cpp index 7309ce2..1c6b940 100644 --- a/pvtoolsSrc/eget.cpp +++ b/pvtoolsSrc/eget.cpp @@ -1416,7 +1416,7 @@ int main (int argc, char *argv[]) dumpStructure = true; break; case 'i': /* T-types format mode */ - formatTTypes(false); + formatTTypesFlag = false; break; case 't': /* Terse mode */ mode = TerseMode; @@ -1478,7 +1478,7 @@ int main (int argc, char *argv[]) break; } case 'n': - setEnumPrintMode(NumberEnum); + enumMode = NumberEnum; break; case '?': fprintf(stderr, @@ -1526,7 +1526,7 @@ int main (int argc, char *argv[]) std::cout << std::boolalpha; terseSeparator(fieldSeparator); - terseArrayCount(false); + arrayCountFlag = false; bool allOK = true; diff --git a/pvtoolsSrc/pvget.cpp b/pvtoolsSrc/pvget.cpp index 843955b..641b39c 100644 --- a/pvtoolsSrc/pvget.cpp +++ b/pvtoolsSrc/pvget.cpp @@ -427,7 +427,7 @@ int main (int argc, char *argv[]) mode = TerseMode; break; case 'i': /* T-types format mode */ - formatTTypes(false); + formatTTypesFlag = false; break; case 'm': /* Monitor mode */ monitor = true; @@ -468,7 +468,7 @@ int main (int argc, char *argv[]) break; } case 'n': - setEnumPrintMode(NumberEnum); + enumMode = NumberEnum; break; case '?': fprintf(stderr, diff --git a/pvtoolsSrc/pvput.cpp b/pvtoolsSrc/pvput.cpp index db5ae9a..f2cdddb 100644 --- a/pvtoolsSrc/pvput.cpp +++ b/pvtoolsSrc/pvput.cpp @@ -610,7 +610,6 @@ int main (int argc, char *argv[]) std::cout << std::boolalpha; terseSeparator(fieldSeparator); - setEnumPrintMode(enumMode); epics::pvAccess::ca::CAClientFactory::start(); diff --git a/pvtoolsSrc/pvutils.cpp b/pvtoolsSrc/pvutils.cpp index b0eab26..3a6ccc9 100644 --- a/pvtoolsSrc/pvutils.cpp +++ b/pvtoolsSrc/pvutils.cpp @@ -50,28 +50,11 @@ void terseSeparator(char c) } char arrayCountFlag = true; -void terseArrayCount(bool flag) -{ - arrayCountFlag = flag; -} EnumMode enumMode = AutoEnum; -void setEnumPrintMode(EnumMode mode) -{ - enumMode = mode; -} bool formatTTypesFlag = true; -void formatTTypes(bool flag) -{ - formatTTypesFlag = flag; -} - bool printUserTagFlag = true; -void printUserTag(bool flag) -{ - printUserTagFlag = flag; -} std::ostream& terse(std::ostream& o, PVField::const_shared_pointer const & pv) { diff --git a/pvtoolsSrc/pvutils.h b/pvtoolsSrc/pvutils.h index da4139c..3f1c03c 100644 --- a/pvtoolsSrc/pvutils.h +++ b/pvtoolsSrc/pvutils.h @@ -8,7 +8,6 @@ void convertArray(std::string*, epics::pvData::PVScalarArray * pv, int notFirst) void convertStructureArray(std::string*, epics::pvData::PVStructureArray * pvdata, int notFirst); void terseSeparator(char c); -void terseArrayCount(bool flag); std::ostream& terse(std::ostream& o, epics::pvData::PVField::const_shared_pointer const & pv); std::ostream& terseUnion(std::ostream& o, epics::pvData::PVUnion::const_shared_pointer const & pvUnion); std::ostream& terseStructure(std::ostream& o, const epics::pvData::PVStructure::const_shared_pointer &pvStructure); @@ -17,14 +16,10 @@ std::ostream& terseStructureArray(std::ostream& o, epics::pvData::PVStructureArr std::ostream& terseUnionArray(std::ostream& o, epics::pvData::PVUnionArray::const_shared_pointer const & pvArray); enum EnumMode { AutoEnum, NumberEnum, StringEnum }; -void setEnumPrintMode(EnumMode mode); -void formatTTypes(bool flag); bool isTType(epics::pvData::PVStructure::const_shared_pointer const & pvStructure); bool formatTType(std::ostream& o, const epics::pvData::PVStructure::const_shared_pointer &pvStructure); -void printUserTag(bool flag); - std::ostream& printEnumT(std::ostream& o, epics::pvData::PVStructure const & pvEnumT); std::ostream& printEnumT(std::ostream& o, epics::pvData::PVStructure::const_shared_pointer const & pvEnumT); std::ostream& printTimeT(std::ostream& o, epics::pvData::PVStructure::const_shared_pointer const & pvTimeT); From 1adea89e77b74e41f69531f1fe2daa97d2f7cc00 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Tue, 24 Apr 2018 08:33:25 -0700 Subject: [PATCH 40/42] pvtools: combine separator and fieldSeparator both always have the same value anyway... --- pvtoolsSrc/eget.cpp | 43 ++++++++++++++++++++---------------------- pvtoolsSrc/pvget.cpp | 3 --- pvtoolsSrc/pvlist.cpp | 1 - pvtoolsSrc/pvput.cpp | 3 --- pvtoolsSrc/pvutils.cpp | 26 +++++++++++-------------- pvtoolsSrc/pvutils.h | 1 - 6 files changed, 31 insertions(+), 46 deletions(-) diff --git a/pvtoolsSrc/eget.cpp b/pvtoolsSrc/eget.cpp index 1c6b940..a936c9e 100644 --- a/pvtoolsSrc/eget.cpp +++ b/pvtoolsSrc/eget.cpp @@ -36,8 +36,6 @@ using namespace epics::pvAccess; enum PrintMode { ValueOnlyMode, StructureMode, TerseMode }; PrintMode mode = ValueOnlyMode; -char fieldSeparator = ' '; - bool columnMajor = false; bool transpose = false; @@ -242,7 +240,7 @@ void formatTable(std::ostream& o, { for (size_t i = 0; i < numColumns; i++) { - if (separator == ' ') + if (fieldSeparator == ' ') { int width = std::max(labels[i].size()+padding, maxColumnLength); o << std::setw(width) << std::right; @@ -250,7 +248,7 @@ void formatTable(std::ostream& o, } else if (i > 0) { - o << separator; + o << fieldSeparator; } o << labels[i]; @@ -263,7 +261,7 @@ void formatTable(std::ostream& o, { for (size_t i = 0; i < numColumns; i++) { - if (separator == ' ' && (showHeader || numColumns > 1)) + if (fieldSeparator == ' ' && (showHeader || numColumns > 1)) { int width = std::max(labels[i].size()+padding, maxColumnLength); o << setw(width) << std::right; @@ -271,7 +269,7 @@ void formatTable(std::ostream& o, } else if (i > 0) { - o << separator; + o << fieldSeparator; } PVScalarArrayPtr array = columnData[i]; @@ -297,7 +295,7 @@ void formatTable(std::ostream& o, { if (showHeader && labels.size()) { - if (separator == ' ') + if (fieldSeparator == ' ') { o << std::setw(maxLabelColumnLength) << std::left; } @@ -306,12 +304,12 @@ void formatTable(std::ostream& o, for (size_t r = 0; r < maxValues; r++) { - if (separator == ' ' && (showHeader || numColumns > 1)) + if (fieldSeparator == ' ' && (showHeader || numColumns > 1)) { o << std::setw(maxColumnLength) << std::right; } else if (showHeader || r > 0) - o << separator; + o << fieldSeparator; PVScalarArrayPtr array = columnData[i]; if (array.get() && r < array->getLength()) @@ -433,10 +431,10 @@ void formatNTMatrix(std::ostream& o, PVStructurePtr const & pvStruct) { for (int32 c = 0; c < cols; c++) { - if (separator == ' ' && cols > 1) + if (fieldSeparator == ' ' && cols > 1) o << std::setw(maxColumnLength) << std::right; else if (c > 0) - o << separator; + o << fieldSeparator; if (columnMajor) value->dumpValue(o, r + c * rows); @@ -459,10 +457,10 @@ void formatNTMatrix(std::ostream& o, PVStructurePtr const & pvStruct) { for (int32 r = 0; r < rows; r++) { - if (separator == ' ' && rows > 1) + if (fieldSeparator == ' ' && rows > 1) o << std::setw(maxColumnLength) << std::right; else if (r > 0) - o << separator; + o << fieldSeparator; if (columnMajor) value->dumpValue(o, ix++); @@ -540,7 +538,7 @@ void formatNTNameValue(std::ostream& o, PVStructurePtr const & pvStruct) { for (size_t i = 0; i < numColumns; i++) { - if (separator == ' ') + if (fieldSeparator == ' ') { int width = std::max(nameData[i].size()+padding, maxColumnLength); o << std::setw(width) << std::right; @@ -548,7 +546,7 @@ void formatNTNameValue(std::ostream& o, PVStructurePtr const & pvStruct) } else if (i > 0) { - o << separator; + o << fieldSeparator; } o << nameData[i]; @@ -559,7 +557,7 @@ void formatNTNameValue(std::ostream& o, PVStructurePtr const & pvStruct) // then values for (size_t i = 0; i < numColumns; i++) { - if (separator == ' ' && showHeader) + if (fieldSeparator == ' ' && showHeader) { int width = std::max(nameData[i].size()+padding, maxColumnLength); o << std::setw(width) << std::right; @@ -567,7 +565,7 @@ void formatNTNameValue(std::ostream& o, PVStructurePtr const & pvStruct) } else if (i > 0) { - o << separator; + o << fieldSeparator; } array->dumpValue(o, i); } @@ -586,20 +584,20 @@ void formatNTNameValue(std::ostream& o, PVStructurePtr const & pvStruct) { if (showHeader) { - if (separator == ' ') + if (fieldSeparator == ' ') { o << std::setw(maxLabelColumnLength) << std::left; } o << nameData[i]; } - if (separator == ' ' && showHeader) + if (fieldSeparator == ' ' && showHeader) { o << std::setw(maxColumnLength) << std::right; } else if (showHeader) { - o << separator; + o << fieldSeparator; } array->dumpValue(o, i); @@ -899,7 +897,7 @@ void printValue(std::string const & channelName, PVStructure::shared_pointer con if (forceTerseWithName) { if (!channelName.empty()) - std::cout << channelName << separator; + std::cout << channelName << fieldSeparator; terseStructure(std::cout, pv) << std::endl; } else if (mode == ValueOnlyMode) @@ -1044,7 +1042,7 @@ void usage (void) " -p : Set default provider name, default is '%s'\n" " -q: Quiet mode, print only error messages\n" " -d: Enable debug output\n" - " -F : Use as an alternate output field separator\n" + " -F : Use as an alternate output field fieldSeparator\n" " -f : Use as an input that provides a list PV name(s) to be read, use '-' for stdin\n" " -c: Wait for clean shutdown and report used instance count (for expert users)\n" " enum format:\n" @@ -1525,7 +1523,6 @@ int main (int argc, char *argv[]) SET_LOG_LEVEL(debug ? logLevelDebug : logLevelError); std::cout << std::boolalpha; - terseSeparator(fieldSeparator); arrayCountFlag = false; bool allOK = true; diff --git a/pvtoolsSrc/pvget.cpp b/pvtoolsSrc/pvget.cpp index 641b39c..c4ad4b5 100644 --- a/pvtoolsSrc/pvget.cpp +++ b/pvtoolsSrc/pvget.cpp @@ -48,8 +48,6 @@ string defaultProvider("pva"); enum PrintMode { ValueOnlyMode, StructureMode, TerseMode }; PrintMode mode = ValueOnlyMode; -char fieldSeparator = ' '; - void usage (void) { fprintf (stderr, "\nUsage: pvget [options] ...\n\n" @@ -531,7 +529,6 @@ int main (int argc, char *argv[]) SET_LOG_LEVEL(debugFlag ? logLevelDebug : logLevelError); std::cout << std::boolalpha; - terseSeparator(fieldSeparator); // ================ Connect channels and start operations diff --git a/pvtoolsSrc/pvlist.cpp b/pvtoolsSrc/pvlist.cpp index e9840c6..7423e50 100644 --- a/pvtoolsSrc/pvlist.cpp +++ b/pvtoolsSrc/pvlist.cpp @@ -490,7 +490,6 @@ int main (int argc, char *argv[]) int opt; /* getopt() current option */ bool debug = false; double timeOut = DEFAULT_TIMEOUT; - // char fieldSeparator = ' '; bool printInfo = false; /* diff --git a/pvtoolsSrc/pvput.cpp b/pvtoolsSrc/pvput.cpp index f2cdddb..98efae1 100644 --- a/pvtoolsSrc/pvput.cpp +++ b/pvtoolsSrc/pvput.cpp @@ -51,8 +51,6 @@ const string noAddress; enum PrintMode { ValueOnlyMode, StructureMode, TerseMode }; PrintMode mode = ValueOnlyMode; -char fieldSeparator = ' '; - bool debug = false; void usage (bool details=false) @@ -609,7 +607,6 @@ int main (int argc, char *argv[]) SET_LOG_LEVEL(debug ? logLevelDebug : logLevelError); std::cout << std::boolalpha; - terseSeparator(fieldSeparator); epics::pvAccess::ca::CAClientFactory::start(); diff --git a/pvtoolsSrc/pvutils.cpp b/pvtoolsSrc/pvutils.cpp index 3a6ccc9..28ba4cb 100644 --- a/pvtoolsSrc/pvutils.cpp +++ b/pvtoolsSrc/pvutils.cpp @@ -43,11 +43,7 @@ std::ostream& operator<<(std::ostream& o, const dump_stack_only_on_debug& d) return o; } -char separator = ' '; -void terseSeparator(char c) -{ - separator = c; -} +char fieldSeparator = ' '; char arrayCountFlag = true; @@ -132,7 +128,7 @@ std::ostream& printTimeT(std::ostream& o, epics::pvData::PVStructure::const_shar epicsTimeToStrftime(timeText, sizeof(timeText), "%Y-%m-%dT%H:%M:%S.%03f", &epicsTS); o << timeText; if (printUserTagFlag && tagf) - o << separator << tagf->getAs(); + o << fieldSeparator << tagf->getAs(); return o; } @@ -175,14 +171,14 @@ std::ostream& printAlarmT(std::ostream& o, epics::pvData::PVStructure::const_sha o << v; else o << severityNames[v]; - o << separator; + o << fieldSeparator; v = pvStatus->get(); if (v < 0 || v > 7) o << v; else o << statusNames[v]; - o << separator; + o << fieldSeparator; if (pvMessage->get().empty()) o << ""; else @@ -249,7 +245,7 @@ std::ostream& terseStructure(std::ostream& o, PVStructure::const_shared_pointer if (first) first = false; else - o << separator; + o << fieldSeparator; terse(o, fieldsData[i]); } @@ -277,7 +273,7 @@ std::ostream& terseScalarArray(std::ostream& o, const PVScalarArray::const_share o << '0'; return o; } - o << length << separator; + o << length << fieldSeparator; } bool first = true; @@ -285,7 +281,7 @@ std::ostream& terseScalarArray(std::ostream& o, const PVScalarArray::const_share if (first) first = false; else - o << separator; + o << fieldSeparator; pvArray->dumpValue(o, i); } @@ -308,7 +304,7 @@ std::ostream& terseStructureArray(std::ostream& o, PVStructureArray::const_share o << '0'; return o; } - o << length << separator; + o << length << fieldSeparator; } PVStructureArray::const_svector data = pvArray->view(); @@ -317,7 +313,7 @@ std::ostream& terseStructureArray(std::ostream& o, PVStructureArray::const_share if (first) first = false; else - o << separator; + o << fieldSeparator; terseStructure(o, data[i]); } @@ -334,7 +330,7 @@ std::ostream& terseUnionArray(std::ostream& o, PVUnionArray::const_shared_pointe o << '0'; return o; } - o << length << separator; + o << length << fieldSeparator; } PVUnionArray::const_svector data = pvArray->view(); @@ -343,7 +339,7 @@ std::ostream& terseUnionArray(std::ostream& o, PVUnionArray::const_shared_pointe if (first) first = false; else - o << separator; + o << fieldSeparator; terseUnion(o, data[i]); } diff --git a/pvtoolsSrc/pvutils.h b/pvtoolsSrc/pvutils.h index 3f1c03c..73439ac 100644 --- a/pvtoolsSrc/pvutils.h +++ b/pvtoolsSrc/pvutils.h @@ -7,7 +7,6 @@ void convertStructure(std::string* buffer, epics::pvData::PVStructure *data, int void convertArray(std::string*, epics::pvData::PVScalarArray * pv, int notFirst); void convertStructureArray(std::string*, epics::pvData::PVStructureArray * pvdata, int notFirst); -void terseSeparator(char c); std::ostream& terse(std::ostream& o, epics::pvData::PVField::const_shared_pointer const & pv); std::ostream& terseUnion(std::ostream& o, epics::pvData::PVUnion::const_shared_pointer const & pvUnion); std::ostream& terseStructure(std::ostream& o, const epics::pvData::PVStructure::const_shared_pointer &pvStructure); From a75720120caa94f4d84609c7713e3d64cb46163b Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Tue, 24 Apr 2018 12:33:54 -0700 Subject: [PATCH 41/42] client: call ChannelSearchManager::cancel() --- src/remoteClient/clientContextImpl.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index 53ca04f..8c4ea3b 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -4102,6 +4102,8 @@ public: m_timer->close(); + m_channelSearchManager->cancel(); + // this will also close all PVA transports destroyAllChannels(); From 51fdd506d04e036ff85707b615a87bec605e4279 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Tue, 24 Apr 2018 13:50:59 -0700 Subject: [PATCH 42/42] update release notes --- documentation/release_notes.h | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/documentation/release_notes.h b/documentation/release_notes.h index 0729638..487be13 100644 --- a/documentation/release_notes.h +++ b/documentation/release_notes.h @@ -1,8 +1,33 @@ /** @page pvarelease_notes Release Notes -Release 6.0.0 (UNRELEASED) +Release 6.1.0 (UNRELEASED) ========================== +- Deprecations + - pv/namedLockPattern.h +- Removals + - Remove deprecated methods configure(), flush(), and poll() from ChannelProvider. + - Remove RPCClient::sendRequest() + - Remove RPCService::destroy() and dispose() + - Typedefs GUID, Service +- Fixes + - pvAccessLog() add EPICS_PRINTF_STYLE() + - ioc: shutdown PVA server via epicsAtExit() + - fix 'pva' provider registration during static linking + - Various fixes related to shared_ptr loop breaking. + - Several *NULL bugs. + - PVA client context: avoid lock order violations +- Changes + - pvac::Monitor - shallow copy into Monitor::root + - pvget -m shows time and alarm if available (thanks to Andrew Starritt) +- Additions + - pvput to NTEnum via. string now supported + - pvac::* add valid() method and boolean cast shorthand. Also reset() and operator<<(ostream, ...) + - Add pvac::ClientProvider::named() + +Release 6.0.0 (Dec 2017) +======================== + - Incompatible changes - Requires pvDataCPP >=7.0.0 due to headers moved from pvDataCPP into this module: requester.h, destoryable.h, and monitor.h - Major changes to shared_ptr ownership rules for epics::pvAccess::ChannelProvider and