From bd88e35448413d05ab1cafeff0d38c9aad896b72 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 16 May 2018 09:14:49 -0700 Subject: [PATCH 1/7] client.h update doc --- src/client/pva/client.h | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/client/pva/client.h b/src/client/pva/client.h index 2da8e67..556e5ab 100644 --- a/src/client/pva/client.h +++ b/src/client/pva/client.h @@ -92,8 +92,7 @@ struct PutEvent Cancel, //!< request cancelled before completion Success, //!< It worked! } event; - std::string message; - void *priv; + std::string message; //!< Check when event==Fail }; //! Information on get/rpc completion @@ -177,15 +176,19 @@ struct MonitorEvent Disconnect=4,//!< subscription interrupted due to loss of communication Data=8, //!< Data queue not empty. Call Monitor::poll() } event; - std::string message; // set for event=Fail + std::string message; //!< set for event=Fail }; /** Subscription usable w/o callbacks * - * Basic usage is to call wait(). + * Basic usage is to call wait() or test(). * If true is returned, then the 'event', 'root', 'changed', and 'overrun' * members have been updated with a new event. * Test 'event.event' first to find out which kind of event has occured. + * + * Note that wait()/test() methods are distinct from base class poll(). + * wait()/test() check for the arrival of MonitorEvent + * while poll() checks for the availability of data (eg. following a Data event). */ struct epicsShareClass MonitorSync : public Monitor { @@ -195,15 +198,19 @@ struct epicsShareClass MonitorSync : public Monitor ~MonitorSync(); //! wait for new event + //! @returns true when a new event was received. + //! false if wake() was called. bool wait(); //! wait for new event //! @return false on timeout bool wait(double timeout); - //! check if new event is available + //! check if new event is immediately available. + //! Does not block. bool test(); - //! Abort one call to wait() - //! wait() will return with MonitorEvent::Fail + //! Abort one call to wait(), either concurrent or future. + //! Calls are queued. + //! wait() will return with MonitorEvent::Fail. void wake(); //! most recent event @@ -352,7 +359,6 @@ public: //! Initiate request to change PV //! @param cb Completion notification callback. Must outlive Operation (call Operation::cancel() to force release) - //! TODO: produce bitset to mask fields being set Operation put(PutCallback* cb, epics::pvData::PVStructure::const_shared_pointer pvRequest = epics::pvData::PVStructure::const_shared_pointer()); @@ -374,7 +380,7 @@ public: }; //! Begin subscription - //! @param cb Completion notification callback. Must outlive Operation (call Operation::cancel() to force release) + //! @param cb Completion notification callback. Must outlive Monitor (call Monitor::cancel() to force release) Monitor monitor(MonitorCallback *cb, epics::pvData::PVStructure::const_shared_pointer pvRequest = epics::pvData::PVStructure::const_shared_pointer()); From 6abfe9d196ee8d1db251faa216e24bcaaa133b42 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 18 May 2018 08:53:52 -0700 Subject: [PATCH 2/7] server: cleanup handling of vector of providers avoid keeping extra refs. situation is confusing enough as it is. --- src/server/pv/responseHandlers.h | 12 ++++-------- src/server/pv/serverContextImpl.h | 6 ++---- src/server/responseHandlers.cpp | 13 +++++++------ src/server/serverContext.cpp | 2 +- 4 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/server/pv/responseHandlers.h b/src/server/pv/responseHandlers.h index c391c28..d1c2b68 100644 --- a/src/server/pv/responseHandlers.h +++ b/src/server/pv/responseHandlers.h @@ -126,9 +126,6 @@ public: virtual void handleResponse(osiSockAddr* responseFrom, Transport::shared_pointer const & transport, epics::pvData::int8 version, epics::pvData::int8 command, std::size_t payloadSize, epics::pvData::ByteBuffer* payloadBuffer) OVERRIDE FINAL; - -private: - std::vector _providers; }; @@ -173,10 +170,9 @@ private: class ServerCreateChannelHandler : public AbstractServerResponseHandler { public: - ServerCreateChannelHandler(ServerContextImpl::shared_pointer const & context) : - AbstractServerResponseHandler(context, "Create channel request") { - _providers = context->getChannelProviders(); - } + ServerCreateChannelHandler(ServerContextImpl::shared_pointer const & context) + :AbstractServerResponseHandler(context, "Create channel request") + {} virtual ~ServerCreateChannelHandler() {} virtual void handleResponse(osiSockAddr* responseFrom, @@ -184,10 +180,10 @@ public: std::size_t payloadSize, epics::pvData::ByteBuffer* payloadBuffer) OVERRIDE FINAL; private: + // Name of the magic "server" PV used to implement channelList() and server info static std::string SERVER_CHANNEL_NAME; void disconnect(Transport::shared_pointer const & transport); - std::vector _providers; }; namespace detail { diff --git a/src/server/pv/serverContextImpl.h b/src/server/pv/serverContextImpl.h index b01349e..dae5dde 100644 --- a/src/server/pv/serverContextImpl.h +++ b/src/server/pv/serverContextImpl.h @@ -129,7 +129,7 @@ public: * Get channel providers. * @return channel providers. */ - virtual std::vector& getChannelProviders() OVERRIDE FINAL; + virtual const std::vector& getChannelProviders() OVERRIDE FINAL; /** * Return true if channel provider name is provided by configuration (e.g. system env. var.). @@ -224,9 +224,7 @@ private: */ ResponseHandler::shared_pointer _responseHandler; - /** - * Channel provider. - */ + // const after loadConfiguration() std::vector _channelProviders; /** diff --git a/src/server/responseHandlers.cpp b/src/server/responseHandlers.cpp index 3251179..61319be 100644 --- a/src/server/responseHandlers.cpp +++ b/src/server/responseHandlers.cpp @@ -229,7 +229,7 @@ void ServerEchoHandler::handleResponse(osiSockAddr* responseFrom, std::string ServerSearchHandler::SUPPORTED_PROTOCOL = "tcp"; ServerSearchHandler::ServerSearchHandler(ServerContextImpl::shared_pointer const & context) : - AbstractServerResponseHandler(context, "Search request"), _providers(context->getChannelProviders()) + AbstractServerResponseHandler(context, "Search request") { // initialize random seed with some random value srand ( time(NULL) ); @@ -332,15 +332,14 @@ void ServerSearchHandler::handleResponse(osiSockAddr* responseFrom, if (allowed) { - // TODO object pool!!! + const std::vector& _providers = _context->getChannelProviders(); + int providerCount = _providers.size(); std::tr1::shared_ptr tp(new ServerChannelFindRequesterImpl(_context, providerCount)); tp->set(name, searchSequenceId, cid, responseAddress, responseRequired, false); - // TODO use std::make_shared - ChannelFindRequester::shared_pointer spr = tp; for (int i = 0; i < providerCount; i++) - _providers[i]->channelFind(name, spr); + _providers[i]->channelFind(name, tp); } } } @@ -575,7 +574,7 @@ public: PVStringArray::shared_pointer allChannelNames = result->getSubFieldT("value"); ChannelListRequesterImpl::shared_pointer listListener(new ChannelListRequesterImpl()); - std::vector& providers = m_serverContext->getChannelProviders(); + const std::vector& providers = m_serverContext->getChannelProviders(); size_t providerCount = providers.size(); for (size_t i = 0; i < providerCount; i++) @@ -763,6 +762,8 @@ void ServerCreateChannelHandler::handleResponse(osiSockAddr* responseFrom, } else { + const std::vector& _providers(_context->getChannelProviders()); + if (_providers.size() == 1) ServerChannelRequesterImpl::create(_providers[0], transport, channelName, cid, css); else diff --git a/src/server/serverContext.cpp b/src/server/serverContext.cpp index aa72bbe..70135e2 100644 --- a/src/server/serverContext.cpp +++ b/src/server/serverContext.cpp @@ -502,7 +502,7 @@ BlockingUDPTransport::shared_pointer ServerContextImpl::getBroadcastTransport() return _broadcastTransport; } -std::vector& ServerContextImpl::getChannelProviders() +const std::vector& ServerContextImpl::getChannelProviders() { return _channelProviders; } From 6ccca1ce58917a9c9b768c07bdb62f0466c116bd Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 18 May 2018 10:12:34 -0700 Subject: [PATCH 3/7] const-ify string constants yes really... --- pvtoolsSrc/eget.cpp | 2 -- pvtoolsSrc/pvlist.cpp | 4 +--- src/ca/caProvider.cpp | 3 +-- src/pipelineService/pipelineServer.cpp | 8 ++++---- src/remoteClient/clientContextImpl.cpp | 3 +-- src/rpcService/rpcServer.cpp | 8 ++++---- src/server/pv/responseHandlers.h | 4 ++-- src/server/responseHandlers.cpp | 8 ++++---- testApp/remote/channelAccessIFTest.cpp | 12 ++++++------ testApp/remote/channelAccessIFTest.h | 12 ++++++------ testApp/remote/testServer.cpp | 4 ++-- 11 files changed, 31 insertions(+), 37 deletions(-) diff --git a/pvtoolsSrc/eget.cpp b/pvtoolsSrc/eget.cpp index a936c9e..3e792a6 100644 --- a/pvtoolsSrc/eget.cpp +++ b/pvtoolsSrc/eget.cpp @@ -937,8 +937,6 @@ void printValue(std::string const & channelName, PVStructure::shared_pointer con dumpValue(channelName, pv); } -static string emptyString; - // only in ValueOnlyMode // NOTE: names might be empty void printValues(shared_vector const & names, vector const & values) diff --git a/pvtoolsSrc/pvlist.cpp b/pvtoolsSrc/pvlist.cpp index 7423e50..53ecd63 100644 --- a/pvtoolsSrc/pvlist.cpp +++ b/pvtoolsSrc/pvlist.cpp @@ -67,8 +67,6 @@ string toHex(int8* ba, size_t len) { } -static string emptyString; - std::size_t readSize(ByteBuffer* buffer) { int8 b = buffer->getByte(); if(b==-1) @@ -94,7 +92,7 @@ string deserializeString(ByteBuffer* buffer) { return str; } else - return emptyString; + return std::string(); } struct ServerEntry { diff --git a/src/ca/caProvider.cpp b/src/ca/caProvider.cpp index 6ec6fb9..2c459f9 100644 --- a/src/ca/caProvider.cpp +++ b/src/ca/caProvider.cpp @@ -119,9 +119,8 @@ Channel::shared_pointer CAChannelProvider::createChannel( ChannelRequester::shared_pointer const & channelRequester, short priority) { - static std::string emptyString; Channel::shared_pointer channel( - createChannel(channelName, channelRequester, priority, emptyString)); + createChannel(channelName, channelRequester, priority, std::string())); return channel; } diff --git a/src/pipelineService/pipelineServer.cpp b/src/pipelineService/pipelineServer.cpp index 731b34b..053ba62 100644 --- a/src/pipelineService/pipelineServer.cpp +++ b/src/pipelineService/pipelineServer.cpp @@ -454,9 +454,9 @@ class PipelineChannelProvider : public: POINTER_DEFINITIONS(PipelineChannelProvider); - static string PROVIDER_NAME; + static const string PROVIDER_NAME; - static Status noSuchChannelStatus; + static const Status noSuchChannelStatus; // TODO thread pool support @@ -619,8 +619,8 @@ private: epics::pvData::Mutex m_mutex; }; -string PipelineChannelProvider::PROVIDER_NAME("PipelineService"); -Status PipelineChannelProvider::noSuchChannelStatus(Status::STATUSTYPE_ERROR, "no such channel"); +const string PipelineChannelProvider::PROVIDER_NAME("PipelineService"); +const Status PipelineChannelProvider::noSuchChannelStatus(Status::STATUSTYPE_ERROR, "no such channel"); PipelineServer::PipelineServer() :m_channelProviderImpl(new PipelineChannelProvider) diff --git a/src/remoteClient/clientContextImpl.cpp b/src/remoteClient/clientContextImpl.cpp index 8c4ea3b..120603b 100644 --- a/src/remoteClient/clientContextImpl.cpp +++ b/src/remoteClient/clientContextImpl.cpp @@ -54,7 +54,6 @@ Status ClientChannelImpl::channelDestroyed( Status::STATUSTYPE_WARNING, "channel destroyed"); Status ClientChannelImpl::channelDisconnected( Status::STATUSTYPE_WARNING, "channel disconnected"); -string emptyString; }} namespace { @@ -3096,7 +3095,7 @@ public: ChannelRequester::shared_pointer const & channelRequester, short priority) OVERRIDE FINAL { - return createChannel(channelName, channelRequester, priority, emptyString); + return createChannel(channelName, channelRequester, priority, std::string()); } virtual Channel::shared_pointer createChannel( diff --git a/src/rpcService/rpcServer.cpp b/src/rpcService/rpcServer.cpp index 37fa5f3..8b8c4a8 100644 --- a/src/rpcService/rpcServer.cpp +++ b/src/rpcService/rpcServer.cpp @@ -248,9 +248,9 @@ class RPCChannelProvider : public: POINTER_DEFINITIONS(RPCChannelProvider); - static string PROVIDER_NAME; + static const string PROVIDER_NAME; - static Status noSuchChannelStatus; + static const Status noSuchChannelStatus; // TODO thread pool support @@ -413,8 +413,8 @@ private: epics::pvData::Mutex m_mutex; }; -string RPCChannelProvider::PROVIDER_NAME("rpcService"); -Status RPCChannelProvider::noSuchChannelStatus(Status::STATUSTYPE_ERROR, "no such channel"); +const string RPCChannelProvider::PROVIDER_NAME("rpcService"); +const Status RPCChannelProvider::noSuchChannelStatus(Status::STATUSTYPE_ERROR, "no such channel"); RPCServer::RPCServer(const Configuration::const_shared_pointer &conf) diff --git a/src/server/pv/responseHandlers.h b/src/server/pv/responseHandlers.h index d1c2b68..c7cb4ed 100644 --- a/src/server/pv/responseHandlers.h +++ b/src/server/pv/responseHandlers.h @@ -118,7 +118,7 @@ public: // TODO static std::map > s_channelNameToProvider; - static std::string SUPPORTED_PROTOCOL; + static const std::string SUPPORTED_PROTOCOL; ServerSearchHandler(ServerContextImpl::shared_pointer const & context); virtual ~ServerSearchHandler() {} @@ -181,7 +181,7 @@ public: private: // Name of the magic "server" PV used to implement channelList() and server info - static std::string SERVER_CHANNEL_NAME; + static const std::string SERVER_CHANNEL_NAME; void disconnect(Transport::shared_pointer const & transport); }; diff --git a/src/server/responseHandlers.cpp b/src/server/responseHandlers.cpp index 61319be..c2eb0be 100644 --- a/src/server/responseHandlers.cpp +++ b/src/server/responseHandlers.cpp @@ -226,7 +226,7 @@ void ServerEchoHandler::handleResponse(osiSockAddr* responseFrom, /****************************************************************************************/ -std::string ServerSearchHandler::SUPPORTED_PROTOCOL = "tcp"; +const std::string ServerSearchHandler::SUPPORTED_PROTOCOL = "tcp"; ServerSearchHandler::ServerSearchHandler(ServerContextImpl::shared_pointer const & context) : AbstractServerResponseHandler(context, "Search request") @@ -527,7 +527,7 @@ private: static Structure::const_shared_pointer channelListStructure; static Structure::const_shared_pointer infoStructure; - static std::string helpString; + static const std::string helpString; ServerContextImpl::shared_pointer m_serverContext; @@ -680,7 +680,7 @@ Structure::const_shared_pointer ServerRPCService::infoStructure = createStructure(); -std::string ServerRPCService::helpString = +const std::string ServerRPCService::helpString = "pvAccess server RPC service.\n" "arguments:\n" "\tstring op\toperation to execute\n" @@ -695,7 +695,7 @@ std::string ServerRPCService::helpString = namespace epics { namespace pvAccess { -std::string ServerCreateChannelHandler::SERVER_CHANNEL_NAME = "server"; +const std::string ServerCreateChannelHandler::SERVER_CHANNEL_NAME = "server"; void ServerCreateChannelHandler::handleResponse(osiSockAddr* responseFrom, Transport::shared_pointer const & transport, int8 version, int8 command, diff --git a/testApp/remote/channelAccessIFTest.cpp b/testApp/remote/channelAccessIFTest.cpp index 3f09a30..8429c7a 100644 --- a/testApp/remote/channelAccessIFTest.cpp +++ b/testApp/remote/channelAccessIFTest.cpp @@ -32,17 +32,17 @@ namespace TR1 = std::tr1; // int value, 1Hz increment by one -std::string ChannelAccessIFTest::TEST_COUNTER_CHANNEL_NAME = "testCounter"; +const std::string ChannelAccessIFTest::TEST_COUNTER_CHANNEL_NAME = "testCounter"; // int value, increment on process -std::string ChannelAccessIFTest::TEST_SIMPLECOUNTER_CHANNEL_NAME = "testSimpleCounter"; +const std::string ChannelAccessIFTest::TEST_SIMPLECOUNTER_CHANNEL_NAME = "testSimpleCounter"; // double value, NTScalar -std::string ChannelAccessIFTest::TEST_CHANNEL_NAME = "testValue"; +const std::string ChannelAccessIFTest::TEST_CHANNEL_NAME = "testValue"; // double value -std::string ChannelAccessIFTest::TEST_VALUEONLY_CHANNEL_NAME = "testValueOnly"; +const std::string ChannelAccessIFTest::TEST_VALUEONLY_CHANNEL_NAME = "testValueOnly"; // RPC sum service: int a + int b -> int c -std::string ChannelAccessIFTest::TEST_SUMRPC_CHANNEL_NAME = "testSum"; +const std::string ChannelAccessIFTest::TEST_SUMRPC_CHANNEL_NAME = "testSum"; // double[] value -std::string ChannelAccessIFTest::TEST_ARRAY_CHANNEL_NAME = "testArray1"; +const std::string ChannelAccessIFTest::TEST_ARRAY_CHANNEL_NAME = "testArray1"; #ifdef ENABLE_STRESS_TESTS #define EXTRA_STRESS_TESTS 5 diff --git a/testApp/remote/channelAccessIFTest.h b/testApp/remote/channelAccessIFTest.h index 5c562dd..4b31222 100644 --- a/testApp/remote/channelAccessIFTest.h +++ b/testApp/remote/channelAccessIFTest.h @@ -16,12 +16,12 @@ public: protected: - static std::string TEST_COUNTER_CHANNEL_NAME; - static std::string TEST_SIMPLECOUNTER_CHANNEL_NAME; - static std::string TEST_CHANNEL_NAME; - static std::string TEST_VALUEONLY_CHANNEL_NAME; - static std::string TEST_SUMRPC_CHANNEL_NAME; - static std::string TEST_ARRAY_CHANNEL_NAME; + static const std::string TEST_COUNTER_CHANNEL_NAME; + static const std::string TEST_SIMPLECOUNTER_CHANNEL_NAME; + static const std::string TEST_CHANNEL_NAME; + static const std::string TEST_VALUEONLY_CHANNEL_NAME; + static const std::string TEST_SUMRPC_CHANNEL_NAME; + static const std::string TEST_ARRAY_CHANNEL_NAME; ChannelProvider::shared_pointer getChannelProvider() { return m_provider; } diff --git a/testApp/remote/testServer.cpp b/testApp/remote/testServer.cpp index 7aceadf..c1c84a0 100644 --- a/testApp/remote/testServer.cpp +++ b/testApp/remote/testServer.cpp @@ -2515,7 +2515,7 @@ public: typedef std::tr1::shared_ptr shared_pointer; typedef std::tr1::shared_ptr const_shared_pointer; - static string PROVIDER_NAME; + static const string PROVIDER_NAME; MockServerChannelProvider() : m_mockChannelFind(), @@ -2699,7 +2699,7 @@ private: epics::auto_ptr m_imgThread; }; -string MockServerChannelProvider::PROVIDER_NAME = "local"; +const string MockServerChannelProvider::PROVIDER_NAME = "local"; struct TestServer { From ce19199edd616007cf2bfecf2c3ed8a9d26d3d65 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 18 May 2018 11:58:35 -0700 Subject: [PATCH 4/7] cleanup --- src/server/pv/serverContextImpl.h | 30 +++--------------------------- src/server/serverContext.cpp | 2 +- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/src/server/pv/serverContextImpl.h b/src/server/pv/serverContextImpl.h index dae5dde..32f626d 100644 --- a/src/server/pv/serverContextImpl.h +++ b/src/server/pv/serverContextImpl.h @@ -123,7 +123,7 @@ public: * Broadcast (UDP send) transport. * @return broadcast transport. */ - BlockingUDPTransport::shared_pointer getBroadcastTransport(); + const BlockingUDPTransport::shared_pointer& getBroadcastTransport(); /** * Get channel providers. @@ -188,9 +188,6 @@ private: */ epics::pvData::int32 _receiveBufferSize; - /** - * Timer. - */ epics::pvData::Timer::shared_pointer _timer; /** @@ -198,14 +195,11 @@ private: */ BlockingUDPTransportVector _udpTransports; - /** - * UDP socket used to sending. + /** UDP socket used to sending. + * constant after ServerContextImpl::initialize() */ BlockingUDPTransport::shared_pointer _broadcastTransport; - /** - * Beacon emitter. - */ BeaconEmitter::shared_pointer _beaconEmitter; /** @@ -219,22 +213,13 @@ private: */ TransportRegistry _transportRegistry; - /** - * Response handler. - */ ResponseHandler::shared_pointer _responseHandler; // const after loadConfiguration() std::vector _channelProviders; - /** - * Run mutex. - */ epics::pvData::Mutex _mutex; - /** - * Run event. - */ epics::pvData::Event _runEvent; /** @@ -242,19 +227,10 @@ private: */ BeaconServerStatusProvider::shared_pointer _beaconServerStatusProvider; - /** - * Generate ServerGUID. - */ void generateGUID(); - /** - * Initialize logger. - */ void initializeLogger(); - /** - * Load configuration. - */ void loadConfiguration(); Configuration::const_shared_pointer configuration; diff --git a/src/server/serverContext.cpp b/src/server/serverContext.cpp index 70135e2..5f9ceeb 100644 --- a/src/server/serverContext.cpp +++ b/src/server/serverContext.cpp @@ -497,7 +497,7 @@ const osiSockAddr* ServerContextImpl::getServerInetAddress() return NULL; } -BlockingUDPTransport::shared_pointer ServerContextImpl::getBroadcastTransport() +const BlockingUDPTransport::shared_pointer& ServerContextImpl::getBroadcastTransport() { return _broadcastTransport; } From ba41fa6d627e040e4ab0e3d78ba964cc32e2f038 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 18 May 2018 15:17:01 -0700 Subject: [PATCH 5/7] notes --- src/remote/channelSearchManager.cpp | 2 ++ src/server/responseHandlers.cpp | 1 + 2 files changed, 3 insertions(+) diff --git a/src/remote/channelSearchManager.cpp b/src/remote/channelSearchManager.cpp index 0fe9da1..c69b5f8 100644 --- a/src/remote/channelSearchManager.cpp +++ b/src/remote/channelSearchManager.cpp @@ -50,6 +50,8 @@ public: namespace epics { namespace pvAccess { +// these are byte offset in a CMD_SEARCH request message +// used to mangle a buffer to support incremental construction. (ick!!!) 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; diff --git a/src/server/responseHandlers.cpp b/src/server/responseHandlers.cpp index c2eb0be..1b97405 100644 --- a/src/server/responseHandlers.cpp +++ b/src/server/responseHandlers.cpp @@ -283,6 +283,7 @@ void ServerSearchHandler::handleResponse(osiSockAddr* responseFrom, const int32 count = payloadBuffer->getShort() & 0xFFFF; // TODO DoS attack? + // You bet! With a reply address encoded in the request we don't even need a forged UDP header. const bool responseRequired = (QOS_REPLY_REQUIRED & qosCode) != 0; // From 7a523dd948a67009634826066514ce77236ca6b1 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 18 May 2018 15:20:26 -0700 Subject: [PATCH 6/7] drop unused NOMINMAX --- pvtoolsSrc/eget.cpp | 3 --- src/remote/codec.cpp | 5 ----- src/server/beaconEmitter.cpp | 4 ---- src/server/responseHandlers.cpp | 4 ---- testApp/remote/testCodec.cpp | 5 ----- testApp/remote/testServer.cpp | 4 ---- 6 files changed, 25 deletions(-) diff --git a/pvtoolsSrc/eget.cpp b/pvtoolsSrc/eget.cpp index 3e792a6..589ef06 100644 --- a/pvtoolsSrc/eget.cpp +++ b/pvtoolsSrc/eget.cpp @@ -1,6 +1,3 @@ -#if defined(_WIN32) && !defined(NOMINMAX) -#define NOMINMAX -#endif #include #include diff --git a/src/remote/codec.cpp b/src/remote/codec.cpp index 8efa03c..7859970 100644 --- a/src/remote/codec.cpp +++ b/src/remote/codec.cpp @@ -4,11 +4,6 @@ * in file LICENSE that is included with this distribution. */ -#if defined(_WIN32) && !defined(NOMINMAX) -#define NOMINMAX -#endif - - #include #include #include diff --git a/src/server/beaconEmitter.cpp b/src/server/beaconEmitter.cpp index 9001085..5341f3c 100644 --- a/src/server/beaconEmitter.cpp +++ b/src/server/beaconEmitter.cpp @@ -4,10 +4,6 @@ * in file LICENSE that is included with this distribution. */ -#if defined(_WIN32) && !defined(NOMINMAX) -#define NOMINMAX -#endif - #include #define epicsExportSharedSymbols diff --git a/src/server/responseHandlers.cpp b/src/server/responseHandlers.cpp index 1b97405..e529b9f 100644 --- a/src/server/responseHandlers.cpp +++ b/src/server/responseHandlers.cpp @@ -4,10 +4,6 @@ * in file LICENSE that is included with this distribution. */ -#if defined(_WIN32) && !defined(NOMINMAX) -#define NOMINMAX -#endif - #include #include #include diff --git a/testApp/remote/testCodec.cpp b/testApp/remote/testCodec.cpp index 3d1be6b..0ad7959 100644 --- a/testApp/remote/testCodec.cpp +++ b/testApp/remote/testCodec.cpp @@ -2,11 +2,6 @@ * testCodec.cpp */ -#if defined(_WIN32) && !defined(NOMINMAX) -#define NOMINMAX -#endif - - #include #include #include diff --git a/testApp/remote/testServer.cpp b/testApp/remote/testServer.cpp index c1c84a0..8c12916 100644 --- a/testApp/remote/testServer.cpp +++ b/testApp/remote/testServer.cpp @@ -2,10 +2,6 @@ * testServer.cpp */ -#if defined(_WIN32) && !defined(NOMINMAX) -#define NOMINMAX -#endif - // disable buggy boost enable_shared_from_this assert code #define BOOST_DISABLE_ASSERTS From 12e7b7864bad400f5c677052a8c946fd20080739 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 18 May 2018 17:04:07 -0700 Subject: [PATCH 7/7] fix s_channelNameToProvider hack Move from global to ServerContext, apply lock, and properly test if no provider has claimed, eg an attempt at unadvertised PV (still wouldn't work, but at least wouldn't crash). --- src/server/pv/responseHandlers.h | 3 --- src/server/pv/serverContextImpl.h | 5 +++++ src/server/responseHandlers.cpp | 18 +++++++++++++----- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/server/pv/responseHandlers.h b/src/server/pv/responseHandlers.h index c7cb4ed..a302cc7 100644 --- a/src/server/pv/responseHandlers.h +++ b/src/server/pv/responseHandlers.h @@ -115,9 +115,6 @@ private: class ServerSearchHandler : public AbstractServerResponseHandler { public: - // TODO - static std::map > s_channelNameToProvider; - static const std::string SUPPORTED_PROTOCOL; ServerSearchHandler(ServerContextImpl::shared_pointer const & context); diff --git a/src/server/pv/serverContextImpl.h b/src/server/pv/serverContextImpl.h index 32f626d..3bef569 100644 --- a/src/server/pv/serverContextImpl.h +++ b/src/server/pv/serverContextImpl.h @@ -137,6 +137,9 @@ public: */ bool isChannelProviderNamePreconfigured(); + // used by ServerChannelFindRequesterImpl + typedef std::map > s_channelNameToProvider_t; + s_channelNameToProvider_t s_channelNameToProvider; private: /** @@ -218,7 +221,9 @@ private: // const after loadConfiguration() std::vector _channelProviders; +public: epics::pvData::Mutex _mutex; +private: epics::pvData::Event _runEvent; diff --git a/src/server/responseHandlers.cpp b/src/server/responseHandlers.cpp index e529b9f..b6adda1 100644 --- a/src/server/responseHandlers.cpp +++ b/src/server/responseHandlers.cpp @@ -400,8 +400,6 @@ ServerChannelFindRequesterImpl* ServerChannelFindRequesterImpl::set(std::string return this; } -std::map > ServerSearchHandler::s_channelNameToProvider; - void ServerChannelFindRequesterImpl::channelFindResult(const Status& /*status*/, ChannelFind::shared_pointer const & channelFind, bool wasFound) { // TODO status @@ -427,7 +425,8 @@ void ServerChannelFindRequesterImpl::channelFindResult(const Status& /*status*/, { if (wasFound && _expectedResponseCount > 1) { - ServerSearchHandler::s_channelNameToProvider[_name] = channelFind->getChannelProvider(); + Lock L(_context->_mutex); + _context->s_channelNameToProvider[_name] = channelFind->getChannelProvider(); } _wasFound = wasFound; @@ -760,11 +759,20 @@ void ServerCreateChannelHandler::handleResponse(osiSockAddr* responseFrom, else { const std::vector& _providers(_context->getChannelProviders()); + ServerContextImpl::s_channelNameToProvider_t::const_iterator it; if (_providers.size() == 1) ServerChannelRequesterImpl::create(_providers[0], transport, channelName, cid, css); - else - ServerChannelRequesterImpl::create(ServerSearchHandler::s_channelNameToProvider[channelName].lock(), transport, channelName, cid, css); // TODO !!!! + else { + ChannelProvider::shared_pointer prov; + { + Lock L(_context->_mutex); + if((it = _context->s_channelNameToProvider.find(channelName)) != _context->s_channelNameToProvider.end()) + prov = it->second.lock(); + } + if(prov) + ServerChannelRequesterImpl::create(prov, transport, channelName, cid, css); + } } }