From 95a02142405621a2907f05de60867d43f0d38f8e Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Wed, 10 Dec 2014 13:43:37 -0800 Subject: [PATCH] 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 --- resource/csdk/stack/src/ocstack.c | 12 ++++++++++-- resource/include/InProcClientWrapper.h | 2 +- resource/src/InProcClientWrapper.cpp | 18 ++++++++++++++++-- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/resource/csdk/stack/src/ocstack.c b/resource/csdk/stack/src/ocstack.c index eb0aef8..f09d19f 100644 --- 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 --- 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 clientWrapper; }; struct SubscribePresenceContext diff --git a/resource/src/InProcClientWrapper.cpp b/resource/src/InProcClientWrapper.cpp index e7804c1..e9580b2 100644 --- 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 -- 2.7.4