From 82f3c35881ab1a32c61c26f5ef781e57e7b3cab8 Mon Sep 17 00:00:00 2001 From: KIM JungYong Date: Tue, 21 Mar 2017 19:00:43 +0900 Subject: [PATCH] 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 Reviewed-on: https://gerrit.iotivity.org/gerrit/18047 Tested-by: jenkins-iotivity Reviewed-by: Uze Choi Tested-by: Uze Choi --- service/notification/SConscript | 4 ++-- .../src/consumer/NSConsumerScheduler.c | 4 +++- service/notification/unittest/NSConsumerTest2.cpp | 26 +++++++++++++++++----- service/notification/unittest/NSProviderTest2.cpp | 20 ++++++++++++++++- 4 files changed, 45 insertions(+), 9 deletions(-) diff --git a/service/notification/SConscript b/service/notification/SConscript index 62d0b05..d6d0fdf 100755 --- 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 --- 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 --- 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 --- 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); -- 2.7.4