From 3a73cb687bef05c28267ada7c0f4663a85357a33 Mon Sep 17 00:00:00 2001
From: Matej Sekoranja <matej.sekoranja@cosylab.com>
Date: Tue, 14 Jun 2016 22:34:14 +0200
Subject: [PATCH] ca provider destruction fixed

resolves #53
---
 src/ca/caChannel.cpp   | 29 +++++++++++++++++++++++++----
 src/ca/caProvider.cpp  | 18 ++++++++++++++++--
 src/ca/pv/caChannel.h  |  5 ++++-
 src/ca/pv/caProvider.h |  7 ++++++-
 4 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/src/ca/caChannel.cpp b/src/ca/caChannel.cpp
index 2e51da1..874d614 100644
--- a/src/ca/caChannel.cpp
+++ b/src/ca/caChannel.cpp
@@ -251,7 +251,8 @@ CAChannel::CAChannel(std::string const & _channelName,
     channelRequester(_channelRequester),
     channelID(0),
     channelType(0),
-    elementCount(0)
+    elementCount(0),
+    destroyed(false)
 {
     PVACCESS_REFCOUNT_MONITOR_CONSTRUCT(caChannel);
 }
@@ -265,6 +266,8 @@ void CAChannel::activate(short priority)
                                    &channelID);
     if (result == ECA_NORMAL)
     {
+        channelProvider->registerChannel(shared_from_this());
+
         // TODO be sure that ca_connection_handler is not called before this call
         EXCEPTION_GUARD(channelRequester->channelCreated(Status::Ok, shared_from_this()));
     }
@@ -278,6 +281,19 @@ void CAChannel::activate(short priority)
 CAChannel::~CAChannel()
 {
     PVACCESS_REFCOUNT_MONITOR_DESTRUCT(caChannel);
+
+    Lock lock(requestsMutex);
+    {
+        if (destroyed)
+            return;
+        destroyed = true;
+    }
+
+    channelProvider->unregisterChannel(this);
+
+    /* Clear CA Channel */
+    threadAttach();
+    ca_clear_channel(channelID);
 }
 
 
@@ -454,19 +470,24 @@ void CAChannel::printInfo(std::ostream& out)
 
 void CAChannel::destroy()
 {
-    threadAttach();
-
     Lock lock(requestsMutex);
     {
+        if (destroyed)
+            return;
+        destroyed = true;
+
         while (!requests.empty())
         {
-            ChannelRequest::shared_pointer request = requests.rbegin()->second.lock();
+            ChannelRequest::shared_pointer request = requests.begin()->second.lock();
             if (request)
                 request->destroy();
         }
     }
 
+    channelProvider->unregisterChannel(shared_from_this());
+
     /* Clear CA Channel */
+    threadAttach();
     ca_clear_channel(channelID);
 }
 
diff --git a/src/ca/caProvider.cpp b/src/ca/caProvider.cpp
index 0ec6d3c..7f3ebc6 100644
--- a/src/ca/caProvider.cpp
+++ b/src/ca/caProvider.cpp
@@ -25,13 +25,15 @@ using namespace epics::pvAccess::ca;
 
 std::string CAChannelProvider::PROVIDER_NAME = "ca";
 
-CAChannelProvider::CAChannelProvider() : current_context(0)
+CAChannelProvider::CAChannelProvider() : current_context(0), destroyed(false)
 {
     initialize();
 }
 
 CAChannelProvider::~CAChannelProvider()
 {
+    // call destroy() to destroy CA context
+    destroy();
 }
 
 std::string CAChannelProvider::getProviderName()
@@ -107,11 +109,17 @@ void CAChannelProvider::destroy()
 {
     Lock lock(channelsMutex);
     {
+        if (destroyed)
+            return;
+        destroyed = true;
+
         while (!channels.empty())
         {
-            Channel::shared_pointer channel = channels.rbegin()->second.lock();
+            Channel::shared_pointer channel = channels.begin()->second.lock();
             if (channel)
                 channel->destroy();
+            else
+                channels.erase(channels.begin());
         }
     }
 
@@ -136,6 +144,12 @@ void CAChannelProvider::unregisterChannel(Channel::shared_pointer const & channe
     channels.erase(channel.get());
 }
 
+void CAChannelProvider::unregisterChannel(Channel* pchannel)
+{
+    Lock lock(channelsMutex);
+    channels.erase(pchannel);
+}
+
 void CAChannelProvider::initialize()
 {
     /* Create Channel Access */
diff --git a/src/ca/pv/caChannel.h b/src/ca/pv/caChannel.h
index e7b5cfa..f2ef22b 100644
--- a/src/ca/pv/caChannel.h
+++ b/src/ca/pv/caChannel.h
@@ -112,9 +112,12 @@ private:
 
     epics::pvData::Mutex requestsMutex;
     // TODO std::unordered_map
-    // void* is not the nicest thing, but there is no fast weak_ptr==
+    // void* is not the nicest thing, but there is no fast weak_ptr::operator==
     typedef std::map<void*, ChannelRequest::weak_pointer> RequestsList;
     RequestsList requests;
+
+    // synced on requestsMutex
+    bool destroyed;
 };
 
 
diff --git a/src/ca/pv/caProvider.h b/src/ca/pv/caProvider.h
index 73c2c45..775ded4 100644
--- a/src/ca/pv/caProvider.h
+++ b/src/ca/pv/caProvider.h
@@ -23,6 +23,7 @@ class epicsShareClass CAChannelProvider :
     public std::tr1::enable_shared_from_this<CAChannelProvider>
 {
 public:
+    POINTER_DEFINITIONS(CAChannelProvider);
 
     static std::string PROVIDER_NAME;
 
@@ -63,6 +64,7 @@ public:
 
     void registerChannel(Channel::shared_pointer const & channel);
     void unregisterChannel(Channel::shared_pointer const & channel);
+    void unregisterChannel(Channel* pchannel);
 
 private:
 
@@ -72,9 +74,12 @@ private:
 
     epics::pvData::Mutex channelsMutex;
     // TODO std::unordered_map
-    // void* is not the nicest thing, but there is no fast weak_ptr==
+    // void* is not the nicest thing, but there is no fast weak_ptr::operator==
     typedef std::map<void*, Channel::weak_pointer> ChannelList;
     ChannelList channels;
+
+    // synced on channelsMutex
+    bool destroyed;
 };