From: Way Vadhanasin Date: Sat, 4 Mar 2017 07:16:04 +0000 (-0800) Subject: IOT-1867 Add reference count to OCInit and OCStop X-Git-Tag: 1.3.0~544 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5343a1b86221b54a5513dc68b055c00e53318cef;p=platform%2Fupstream%2Fiotivity.git IOT-1867 Add reference count to OCInit and OCStop This change adds reference count to OCStack and synchronizes the two C APIs. The change also introduces ocatomic.h, which defines some useful atomic utility functions. Change-Id: I2ea4023ab1d1dfda882a0d289db6d8ffdac1bdc4 Signed-off-by: Way Vadhanasin Reviewed-on: https://gerrit.iotivity.org/gerrit/17625 Tested-by: jenkins-iotivity Reviewed-by: Dan Mihai --- diff --git a/resource/c_common/SConscript b/resource/c_common/SConscript index bc1044b..f64c3c8 100644 --- a/resource/c_common/SConscript +++ b/resource/c_common/SConscript @@ -135,6 +135,7 @@ env.AppendUnique(CPPPATH = [ os.path.join(Dir('.').abspath, 'oic_malloc', 'include'), os.path.join(Dir('.').abspath, 'oic_string', 'include'), os.path.join(Dir('.').abspath, 'oic_time', 'include'), + os.path.join(Dir('.').abspath, 'ocatomic', 'include'), os.path.join(Dir('.').abspath, 'ocrandom', 'include'), os.path.join(Dir('.').abspath, 'octhread', 'include') ]) @@ -180,11 +181,19 @@ elif target_os in ['windows']: else: common_src.append('octhread/src/noop/octhread.c') +if target_os in ['windows', 'msys_nt']: + common_src.append('ocatomic/src/windows/ocatomic.c') +elif target_os in ['arduino']: + common_src.append('ocatomic/src/arduino/ocatomic.c') +else: + common_src.append('ocatomic/src/others/ocatomic.c') + common_env.AppendUnique(LIBS = ['logger']) common_env.AppendUnique(CPPPATH = ['#resource/csdk/logger/include']) commonlib = common_env.StaticLibrary('c_common', common_src) common_env.InstallTarget(commonlib, 'c_common') common_env.UserInstallTargetLib(commonlib, 'c_common') +common_env.UserInstallTargetHeader('iotivity_debug.h', 'c_common', 'iotivity_debug.h') common_env.UserInstallTargetHeader('platform_features.h', 'c_common', 'platform_features.h') env.PrependUnique(LIBS = ['c_common']) diff --git a/resource/c_common/ocatomic/include/ocatomic.h b/resource/c_common/ocatomic/include/ocatomic.h new file mode 100644 index 0000000..48ea2ee --- /dev/null +++ b/resource/c_common/ocatomic/include/ocatomic.h @@ -0,0 +1,49 @@ +/* ***************************************************************** + * + * Copyright 2017 Microsoft + * + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************/ +#ifndef OC_ATOMIC_H +#define OC_ATOMIC_H + +#include + +#ifdef __cplusplus +extern "C" +{ +#endif /* __cplusplus */ + +/** + * Increments (increases by one) the value of the specified int32_t variable atomically. + * + * @param[in] addend Pointer to the variable to be incremented. + * @return int32_t The resulting incremented value. + */ +int32_t oc_atomic_increment(volatile int32_t *addend); + +/** + * Decrements (decreases by one) the value of the specified int32_t variable atomically. + * + * @param[in] addend Pointer to the variable to be decremented. + * @return int32_t The resulting decremented value. + */ +int32_t oc_atomic_decrement(volatile int32_t *addend); + +#ifdef __cplusplus +} /* extern "C" */ +#endif /* __cplusplus */ + +#endif /* OC_ATOMIC_H */ diff --git a/resource/c_common/ocatomic/src/arduino/ocatomic.c b/resource/c_common/ocatomic/src/arduino/ocatomic.c new file mode 100644 index 0000000..5c5b6db --- /dev/null +++ b/resource/c_common/ocatomic/src/arduino/ocatomic.c @@ -0,0 +1,38 @@ +/* ***************************************************************** + * + * Copyright 2017 Microsoft + * + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************/ + +/** + * @file + * This file implements stubs for atomic functions. These stubs are not designed + * to be used by multi-threaded OS. + */ + +#include "ocatomic.h" + +int32_t oc_atomic_increment(volatile int32_t *addend) +{ + (*addend)++; + return *addend; +} + +int32_t oc_atomic_decrement(volatile int32_t *addend) +{ + (*addend)--; + return *addend; +} diff --git a/resource/c_common/ocatomic/src/others/ocatomic.c b/resource/c_common/ocatomic/src/others/ocatomic.c new file mode 100644 index 0000000..8367cf0 --- /dev/null +++ b/resource/c_common/ocatomic/src/others/ocatomic.c @@ -0,0 +1,35 @@ +/* ***************************************************************** + * + * Copyright 2017 Microsoft + * + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************/ + +/** + * @file + * This file implements APIs related to atomic operations compatible with GCC compilers. + */ + +#include "ocatomic.h" + +int32_t oc_atomic_increment(volatile int32_t *addend) +{ + return __sync_add_and_fetch(addend, 1); +} + +int32_t oc_atomic_decrement(volatile int32_t *addend) +{ + return __sync_sub_and_fetch(addend, 1); +} diff --git a/resource/c_common/ocatomic/src/windows/ocatomic.c b/resource/c_common/ocatomic/src/windows/ocatomic.c new file mode 100644 index 0000000..3dea8ff --- /dev/null +++ b/resource/c_common/ocatomic/src/windows/ocatomic.c @@ -0,0 +1,36 @@ +/* ***************************************************************** + * + * Copyright 2017 Microsoft + * + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************/ + +/** + * @file + * This file implements APIs related to atomic operations for Windows. + */ + +#include "ocatomic.h" +#include + +int32_t oc_atomic_increment(volatile int32_t *addend) +{ + return InterlockedIncrement((volatile long*)addend); +} + +int32_t oc_atomic_decrement(volatile int32_t *addend) +{ + return InterlockedDecrement((volatile long*)addend); +} diff --git a/resource/csdk/stack/SConscript b/resource/csdk/stack/SConscript index d700e6c..254f463 100644 --- a/resource/csdk/stack/SConscript +++ b/resource/csdk/stack/SConscript @@ -57,6 +57,7 @@ else: liboctbstack_env.PrependUnique(CPPPATH = [ '#/extlibs/timer/', + '#resource/c_common/ocatomic/include', '#resource/csdk/logger/include', '#resource/csdk/include', 'include', diff --git a/resource/csdk/stack/src/ocstack.c b/resource/csdk/stack/src/ocstack.c index 54c1657..5f8e57a 100644 --- a/resource/csdk/stack/src/ocstack.c +++ b/resource/csdk/stack/src/ocstack.c @@ -37,10 +37,14 @@ #define __STDC_LIMIT_MACROS #endif #include "iotivity_config.h" +#include "iotivity_debug.h" #include #include #include #include +#ifdef HAVE_UNISTD_H +#include +#endif #include "ocstack.h" #include "ocstackinternal.h" @@ -64,6 +68,8 @@ #include "cainterface.h" #include "oicgroup.h" #include "ocendpoint.h" +#include "ocatomic.h" +#include "platform_features.h" #if defined(TCP_ADAPTER) && defined(WITH_CLOUD) #include "occonnectionmanager.h" @@ -106,8 +112,7 @@ typedef enum { OC_STACK_UNINITIALIZED = 0, - OC_STACK_INITIALIZED, - OC_STACK_UNINIT_IN_PROGRESS + OC_STACK_INITIALIZED } OCStackState; #ifdef WITH_PRESENCE @@ -156,6 +161,11 @@ CAAdapterStateChangedCB g_adapterHandler = NULL; CAConnectionStateChangedCB g_connectionHandler = NULL; // Persistent Storage callback handler for open/read/write/close/unlink static OCPersistentStorage *g_PersistentStorageHandler = NULL; +// Number of users of OCStack, based on the successful calls to OCInit2 prior to OCStop +// The variable must not be declared static because it is also referenced by the unit test +uint32_t g_ocStackStartCount = 0; +// Number of threads currently executing OCInit2 or OCStop +volatile int32_t g_ocStackStartStopThreadCount = 0; //----------------------------------------------------------------------------- // Macros @@ -438,9 +448,55 @@ static void OCSetNetworkMonitorHandler(CAAdapterStateChangedCB adapterHandler, static OCStackResult OCMapZoneIdToLinkLocalEndpoint(OCDiscoveryPayload *payload, uint32_t ifindex); #endif +/** + * Initialize the stack. + * Caller of this function must serialize calls to this function and the stop counterpart. + * @param mode Mode of operation. + * @param serverFlags The server flag used when the mode of operation is a server mode. + * @param clientFlags The client flag used when the mode of operation is a client mode. + * @param transportType The transport type. + * + * @return ::OC_STACK_OK on success, some other value upon failure. + */ +static OCStackResult OCInitializeInternal(OCMode mode, OCTransportFlags serverFlags, + OCTransportFlags clientFlags, OCTransportAdapter transportType); + +/** + * DeInitialize the stack. + * Caller of this function must serialize calls to this function and the init counterpart. + * + * @return ::OC_STACK_OK on success, some other value upon failure. + */ +static OCStackResult OCDeInitializeInternal(); + //----------------------------------------------------------------------------- // Internal functions //----------------------------------------------------------------------------- +static void OCEnterInitializer() +{ + for (;;) + { + int32_t initCount = oc_atomic_increment(&g_ocStackStartStopThreadCount); + assert(initCount > 0); + if (initCount == 1) + { + break; + } + OC_VERIFY(oc_atomic_decrement(&g_ocStackStartStopThreadCount) >= 0); +#if !defined(ARDUINO) + // Yield execution to the thread that is holding the lock. + sleep(0); +#else // ARDUINO + assert(!"Not expecting initCount to go above 1 on Arduino"); + break; +#endif // ARDUINO + } +} + +static void OCLeaveInitializer() +{ + OC_VERIFY(oc_atomic_decrement(&g_ocStackStartStopThreadCount) >= 0); +} bool checkProxyUri(OCHeaderOption *options, uint8_t numOptions) { @@ -2427,10 +2483,38 @@ OCStackResult OCInit1(OCMode mode, OCTransportFlags serverFlags, OCTransportFlag return OCInit2(mode, OC_DEFAULT_FLAGS, OC_DEFAULT_FLAGS, OC_DEFAULT_ADAPTER); } -OCStackResult OCInit2(OCMode mode, OCTransportFlags serverFlags, OCTransportFlags clientFlags, - OCTransportAdapter transportType) +OCStackResult OCInit2(OCMode mode, OCTransportFlags serverFlags, + OCTransportFlags clientFlags, OCTransportAdapter transportType) +{ + OIC_LOG(INFO, TAG, "Entering OCInit2"); + + // Serialize calls to start and stop the stack. + OCEnterInitializer(); + + OCStackResult result = OC_STACK_OK; + + if (g_ocStackStartCount == 0) + { + // This is the first call to initialize the stack so it gets to do the real work. + result = OCInitializeInternal(mode, serverFlags, clientFlags, transportType); + } + + if (result == OC_STACK_OK) + { + // Increment the start count since we're about to return success. + assert(g_ocStackStartCount != UINT_MAX); + assert(stackState == OC_STACK_INITIALIZED); + g_ocStackStartCount++; + } + + OCLeaveInitializer(); + return result; +} + +OCStackResult OCInitializeInternal(OCMode mode, OCTransportFlags serverFlags, + OCTransportFlags clientFlags, OCTransportAdapter transportType) { - if(stackState == OC_STACK_INITIALIZED) + if (stackState == OC_STACK_INITIALIZED) { OIC_LOG(INFO, TAG, "Subsequent calls to OCInit() without calling \ OCStop() between them are ignored."); @@ -2454,7 +2538,6 @@ OCStackResult OCInit2(OCMode mode, OCTransportFlags serverFlags, OCTransportFlag #endif OCStackResult result = OC_STACK_ERROR; - OIC_LOG(INFO, TAG, "Entering OCInit"); // Validate mode if (!((mode == OC_CLIENT) || (mode == OC_SERVER) || (mode == OC_CLIENT_SERVER) @@ -2583,22 +2666,35 @@ OCStackResult OCStop() { OIC_LOG(INFO, TAG, "Entering OCStop"); - if (stackState == OC_STACK_UNINIT_IN_PROGRESS) + // Serialize calls to start and stop the stack. + OCEnterInitializer(); + + OCStackResult result = OC_STACK_OK; + + if (g_ocStackStartCount == 1) { - OIC_LOG(DEBUG, TAG, "Stack already stopping, exiting"); - return OC_STACK_OK; + // This is the last call to stop the stack, do the real work. + result = OCDeInitializeInternal(); } - else if (stackState != OC_STACK_INITIALIZED) + else if (g_ocStackStartCount == 0) { - OIC_LOG(INFO, TAG, "Stack not initialized"); - return OC_STACK_ERROR; + OIC_LOG(ERROR, TAG, "Too many calls to OCStop"); + assert(!"Too many calls to OCStop"); + result = OC_STACK_ERROR; } - // unset cautil config - CAUtilConfig_t configs = {(CATransportBTFlags_t)CA_DEFAULT_BT_FLAGS}; - CAUtilSetBTConfigure(configs); + if (result == OC_STACK_OK) + { + g_ocStackStartCount--; + } - stackState = OC_STACK_UNINIT_IN_PROGRESS; + OCLeaveInitializer(); + return result; +} + +OCStackResult OCDeInitializeInternal() +{ + assert(stackState == OC_STACK_INITIALIZED); #ifdef WITH_PRESENCE // Ensure that the TTL associated with ANY and ALL presence notifications originating from @@ -2640,6 +2736,10 @@ OCStackResult OCStop() OCCMTerminate(); #endif + // Unset cautil config + CAUtilConfig_t configs = {(CATransportBTFlags_t)CA_DEFAULT_BT_FLAGS}; + CAUtilSetBTConfigure(configs); + stackState = OC_STACK_UNINITIALIZED; return OC_STACK_OK; } diff --git a/resource/csdk/stack/test/stacktests.cpp b/resource/csdk/stack/test/stacktests.cpp index c7e5715..031af22 100644 --- a/resource/csdk/stack/test/stacktests.cpp +++ b/resource/csdk/stack/test/stacktests.cpp @@ -179,6 +179,8 @@ uint8_t InitResourceIndex() #endif } +extern "C" uint32_t g_ocStackStartCount; + class OCDiscoverTests : public testing::Test { protected: @@ -200,7 +202,9 @@ TEST(StackInit, StackInitNullAddr) { itst::DeadmanTimer killSwitch(SHORT_TEST_TIMEOUT); EXPECT_EQ(OC_STACK_OK, OCInit(0, 5683, OC_SERVER)); + EXPECT_EQ(1, g_ocStackStartCount); EXPECT_EQ(OC_STACK_OK, OCStop()); + EXPECT_EQ(0, g_ocStackStartCount); } TEST(StackInit, StackInitNullPort) @@ -221,7 +225,7 @@ TEST(StackInit, StackInitInvalidMode) { itst::DeadmanTimer killSwitch(SHORT_TEST_TIMEOUT); EXPECT_EQ(OC_STACK_ERROR, OCInit(0, 0, (OCMode)10)); - EXPECT_EQ(OC_STACK_ERROR, OCStop()); + EXPECT_EQ(0, g_ocStackStartCount); } TEST(StackStart, StackStartSuccessClient) @@ -266,9 +270,15 @@ TEST(StackStart, StackStartSuccessClientThenServer) TEST(StackStart, StackStartSuccessiveInits) { itst::DeadmanTimer killSwitch(SHORT_TEST_TIMEOUT); + EXPECT_EQ(0, g_ocStackStartCount); EXPECT_EQ(OC_STACK_OK, OCInit("127.0.0.1", 5683, OC_SERVER)); + EXPECT_EQ(1, g_ocStackStartCount); EXPECT_EQ(OC_STACK_OK, OCInit("127.0.0.2", 5683, OC_SERVER)); + EXPECT_EQ(2, g_ocStackStartCount); + EXPECT_EQ(OC_STACK_OK, OCStop()); + EXPECT_EQ(1, g_ocStackStartCount); EXPECT_EQ(OC_STACK_OK, OCStop()); + EXPECT_EQ(0, g_ocStackStartCount); } TEST(StackStart, SetPlatformInfoValid) @@ -612,20 +622,6 @@ TEST(StackDiscovery, DISABLED_DoResourceDeviceDiscovery) EXPECT_EQ(OC_STACK_OK, OCStop()); } -TEST(StackStop, StackStopWithoutInit) -{ - itst::DeadmanTimer killSwitch(SHORT_TEST_TIMEOUT); - EXPECT_EQ(OC_STACK_ERROR, OCStop()); -} - -TEST(StackStop, StackStopRepeated) -{ - itst::DeadmanTimer killSwitch(SHORT_TEST_TIMEOUT); - EXPECT_EQ(OC_STACK_OK, OCInit("127.0.0.1", 5683, OC_CLIENT)); - EXPECT_EQ(OC_STACK_OK, OCStop()); - EXPECT_EQ(OC_STACK_ERROR, OCStop()); -} - TEST(StackResource, DISABLED_UpdateResourceNullURI) { itst::DeadmanTimer killSwitch(SHORT_TEST_TIMEOUT);