From d3ce104c336f8dd9f84734d3159ae86e699251fb Mon Sep 17 00:00:00 2001 From: mrkraimer Date: Sat, 23 Dec 2017 06:35:23 -0500 Subject: [PATCH 1/5] fix issue #77 --- src/ca/caChannel.cpp | 47 ++++++++++++++++++++++++++---------- src/ca/caChannel.h | 7 +++--- src/ca/caProvider.cpp | 54 +++++++++++++++++++++++++++++++++++------- src/ca/caProviderPvt.h | 9 +++++++ 4 files changed, 94 insertions(+), 23 deletions(-) diff --git a/src/ca/caChannel.cpp b/src/ca/caChannel.cpp index a1e3c90..1c1d0e9 100644 --- a/src/ca/caChannel.cpp +++ b/src/ca/caChannel.cpp @@ -374,7 +374,8 @@ CAChannel::CAChannel(std::string const & _channelName, channelRequester(_channelRequester), channelID(0), channelType(0), - elementCount(0) + elementCount(0), + channelCreated(false) { if(DEBUG_LEVEL>0) { cout<< "CAChannel::CAChannel " << channelName << endl; @@ -395,6 +396,9 @@ void CAChannel::activate(short priority) &channelID); if (result == ECA_NORMAL) { + channelCreated = true; + CAChannelProviderPtr provider(channelProvider.lock()); + if(provider) provider->addChannel(shared_from_this()); req->channelCreated(Status::Ok, shared_from_this()); } else { Status errorStatus(Status::STATUSTYPE_ERROR, string(ca_message(result))); @@ -402,6 +406,36 @@ void CAChannel::activate(short priority) } } +CAChannel::~CAChannel() +{ + if(DEBUG_LEVEL>0) { + cout << "CAChannel::~CAChannel() " << channelName << endl; + } + disconnectChannel(); +} + +void CAChannel::disconnectChannel() +{ + if(DEBUG_LEVEL>0) { + cout << "CAChannel::disconnectChannel() " + << channelName + << " channelCreated " << (channelCreated ? "true" : "false") + << endl; + } + bool disconnect = true; + { + Lock lock(requestsMutex); + if(!channelCreated) disconnect = false; + channelCreated = false; + } + if(!disconnect) return; + /* Clear CA Channel */ + threadAttach(); + ca_clear_channel(channelID); +} + + + void CAChannel::addChannelGet(const CAChannelGetPtr & get) { if(DEBUG_LEVEL>0) { @@ -448,17 +482,6 @@ void CAChannel::addChannelMonitor(const CAChannelMonitorPtr & monitor) monitorList.push_back(monitor); } -CAChannel::~CAChannel() -{ - if(DEBUG_LEVEL>0) { - cout << "CAChannel::~CAChannel() " << channelName << endl; - } - /* Clear CA Channel */ - threadAttach(); - ca_clear_channel(channelID); -} - - chid CAChannel::getChannelID() { return channelID; diff --git a/src/ca/caChannel.h b/src/ca/caChannel.h index cdb716b..72d201c 100644 --- a/src/ca/caChannel.h +++ b/src/ca/caChannel.h @@ -22,8 +22,7 @@ namespace epics { namespace pvAccess { namespace ca { -class CAChannel; -typedef std::tr1::shared_ptr CAChannelPtr; + class CAChannelGetField; typedef std::tr1::shared_ptr CAChannelGetFieldPtr; typedef std::tr1::weak_ptr CAChannelGetFieldWPtr; @@ -60,7 +59,7 @@ public: static size_t num_instances; - static shared_pointer create(CAChannelProvider::shared_pointer const & channelProvider, + static CAChannelPtr create(CAChannelProvider::shared_pointer const & channelProvider, std::string const & channelName, short priority, ChannelRequester::shared_pointer const & channelRequester); @@ -112,6 +111,7 @@ public: void addChannelGet(const CAChannelGetPtr & get); void addChannelPut(const CAChannelPutPtr & get); void addChannelMonitor(const CAChannelMonitorPtr & get); + void disconnectChannel(); private: @@ -129,6 +129,7 @@ private: chid channelID; chtype channelType; unsigned elementCount; + bool channelCreated; epics::pvData::Structure::const_shared_pointer structure; epics::pvData::Mutex requestsMutex; diff --git a/src/ca/caProvider.cpp b/src/ca/caProvider.cpp index 3da9c69..9267285 100644 --- a/src/ca/caProvider.cpp +++ b/src/ca/caProvider.cpp @@ -52,7 +52,28 @@ CAChannelProvider::CAChannelProvider(const std::tr1::shared_ptr&) CAChannelProvider::~CAChannelProvider() { - if(DEBUG_LEVEL>0) std::cout << "CAChannelProvider::~CAChannelProvider()\n"; + if(DEBUG_LEVEL>0) { + std::cout << "CAChannelProvider::~CAChannelProvider()" + << " caChannelList.size() " << caChannelList.size() + << std::endl; + } + std::queue channelQ; + { + Lock lock(channelListMutex); + for(size_t i=0; i< caChannelList.size(); ++i) { + CAChannelPtr caChannel(caChannelList[i].lock()); + if(caChannel) channelQ.push(caChannel); + } + } + while(!channelQ.empty()) { + if(DEBUG_LEVEL>0) { + std::cout << "disconnectAllChannels calling disconnectChannel " + << channelQ.front()->getChannelName() + << std::endl; + } + channelQ.front()->disconnectChannel(); + channelQ.pop(); + } } std::string CAChannelProvider::getProviderName() @@ -112,6 +133,23 @@ Channel::shared_pointer CAChannelProvider::createChannel( return CAChannel::create(shared_from_this(), channelName, priority, channelRequester); } +void CAChannelProvider::addChannel(const CAChannelPtr & channel) +{ + if(DEBUG_LEVEL>0) { + std::cout << "CAChannelProvider::addChannel " + << channel->getChannelName() + << std::endl; + } + Lock lock(channelListMutex); + for(size_t i=0; i< caChannelList.size(); ++i) { + if(!(caChannelList[i].lock())) { + caChannelList[i] = channel; + return; + } + } + caChannelList.push_back(channel); +} + void CAChannelProvider::configure(epics::pvData::PVStructure::shared_pointer /*configuration*/) { } @@ -136,13 +174,11 @@ void CAChannelProvider::initialize() /* Create Channel Access */ int result = ca_context_create(ca_enable_preemptive_callback); if (result != ECA_NORMAL) { - throw std::runtime_error(std::string("CA error %s occurred while trying " - "to start channel access:") + ca_message(result)); + throw std::runtime_error( + std::string("CA error %s occurred while trying to start channel access:") + + ca_message(result)); } - current_context = ca_current_context(); - - // TODO create a ca_poll thread, if ca_disable_preemptive_callback } @@ -154,7 +190,9 @@ void ca_factory_cleanup(void*) ChannelProviderRegistry::clients()->remove("ca"); ca_context_destroy(); } catch(std::exception& e) { - LOG(logLevelWarn, "Error when unregister \"ca\" factory"); + std::string message("Error when unregister \"ca\" factory"); + message += e.what(); + LOG(logLevelWarn,message.c_str()); } } @@ -173,7 +211,7 @@ void CAClientFactory::start() registerRefCounter("CAChannelPut", &CAChannelPut::num_instances); registerRefCounter("CAChannelMonitor", &CAChannelMonitor::num_instances); - if(ChannelProviderRegistry::clients()->add("ca", false)) + if(ChannelProviderRegistry::clients()->add("ca", true)) { epicsAtExit(&ca_factory_cleanup, NULL); } diff --git a/src/ca/caProviderPvt.h b/src/ca/caProviderPvt.h index 3e4b3a5..6d01361 100644 --- a/src/ca/caProviderPvt.h +++ b/src/ca/caProviderPvt.h @@ -19,6 +19,10 @@ namespace ca { #define DEBUG_LEVEL 0 +class CAChannel; +typedef std::tr1::shared_ptr CAChannelPtr; +typedef std::tr1::weak_ptr CAChannelWPtr; + class CAChannelProvider; typedef std::tr1::shared_ptr CAChannelProviderPtr; typedef std::tr1::weak_ptr CAChannelProviderWPtr; @@ -64,13 +68,18 @@ public: virtual void destroy() EPICS_DEPRECATED {}; + void addChannel(const CAChannelPtr & get); + /* ---------------------------------------------------------------- */ void threadAttach(); + private: void initialize(); ca_client_context* current_context; + epics::pvData::Mutex channelListMutex; + std::vector caChannelList; }; } From fa46935d3560f512ea5d8c732fc056f3bf8b9438 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Thu, 4 Jan 2018 17:52:36 -0600 Subject: [PATCH 2/5] Clean up compiler warnings --- src/remote/codec.cpp | 2 +- testApp/utils/configurationTest.cpp | 14 -------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/src/remote/codec.cpp b/src/remote/codec.cpp index 8e89293..8233528 100644 --- a/src/remote/codec.cpp +++ b/src/remote/codec.cpp @@ -1600,7 +1600,7 @@ void BlockingServerTCPTransportCodec::authenticationCompleted(epics::pvData::Sta string errorMessage = "Re-authentication failed: " + status.getMessage(); if (!status.getStackDump().empty()) errorMessage += "\n" + status.getStackDump(); - LOG(logLevelInfo, errorMessage.c_str()); + LOG(logLevelInfo, "%s", errorMessage.c_str()); close(); } diff --git a/testApp/utils/configurationTest.cpp b/testApp/utils/configurationTest.cpp index f00c613..316db3a 100644 --- a/testApp/utils/configurationTest.cpp +++ b/testApp/utils/configurationTest.cpp @@ -32,20 +32,6 @@ void setenv(char * a, char * b, int c) using namespace epics::pvAccess; using namespace epics::pvData; -static const char indata[] = - "hello = world \n" - " # oops\n" - " #dd=da\n" - " empty = \n" - " this = is a test\n\n" - ; - -static const char expectdata[] = - "empty = \n" - "hello = world\n" - "this = is a test\n" - ; - static void showEnv(const char *name) { testDiag("%s = \"%s\"", name, getenv(name)); From 4d2e682a957943f632c621a45940f9dcff054bee Mon Sep 17 00:00:00 2001 From: mrkraimer Date: Mon, 29 Jan 2018 10:19:56 -0500 Subject: [PATCH 3/5] make changes suggested by andrew that remove warning messages --- src/ca/caChannel.cpp | 5 +---- src/ca/caProvider.cpp | 4 +--- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/ca/caChannel.cpp b/src/ca/caChannel.cpp index 1c1d0e9..d145a71 100644 --- a/src/ca/caChannel.cpp +++ b/src/ca/caChannel.cpp @@ -422,13 +422,10 @@ void CAChannel::disconnectChannel() << " channelCreated " << (channelCreated ? "true" : "false") << endl; } - bool disconnect = true; { Lock lock(requestsMutex); - if(!channelCreated) disconnect = false; - channelCreated = false; + if(!channelCreated) return; } - if(!disconnect) return; /* Clear CA Channel */ threadAttach(); ca_clear_channel(channelID); diff --git a/src/ca/caProvider.cpp b/src/ca/caProvider.cpp index 9267285..e7fb52e 100644 --- a/src/ca/caProvider.cpp +++ b/src/ca/caProvider.cpp @@ -190,9 +190,7 @@ void ca_factory_cleanup(void*) ChannelProviderRegistry::clients()->remove("ca"); ca_context_destroy(); } catch(std::exception& e) { - std::string message("Error when unregister \"ca\" factory"); - message += e.what(); - LOG(logLevelWarn,message.c_str()); + LOG(logLevelWarn, "Error on unregistering \"ca\" factory: %s", e.what()); } } From 5c5da3b515eb1f8d1dc4a628f6e6c86c0fe1cdd2 Mon Sep 17 00:00:00 2001 From: Marty Kraimer Date: Mon, 29 Jan 2018 13:53:25 -0500 Subject: [PATCH 4/5] Revert "When caProvider is destroyed make sure all channels are cleared" --- src/ca/caChannel.cpp | 44 ++++++++++------------------------- src/ca/caChannel.h | 7 +++--- src/ca/caProvider.cpp | 52 +++++++----------------------------------- src/ca/caProviderPvt.h | 9 -------- 4 files changed, 23 insertions(+), 89 deletions(-) diff --git a/src/ca/caChannel.cpp b/src/ca/caChannel.cpp index d145a71..a1e3c90 100644 --- a/src/ca/caChannel.cpp +++ b/src/ca/caChannel.cpp @@ -374,8 +374,7 @@ CAChannel::CAChannel(std::string const & _channelName, channelRequester(_channelRequester), channelID(0), channelType(0), - elementCount(0), - channelCreated(false) + elementCount(0) { if(DEBUG_LEVEL>0) { cout<< "CAChannel::CAChannel " << channelName << endl; @@ -396,9 +395,6 @@ void CAChannel::activate(short priority) &channelID); if (result == ECA_NORMAL) { - channelCreated = true; - CAChannelProviderPtr provider(channelProvider.lock()); - if(provider) provider->addChannel(shared_from_this()); req->channelCreated(Status::Ok, shared_from_this()); } else { Status errorStatus(Status::STATUSTYPE_ERROR, string(ca_message(result))); @@ -406,33 +402,6 @@ void CAChannel::activate(short priority) } } -CAChannel::~CAChannel() -{ - if(DEBUG_LEVEL>0) { - cout << "CAChannel::~CAChannel() " << channelName << endl; - } - disconnectChannel(); -} - -void CAChannel::disconnectChannel() -{ - if(DEBUG_LEVEL>0) { - cout << "CAChannel::disconnectChannel() " - << channelName - << " channelCreated " << (channelCreated ? "true" : "false") - << endl; - } - { - Lock lock(requestsMutex); - if(!channelCreated) return; - } - /* Clear CA Channel */ - threadAttach(); - ca_clear_channel(channelID); -} - - - void CAChannel::addChannelGet(const CAChannelGetPtr & get) { if(DEBUG_LEVEL>0) { @@ -479,6 +448,17 @@ void CAChannel::addChannelMonitor(const CAChannelMonitorPtr & monitor) monitorList.push_back(monitor); } +CAChannel::~CAChannel() +{ + if(DEBUG_LEVEL>0) { + cout << "CAChannel::~CAChannel() " << channelName << endl; + } + /* Clear CA Channel */ + threadAttach(); + ca_clear_channel(channelID); +} + + chid CAChannel::getChannelID() { return channelID; diff --git a/src/ca/caChannel.h b/src/ca/caChannel.h index 72d201c..cdb716b 100644 --- a/src/ca/caChannel.h +++ b/src/ca/caChannel.h @@ -22,7 +22,8 @@ namespace epics { namespace pvAccess { namespace ca { - +class CAChannel; +typedef std::tr1::shared_ptr CAChannelPtr; class CAChannelGetField; typedef std::tr1::shared_ptr CAChannelGetFieldPtr; typedef std::tr1::weak_ptr CAChannelGetFieldWPtr; @@ -59,7 +60,7 @@ public: static size_t num_instances; - static CAChannelPtr create(CAChannelProvider::shared_pointer const & channelProvider, + static shared_pointer create(CAChannelProvider::shared_pointer const & channelProvider, std::string const & channelName, short priority, ChannelRequester::shared_pointer const & channelRequester); @@ -111,7 +112,6 @@ public: void addChannelGet(const CAChannelGetPtr & get); void addChannelPut(const CAChannelPutPtr & get); void addChannelMonitor(const CAChannelMonitorPtr & get); - void disconnectChannel(); private: @@ -129,7 +129,6 @@ private: chid channelID; chtype channelType; unsigned elementCount; - bool channelCreated; epics::pvData::Structure::const_shared_pointer structure; epics::pvData::Mutex requestsMutex; diff --git a/src/ca/caProvider.cpp b/src/ca/caProvider.cpp index e7fb52e..3da9c69 100644 --- a/src/ca/caProvider.cpp +++ b/src/ca/caProvider.cpp @@ -52,28 +52,7 @@ CAChannelProvider::CAChannelProvider(const std::tr1::shared_ptr&) CAChannelProvider::~CAChannelProvider() { - if(DEBUG_LEVEL>0) { - std::cout << "CAChannelProvider::~CAChannelProvider()" - << " caChannelList.size() " << caChannelList.size() - << std::endl; - } - std::queue channelQ; - { - Lock lock(channelListMutex); - for(size_t i=0; i< caChannelList.size(); ++i) { - CAChannelPtr caChannel(caChannelList[i].lock()); - if(caChannel) channelQ.push(caChannel); - } - } - while(!channelQ.empty()) { - if(DEBUG_LEVEL>0) { - std::cout << "disconnectAllChannels calling disconnectChannel " - << channelQ.front()->getChannelName() - << std::endl; - } - channelQ.front()->disconnectChannel(); - channelQ.pop(); - } + if(DEBUG_LEVEL>0) std::cout << "CAChannelProvider::~CAChannelProvider()\n"; } std::string CAChannelProvider::getProviderName() @@ -133,23 +112,6 @@ Channel::shared_pointer CAChannelProvider::createChannel( return CAChannel::create(shared_from_this(), channelName, priority, channelRequester); } -void CAChannelProvider::addChannel(const CAChannelPtr & channel) -{ - if(DEBUG_LEVEL>0) { - std::cout << "CAChannelProvider::addChannel " - << channel->getChannelName() - << std::endl; - } - Lock lock(channelListMutex); - for(size_t i=0; i< caChannelList.size(); ++i) { - if(!(caChannelList[i].lock())) { - caChannelList[i] = channel; - return; - } - } - caChannelList.push_back(channel); -} - void CAChannelProvider::configure(epics::pvData::PVStructure::shared_pointer /*configuration*/) { } @@ -174,11 +136,13 @@ void CAChannelProvider::initialize() /* Create Channel Access */ int result = ca_context_create(ca_enable_preemptive_callback); if (result != ECA_NORMAL) { - throw std::runtime_error( - std::string("CA error %s occurred while trying to start channel access:") - + ca_message(result)); + throw std::runtime_error(std::string("CA error %s occurred while trying " + "to start channel access:") + ca_message(result)); } + current_context = ca_current_context(); + + // TODO create a ca_poll thread, if ca_disable_preemptive_callback } @@ -190,7 +154,7 @@ void ca_factory_cleanup(void*) ChannelProviderRegistry::clients()->remove("ca"); ca_context_destroy(); } catch(std::exception& e) { - LOG(logLevelWarn, "Error on unregistering \"ca\" factory: %s", e.what()); + LOG(logLevelWarn, "Error when unregister \"ca\" factory"); } } @@ -209,7 +173,7 @@ void CAClientFactory::start() registerRefCounter("CAChannelPut", &CAChannelPut::num_instances); registerRefCounter("CAChannelMonitor", &CAChannelMonitor::num_instances); - if(ChannelProviderRegistry::clients()->add("ca", true)) + if(ChannelProviderRegistry::clients()->add("ca", false)) { epicsAtExit(&ca_factory_cleanup, NULL); } diff --git a/src/ca/caProviderPvt.h b/src/ca/caProviderPvt.h index 6d01361..3e4b3a5 100644 --- a/src/ca/caProviderPvt.h +++ b/src/ca/caProviderPvt.h @@ -19,10 +19,6 @@ namespace ca { #define DEBUG_LEVEL 0 -class CAChannel; -typedef std::tr1::shared_ptr CAChannelPtr; -typedef std::tr1::weak_ptr CAChannelWPtr; - class CAChannelProvider; typedef std::tr1::shared_ptr CAChannelProviderPtr; typedef std::tr1::weak_ptr CAChannelProviderWPtr; @@ -68,18 +64,13 @@ public: virtual void destroy() EPICS_DEPRECATED {}; - void addChannel(const CAChannelPtr & get); - /* ---------------------------------------------------------------- */ void threadAttach(); - private: void initialize(); ca_client_context* current_context; - epics::pvData::Mutex channelListMutex; - std::vector caChannelList; }; } From cb21eb4f1ee0230da83f4b6f7a3dd0862212d4de Mon Sep 17 00:00:00 2001 From: mrkraimer Date: Tue, 30 Jan 2018 05:08:43 -0500 Subject: [PATCH 5/5] prevent CAChannel::disconnectChannel() from calling ca_clear_channel twice This was causing a crash when exiting --- src/ca/caChannel.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ca/caChannel.cpp b/src/ca/caChannel.cpp index d145a71..b2cd481 100644 --- a/src/ca/caChannel.cpp +++ b/src/ca/caChannel.cpp @@ -425,6 +425,7 @@ void CAChannel::disconnectChannel() { Lock lock(requestsMutex); if(!channelCreated) return; + channelCreated = false; } /* Clear CA Channel */ threadAttach();