Fix to prevent of crash on the unit test.

As-Is, Notification unit test sometimes crash on running.

Cause by,
1) When provider unit test running,
it use invalid request information.
this invalid request is not made by stack,
made by inside unit test as uninitialized variables.

2) When consumer was destroyed, task queue was deinitilzed.
but main task thread waitting for mutex unlocking,
when mutex unlock, main thread does not update address of queue.
actually, address of queue is modified,
in this moment, main thread try to reference unmodified queue and crash.

Fixed,
1) Uninitialized variables are initialized.
2) When main thread mutex was unlocked, updating queue.
   Exception handling what data of queue is invalid.

Change-Id: I41cd4a100ea2bb7b3e68be3017475c9ecbfbb144
Signed-off-by: KIM JungYong <jyong2.kim@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/18047
Tested-by: jenkins-iotivity <jenkins@iotivity.org>
Reviewed-by: Uze Choi <uzchoi@samsung.com>
Tested-by: Uze Choi <uzchoi@samsung.com>

diff --git a/service/notification/SConscript b/service/notification/SConscript
index 62d0b05..d6d0fdf 100755 (executable)
--- a/service/notification/SConscript
+++ b/service/notification/SConscript
@@ -146,8 +146,8 @@ notification_consumer_env.UserInstallTargetHeader('include/NSCommon.h',\
 
 
 # Go to build Unit test
-#if target_os == 'linux':
-#    SConscript('unittest/SConscript')
+if target_os in ['linux']:
+    SConscript('unittest/SConscript')
 
 # Go to build c++ wrapper
 SConscript('cpp-wrapper/SConscript')

diff --git a/service/notification/src/consumer/NSConsumerScheduler.c b/service/notification/src/consumer/NSConsumerScheduler.c
index b5fd6b9..c299b31 100644 (file)
--- a/service/notification/src/consumer/NSConsumerScheduler.c
+++ b/service/notification/src/consumer/NSConsumerScheduler.c
@@ -170,7 +170,7 @@ void * NSConsumerMsgHandleThreadFunc(void * threadHandle)
             break;
         }
 
-        NSConsumerQueue * queue = *(NSGetMsgHandleQueue());;
+        NSConsumerQueue * queue = *(NSGetMsgHandleQueue());
         if (!queue)
         {
             usleep(2000);
@@ -192,6 +192,7 @@ void * NSConsumerMsgHandleThreadFunc(void * threadHandle)
 
         NSThreadLock(queueHandleThread);
         NS_LOG(DEBUG, "msg handler working");
+        queue = *(NSGetMsgHandleQueue());
         obj = NSPopQueue(queue);
 
         if (obj)
@@ -271,6 +272,7 @@ void NSProviderDeletedPostClean(
 
 void NSConsumerTaskProcessing(NSTask * task)
 {
+    NS_VERIFY_NOT_NULL_V(task);
     switch (task->taskType)
     {
         case TASK_EVENT_CONNECTED:

diff --git a/service/notification/unittest/NSConsumerTest2.cpp b/service/notification/unittest/NSConsumerTest2.cpp
index eb4886a..a172ef2 100644 (file)
--- a/service/notification/unittest/NSConsumerTest2.cpp
+++ b/service/notification/unittest/NSConsumerTest2.cpp
@@ -239,8 +239,8 @@ namespace
         EXPECT_NE((void *)NULL, payload);
         bool getResult = OCRepPayloadSetPropString(payload,
                         NS_ATTRIBUTE_CONSUMER_ID, testProviderID.c_str());
-        getResult &= OCRepPayloadSetPropObjectArray(payload,
-                        NS_ATTRIBUTE_TOPIC_LIST, (const OCRepPayload **)topicList, dimensions);
+        getResult &= OCRepPayloadSetPropObjectArrayAsOwner(payload,
+                        NS_ATTRIBUTE_TOPIC_LIST, topicList, dimensions);
         if (getResult == false)
         {
             OCRepPayloadDestroy(payload);
@@ -250,7 +250,7 @@ namespace
         return payload;
     }
 
-    OCClientResponse * getResponse()
+    OCClientResponse * createResponse()
     {
         OCClientResponse * testResponse =
                 (OCClientResponse *)malloc(sizeof(OCClientResponse));
@@ -355,6 +355,10 @@ namespace
             NSRemoveProvider(g_provider);
             g_provider = NULL;
         }
+    }
+
+    void removeGlobalResponse()
+    {
         if (g_testResponse)
         {
             free((void*)(g_testResponse->resourceUri));
@@ -424,7 +428,7 @@ TEST(NotificationConsumerTest, ExpectCallbackDiscovered)
     revState = NS_STOPPED;
     expectedState = NS_DISCOVERED;
 
-    g_testResponse = getResponse();
+    g_testResponse = createResponse();
     g_testResponse->payload = (OCPayload *)getDiscoverPayload();
     NSIntrospectProvider(NULL, NULL, g_testResponse);
 
@@ -433,6 +437,7 @@ TEST(NotificationConsumerTest, ExpectCallbackDiscovered)
 
     OCRepPayloadDestroy((OCRepPayload *)g_testResponse->payload);
     g_testResponse->payload = NULL;
+    removeGlobalResponse();
 
     EXPECT_EQ(NS_DISCOVERED, revState);
 }
@@ -442,6 +447,7 @@ TEST(NotificationConsumerTest, ExpectCallbackAllow)
     revState = NS_STOPPED;
     expectedState = NS_ALLOW;
 
+    g_testResponse = createResponse();
     g_testResponse->payload = (OCPayload *)getMsgPayload(NS_ALLOW);
     NSConsumerMessageListener(NULL,NULL, g_testResponse);
 
@@ -450,6 +456,7 @@ TEST(NotificationConsumerTest, ExpectCallbackAllow)
 
     OCRepPayloadDestroy((OCRepPayload *)g_testResponse->payload);
     g_testResponse->payload = NULL;
+    removeGlobalResponse();
 
     EXPECT_EQ(NS_ALLOW, revState);
 }
@@ -458,6 +465,7 @@ TEST(NotificationConsumerTest, ExpectReceiveNotification)
 {
     uint64_t id = 100;
 
+    g_testResponse = createResponse();
     g_testResponse->payload = (OCPayload *)getMsgPayload(id);
     NSConsumerMessageListener(NULL,NULL, g_testResponse);
 
@@ -466,6 +474,7 @@ TEST(NotificationConsumerTest, ExpectReceiveNotification)
 
     OCRepPayloadDestroy((OCRepPayload *)g_testResponse->payload);
     g_testResponse->payload = NULL;
+    removeGlobalResponse();
 
     EXPECT_EQ(id, revId);
 }
@@ -475,6 +484,7 @@ TEST(NotificationConsumerTest, ExpectReceiveSyncInfo)
     uint64_t id = 100;
     type = NS_SYNC_DELETED;
 
+    g_testResponse = createResponse();
     g_testResponse->payload = (OCPayload *)getSyncPayload(id, NS_SYNC_READ);
     NSConsumerSyncInfoListener(NULL,NULL, g_testResponse);
 
@@ -483,6 +493,7 @@ TEST(NotificationConsumerTest, ExpectReceiveSyncInfo)
 
     OCRepPayloadDestroy((OCRepPayload *)g_testResponse->payload);
     g_testResponse->payload = NULL;
+    removeGlobalResponse();
 
     EXPECT_EQ(NS_SYNC_READ, type);
 }
@@ -525,6 +536,7 @@ TEST(NotificationConsumerTest, ExpectCallbackTopicUpdated)
 {
     revState = NS_STOPPED;
 
+    g_testResponse = createResponse();
     g_testResponse->payload = (OCPayload *)getTopicPayload();
     g_provider_internal = getProvider(testAddr);
     NSIntrospectTopic((void *)g_provider_internal, NULL, g_testResponse);
@@ -537,6 +549,7 @@ TEST(NotificationConsumerTest, ExpectCallbackTopicUpdated)
 
     OCRepPayloadDestroy((OCRepPayload *)g_testResponse->payload);
     g_testResponse->payload = NULL;
+    removeGlobalResponse();
 
     expectedState = NS_STOPPED;
 }
@@ -552,7 +565,7 @@ TEST(NotificationConsumerTest, ExpectEQTopicList)
     topics.push_back("3");
 
     NSTopicLL * retTopic = NSConsumerGetTopicList(g_provider_internal->providerId);
-    EXPECT_NE((void *)NULL, retTopic);
+    EXPECT_NE((void *)NULL, (void *)retTopic);
 
     NSTopicLL * iter = retTopic;
     std::for_each (topics.begin(), topics.end(),
@@ -598,6 +611,7 @@ TEST(NotificationConsumerTest, ExpectUnsubscribeSuccess)
 
 TEST(NotificationConsumerTest, ExpectUnsubscribeWithPresenceStart)
 {
+    g_testResponse = createResponse();
     OCPresencePayload * payload = OCPresencePayloadCreate(1, 2, OC_PRESENCE_TRIGGER_CREATE, NULL);
     EXPECT_NE((void *)NULL, payload);
     g_testResponse->payload = (OCPayload *)payload;
@@ -606,10 +620,12 @@ TEST(NotificationConsumerTest, ExpectUnsubscribeWithPresenceStart)
 
     EXPECT_EQ(OC_STACK_KEEP_TRANSACTION, ret);
     OCPresencePayloadDestroy(payload);
+    removeGlobalResponse();
 }
 
 TEST(NotificationConsumerTest, ExpectUnsubscribeWithPresenceStop)
 {
+    g_testResponse = createResponse();
     OCPresencePayload * payload = OCPresencePayloadCreate(2, 2, OC_PRESENCE_TRIGGER_DELETE, NULL);
     EXPECT_NE((void *)NULL, payload);
     g_testResponse->payload = (OCPayload *)payload;

diff --git a/service/notification/unittest/NSProviderTest2.cpp b/service/notification/unittest/NSProviderTest2.cpp
index 25b9cae..98203ad 100644 (file)
--- a/service/notification/unittest/NSProviderTest2.cpp
+++ b/service/notification/unittest/NSProviderTest2.cpp
@@ -91,6 +91,8 @@ namespace
         OCEntityHandlerRequest * request =
                 (OCEntityHandlerRequest *)malloc(sizeof(OCEntityHandlerRequest));
         EXPECT_NE((void *)NULL, request);
+        request->requestHandle = NULL;
+        request->resource = NULL;
 
         if (OC_REST_OBSERVE == method)
         {
@@ -99,11 +101,13 @@ namespace
 
             std::string query = std::string(NS_QUERY_CONSUMER_ID)
                     + "=" + testConsumerId;
-            request->query = (char *)malloc(sizeof(char) * query.size() + 1);
+            request->query = (char *)malloc(query.size() + 1);
+            EXPECT_NE((void *)NULL, request->query);
             strncpy(request->query, query.c_str(), query.size() + 1);
         }
         request->method = method;
         request->numRcvdVendorSpecificHeaderOptions = 0;
+        request->payload = NULL;
 
         return request;
     }
@@ -113,6 +117,8 @@ namespace
         OCEntityHandlerRequest * request =
                 (OCEntityHandlerRequest *)malloc(sizeof(OCEntityHandlerRequest));
         EXPECT_NE((void *)NULL, request);
+        request->requestHandle = NULL;
+        request->resource = NULL;
 
         request->method = OC_REST_POST;
         request->numRcvdVendorSpecificHeaderOptions = 0;
@@ -208,6 +214,10 @@ TEST(NotificationProviderTest, ExpectCallbackSubscribeRequestWithAccepterProvide
 
     OCEntityHandlerRequest * syncRequest = getEntityRequest(OC_REST_OBSERVE, OC_OBSERVE_REGISTER);
     NSEntityHandlerSyncCb(flag, syncRequest, NULL);
+    {
+        std::unique_lock< std::mutex > lock{ responseProviderSubLock };
+        responseProviderSub.wait_for(lock, g_waitForResponse);
+    }
 
     free(syncRequest->query);
     free(syncRequest);
@@ -506,12 +516,20 @@ TEST(NotificationProviderTest, ExpectSuccessUnsub)
     OCEntityHandlerRequest * msgRequest =
             getEntityRequest(OC_REST_OBSERVE, OC_OBSERVE_DEREGISTER);
     NSEntityHandlerMessageCb(flag, msgRequest, NULL);
+    {
+        std::unique_lock< std::mutex > lock{ responseProviderSubLock };
+        responseProviderSub.wait_for(lock, g_waitForResponse);
+    }
     free(msgRequest->query);
     free(msgRequest);
 
     OCEntityHandlerRequest * syncRequest =
             getEntityRequest(OC_REST_OBSERVE, OC_OBSERVE_DEREGISTER);
     NSEntityHandlerSyncCb(flag, syncRequest, NULL);
+    {
+        std::unique_lock< std::mutex > lock{ responseProviderSubLock };
+        responseProviderSub.wait_for(lock, g_waitForResponse);
+    }
     free(syncRequest->query);
     free(syncRequest);