IOT-1867 Add reference count to OCInit and OCStop 25/17625/10
authorWay Vadhanasin <wayvad@microsoft.com>
Sat, 4 Mar 2017 07:16:04 +0000 (23:16 -0800)
committerDan Mihai <Daniel.Mihai@microsoft.com>
Sat, 4 Mar 2017 18:02:58 +0000 (18:02 +0000)
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 <wayvad@microsoft.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/17625
Tested-by: jenkins-iotivity <jenkins@iotivity.org>
Reviewed-by: Dan Mihai <Daniel.Mihai@microsoft.com>
resource/c_common/SConscript
resource/c_common/ocatomic/include/ocatomic.h [new file with mode: 0644]
resource/c_common/ocatomic/src/arduino/ocatomic.c [new file with mode: 0644]
resource/c_common/ocatomic/src/others/ocatomic.c [new file with mode: 0644]
resource/c_common/ocatomic/src/windows/ocatomic.c [new file with mode: 0644]
resource/csdk/stack/SConscript
resource/csdk/stack/src/ocstack.c
resource/csdk/stack/test/stacktests.cpp

index bc1044b..f64c3c8 100644 (file)
@@ -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 (file)
index 0000000..48ea2ee
--- /dev/null
@@ -0,0 +1,49 @@
+/* *****************************************************************\r
+ *\r
+ * Copyright 2017 Microsoft\r
+ *\r
+ *\r
+ * Licensed under the Apache License, Version 2.0 (the "License");\r
+ * you may not use this file except in compliance with the License.\r
+ * You may obtain a copy of the License at\r
+ *\r
+ *      http://www.apache.org/licenses/LICENSE-2.0\r
+ *\r
+ * Unless required by applicable law or agreed to in writing, software\r
+ * distributed under the License is distributed on an "AS IS" BASIS,\r
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\r
+ * See the License for the specific language governing permissions and\r
+ * limitations under the License.\r
+ *\r
+ ******************************************************************/\r
+#ifndef OC_ATOMIC_H\r
+#define OC_ATOMIC_H\r
+\r
+#include <stdint.h>\r
+\r
+#ifdef __cplusplus\r
+extern "C"\r
+{\r
+#endif /* __cplusplus */\r
+\r
+/**\r
+ * Increments (increases by one) the value of the specified int32_t variable atomically.\r
+ *\r
+ * @param[in] addend  Pointer to the variable to be incremented.\r
+ * @return int32_t  The resulting incremented value.\r
+ */\r
+int32_t oc_atomic_increment(volatile int32_t *addend);\r
+\r
+/**\r
+ * Decrements (decreases by one) the value of the specified int32_t variable atomically.\r
+ *\r
+ * @param[in] addend  Pointer to the variable to be decremented.\r
+ * @return int32_t  The resulting decremented value.\r
+ */\r
+int32_t oc_atomic_decrement(volatile int32_t *addend);\r
+\r
+#ifdef __cplusplus\r
+} /* extern "C" */\r
+#endif /* __cplusplus */\r
+\r
+#endif /* OC_ATOMIC_H */\r
diff --git a/resource/c_common/ocatomic/src/arduino/ocatomic.c b/resource/c_common/ocatomic/src/arduino/ocatomic.c
new file mode 100644 (file)
index 0000000..5c5b6db
--- /dev/null
@@ -0,0 +1,38 @@
+/* *****************************************************************\r
+ *\r
+ * Copyright 2017 Microsoft\r
+ *\r
+ *\r
+ * Licensed under the Apache License, Version 2.0 (the "License");\r
+ * you may not use this file except in compliance with the License.\r
+ * You may obtain a copy of the License at\r
+ *\r
+ *      http://www.apache.org/licenses/LICENSE-2.0\r
+ *\r
+ * Unless required by applicable law or agreed to in writing, software\r
+ * distributed under the License is distributed on an "AS IS" BASIS,\r
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\r
+ * See the License for the specific language governing permissions and\r
+ * limitations under the License.\r
+ *\r
+ ******************************************************************/\r
+\r
+/**\r
+ * @file\r
+ * This file implements stubs for atomic functions. These stubs are not designed\r
+ * to be used by multi-threaded OS.\r
+ */\r
+\r
+#include "ocatomic.h"\r
+\r
+int32_t oc_atomic_increment(volatile int32_t *addend)\r
+{\r
+    (*addend)++;\r
+    return *addend;\r
+}\r
+\r
+int32_t oc_atomic_decrement(volatile int32_t *addend)\r
+{\r
+    (*addend)--;\r
+    return *addend;\r
+}\r
diff --git a/resource/c_common/ocatomic/src/others/ocatomic.c b/resource/c_common/ocatomic/src/others/ocatomic.c
new file mode 100644 (file)
index 0000000..8367cf0
--- /dev/null
@@ -0,0 +1,35 @@
+/* *****************************************************************\r
+ *\r
+ * Copyright 2017 Microsoft\r
+ *\r
+ *\r
+ * Licensed under the Apache License, Version 2.0 (the "License");\r
+ * you may not use this file except in compliance with the License.\r
+ * You may obtain a copy of the License at\r
+ *\r
+ *      http://www.apache.org/licenses/LICENSE-2.0\r
+ *\r
+ * Unless required by applicable law or agreed to in writing, software\r
+ * distributed under the License is distributed on an "AS IS" BASIS,\r
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\r
+ * See the License for the specific language governing permissions and\r
+ * limitations under the License.\r
+ *\r
+ ******************************************************************/\r
+\r
+/**\r
+ * @file\r
+ * This file implements APIs related to atomic operations compatible with GCC compilers.\r
+ */\r
+\r
+#include "ocatomic.h"\r
+\r
+int32_t oc_atomic_increment(volatile int32_t *addend)\r
+{\r
+    return __sync_add_and_fetch(addend, 1);\r
+}\r
+\r
+int32_t oc_atomic_decrement(volatile int32_t *addend)\r
+{\r
+    return __sync_sub_and_fetch(addend, 1);\r
+}\r
diff --git a/resource/c_common/ocatomic/src/windows/ocatomic.c b/resource/c_common/ocatomic/src/windows/ocatomic.c
new file mode 100644 (file)
index 0000000..3dea8ff
--- /dev/null
@@ -0,0 +1,36 @@
+/* *****************************************************************\r
+ *\r
+ * Copyright 2017 Microsoft\r
+ *\r
+ *\r
+ * Licensed under the Apache License, Version 2.0 (the "License");\r
+ * you may not use this file except in compliance with the License.\r
+ * You may obtain a copy of the License at\r
+ *\r
+ *      http://www.apache.org/licenses/LICENSE-2.0\r
+ *\r
+ * Unless required by applicable law or agreed to in writing, software\r
+ * distributed under the License is distributed on an "AS IS" BASIS,\r
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\r
+ * See the License for the specific language governing permissions and\r
+ * limitations under the License.\r
+ *\r
+ ******************************************************************/\r
+\r
+/**\r
+ * @file\r
+ * This file implements APIs related to atomic operations for Windows.\r
+ */\r
+\r
+#include "ocatomic.h"\r
+#include <windows.h>\r
+\r
+int32_t oc_atomic_increment(volatile int32_t *addend)\r
+{\r
+    return InterlockedIncrement((volatile long*)addend);\r
+}\r
+\r
+int32_t oc_atomic_decrement(volatile int32_t *addend)\r
+{\r
+    return InterlockedDecrement((volatile long*)addend);\r
+}\r
index d700e6c..254f463 100644 (file)
@@ -57,6 +57,7 @@ else:
 
 liboctbstack_env.PrependUnique(CPPPATH = [
         '#/extlibs/timer/',
+        '#resource/c_common/ocatomic/include',
         '#resource/csdk/logger/include',
         '#resource/csdk/include',
         'include',
index 54c1657..5f8e57a 100644 (file)
 #define __STDC_LIMIT_MACROS
 #endif
 #include "iotivity_config.h"
+#include "iotivity_debug.h"
 #include <stdlib.h>
 #include <inttypes.h>
 #include <string.h>
 #include <ctype.h>
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#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"
 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;
 }
index c7e5715..031af22 100644 (file)
@@ -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);