Fixed IOT-110--Crash on destruction

A shared_ptr loop in callbacks for listen and a server destructor
were causing the stack to crash upon destruction of the Platform_impl.
Platform_impl would decrement the ClientWrapper shared_ptr (which would
still be >0 due to the callback having one).  When the OCStop was cleaning
up its resources, it would delete the client context, decrementing the
ClientWrapper ptr completely, resulting in OCStop being called inside
itself.

This bug was fixed in a few ways (each which are important, but would
have fixed this independently).

1- Changed OCStop to protect itself from re-entrance by using a guard-bool
to return immediately if it is being called as a child.

2- ListenContext now contains a weak_ptr to the clientWrapper instead of
a shared_ptr.  This makes it not destruct the clientWrapper upon callback
cleanup.

3- ClientWrapper no longer calls OCStop in the destructor when in BOTH
mode.  This is done to be consistent with the OCInit call in the constructor
as well as to correctly give the ServerWrapper ownership of the Init/Process/
Stop functionality in the BOTH case.

Change-Id: I77c4646938f586e24ac6642fdeeaf82391ab082c
Signed-off-by: Erich Keane <erich.keane@intel.com>

diff --git a/resource/csdk/stack/src/ocstack.c b/resource/csdk/stack/src/ocstack.c
index eb0aef8..f09d19f 100644 (file)
--- a/resource/csdk/stack/src/ocstack.c
+++ b/resource/csdk/stack/src/ocstack.c
@@ -37,7 +37,7 @@
 // Typedefs
 //-----------------------------------------------------------------------------
 typedef enum {
-    OC_STACK_UNINITIALIZED = 0, OC_STACK_INITIALIZED
+    OC_STACK_UNINITIALIZED = 0, OC_STACK_INITIALIZED, OC_STACK_UNINIT_IN_PROGRESS
 } OCStackState;
 
 #ifdef WITH_PRESENCE
@@ -433,12 +433,19 @@ OCStackResult OCStop()
 
     OC_LOG(INFO, TAG, PCF("Entering OCStop"));
 
-    if (stackState != OC_STACK_INITIALIZED)
+    if (stackState == OC_STACK_UNINIT_IN_PROGRESS)
+    {
+        OC_LOG(DEBUG, TAG, PCF("Stack already stopping, exiting"));
+        return OC_STACK_OK;
+    }
+    else if (stackState != OC_STACK_INITIALIZED)
     {
         OC_LOG(ERROR, TAG, PCF("Stack not initialized"));
         return OC_STACK_ERROR;
     }
 
+    stackState = OC_STACK_UNINIT_IN_PROGRESS;
+
     #ifdef WITH_PRESENCE
     // Ensure that the TTL associated with ANY and ALL presence notifications originating from
     // here send with the code "OC_STACK_PRESENCE_STOPPED" result.
@@ -458,6 +465,7 @@ OCStackResult OCStop()
         stackState = OC_STACK_UNINITIALIZED;
         result = OC_STACK_OK;
     } else {
+        stackState = OC_STACK_INITIALIZED;
         result = OC_STACK_ERROR;
     }
 

diff --git a/resource/include/InProcClientWrapper.h b/resource/include/InProcClientWrapper.h
index 39e1df4..9ca0779 100644 (file)
--- a/resource/include/InProcClientWrapper.h
+++ b/resource/include/InProcClientWrapper.h
@@ -49,7 +49,7 @@ namespace OC
         struct ListenContext
         {
             FindCallback callback;
-            IClientWrapper::Ptr clientWrapper;
+            std::weak_ptr<IClientWrapper> clientWrapper;
         };
 
         struct SubscribePresenceContext

diff --git a/resource/src/InProcClientWrapper.cpp b/resource/src/InProcClientWrapper.cpp
index e7804c1..e9580b2 100644 (file)
--- a/resource/src/InProcClientWrapper.cpp
+++ b/resource/src/InProcClientWrapper.cpp
@@ -58,7 +58,12 @@ namespace OC
             m_listeningThread.join();
         }
 
-        OCStop();
+        // only stop if we are the ones who actually called 'init'.  We are counting
+        // on the server to do the stop.
+        if(m_cfg.mode == ModeType::Client)
+        {
+            OCStop();
+        }
     }
 
     void InProcClientWrapper::listeningFunc()
@@ -102,12 +107,21 @@ namespace OC
             return OC_STACK_KEEP_TRANSACTION;
         }
 
+        auto clientWrapper = context->clientWrapper.lock();
+
+        if(!clientWrapper)
+        {
+            oclog() << "listenCallback(): failed to get a shared_ptr to the client wrapper"
+                    << std::flush;
+            return OC_STACK_KEEP_TRANSACTION;
+        }
+
         std::stringstream requestStream;
         requestStream << clientResponse->resJSONPayload;
 
         try
         {
-            ListenOCContainer container(context->clientWrapper, *clientResponse->addr,
+            ListenOCContainer container(clientWrapper, *clientResponse->addr,
                     requestStream);
 
             // loop to ensure valid construction of all resources