From 78a4b59e5564e743d39e01eda35032b973f0eb85 Mon Sep 17 00:00:00 2001 From: rdkdevpilot Date: Sun, 20 Sep 2026 22:18:31 +0530 Subject: [PATCH 1/4] Fix BAD_CHECK_OF_WAIT_COND in t2eventreceiver.c Defect ID: 736257e3-1ee9-4acb-a8b1-7c0b28c99b5c --- source/bulkdata/t2eventreceiver.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source/bulkdata/t2eventreceiver.c b/source/bulkdata/t2eventreceiver.c index 32a2d0199..426dfc2a6 100644 --- a/source/bulkdata/t2eventreceiver.c +++ b/source/bulkdata/t2eventreceiver.c @@ -268,6 +268,11 @@ void* T2ER_EventDispatchThread(void *arg) if(ret != 0) // pthread cond wait failed return after unlock { T2Error("%s pthread_cond_wait failed with error code: %d\n", __FUNCTION__, ret); + if(pthread_mutex_unlock(&erMutex) != 0) + { + T2Error("%s pthread_mutex_unlock for erMutex failed\n", __FUNCTION__); + } + return NULL; } T2Debug("Received signal from T2ER_Push\n"); // Release erMutex before acquiring sTDMutex to avoid lock order reversal and potential deadlock From 1419670fa24c00f0a3f7d00e308b74f9f7a311ed Mon Sep 17 00:00:00 2001 From: tabbas651 Date: Wed, 30 Sep 2026 12:08:50 -0400 Subject: [PATCH 2/4] testcases updated --- source/bulkdata/t2eventreceiver.c | 14 +++++++++++++- source/bulkdata/t2eventreceiver.h | 5 +++++ source/test/bulkdata/profileTest.cpp | 25 +++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/source/bulkdata/t2eventreceiver.c b/source/bulkdata/t2eventreceiver.c index 426dfc2a6..3acf277c8 100644 --- a/source/bulkdata/t2eventreceiver.c +++ b/source/bulkdata/t2eventreceiver.c @@ -42,6 +42,9 @@ static pthread_mutex_t erMutex; static pthread_cond_t erCond; static pthread_mutex_t sTDMutex; +// Seam so unit tests can inject a pthread_cond_wait failure; defaults to the real call. +int (*t2erCondWait)(pthread_cond_t *cond, pthread_mutex_t *mutex) = pthread_cond_wait; + T2ERROR ReportProfiles_storeMarkerEvent(char *profileName, T2Event *eventInfo); /** @@ -264,7 +267,7 @@ void* T2ER_EventDispatchThread(void *arg) while(t2_queue_count(eQueue) == 0 && shouldContinue) { T2Debug("Event Queue size is 0, Waiting events from T2ER_Push\n"); - int ret = pthread_cond_wait(&erCond, &erMutex); + int ret = t2erCondWait(&erCond, &erMutex); if(ret != 0) // pthread cond wait failed return after unlock { T2Error("%s pthread_cond_wait failed with error code: %d\n", __FUNCTION__, ret); @@ -272,6 +275,15 @@ void* T2ER_EventDispatchThread(void *arg) { T2Error("%s pthread_mutex_unlock for erMutex failed\n", __FUNCTION__); } + if(pthread_mutex_lock(&sTDMutex) == 0) + { + stopDispatchThread = true; + pthread_mutex_unlock(&sTDMutex); + } + else + { + T2Error("%s pthread_mutex_lock for sTDMutex failed\n", __FUNCTION__); + } return NULL; } T2Debug("Received signal from T2ER_Push\n"); diff --git a/source/bulkdata/t2eventreceiver.h b/source/bulkdata/t2eventreceiver.h index 172f34f19..057ff40b2 100644 --- a/source/bulkdata/t2eventreceiver.h +++ b/source/bulkdata/t2eventreceiver.h @@ -20,8 +20,13 @@ #ifndef _T2EVENTRECEIVER_H_ #define _T2EVENTRECEIVER_H_ +#include + #include "telemetry2_0.h" +// Seam so unit tests can inject a pthread_cond_wait failure; defaults to the real call. +extern int (*t2erCondWait)(pthread_cond_t *cond, pthread_mutex_t *mutex); + typedef struct _T2Event { char* name; diff --git a/source/test/bulkdata/profileTest.cpp b/source/test/bulkdata/profileTest.cpp index f76e377c1..95b6c18fe 100644 --- a/source/test/bulkdata/profileTest.cpp +++ b/source/test/bulkdata/profileTest.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -1393,6 +1394,30 @@ TEST_F(ProfileTest, EventDispatchThread_NoEventsWait) { } */ +static pthread_mutex_t *g_erCondWaitMutex = nullptr; + +static int erCondWaitAlwaysFails(pthread_cond_t *cond, pthread_mutex_t *mutex) +{ + (void) cond; + g_erCondWaitMutex = mutex; // erMutex is static in t2eventreceiver.c, capture it from the wait call + return EINVAL; +} + +TEST_F(ProfileTest, EventDispatchThread_CondWaitFailureReleasesErMutex) { + int (*originalCondWait)(pthread_cond_t *, pthread_mutex_t *) = t2erCondWait; + g_erCondWaitMutex = nullptr; + t2erCondWait = erCondWaitAlwaysFails; + + void *result = T2ER_EventDispatchThread(nullptr); + + t2erCondWait = originalCondWait; + + ASSERT_EQ(result, nullptr); + ASSERT_NE(g_erCondWaitMutex, nullptr); + ASSERT_EQ(pthread_mutex_trylock(g_erCondWaitMutex), 0) << "erMutex was still held when the dispatch thread exited"; + pthread_mutex_unlock(g_erCondWaitMutex); +} + /* TEST_F(ProfileTest, InitAlreadyInitialized) { EXPECT_CALL(*g_rbusMock, rbus_registerLogHandler(_)) From 6b7c36d03e658081e6767287ccdd20b1fa6f0341 Mon Sep 17 00:00:00 2001 From: tabbas651 Date: Wed, 30 Sep 2026 12:34:43 -0400 Subject: [PATCH 3/4] Addressed copilot review comments --- source/bulkdata/t2eventreceiver.c | 130 +++++++++++++++---------- source/test/bulkdata/profileTest.cpp | 25 ----- source/test/bulkdata/t2markersTest.cpp | 42 ++++++++ 3 files changed, 122 insertions(+), 75 deletions(-) diff --git a/source/bulkdata/t2eventreceiver.c b/source/bulkdata/t2eventreceiver.c index 3acf277c8..236542789 100644 --- a/source/bulkdata/t2eventreceiver.c +++ b/source/bulkdata/t2eventreceiver.c @@ -42,6 +42,12 @@ static pthread_mutex_t erMutex; static pthread_cond_t erCond; static pthread_mutex_t sTDMutex; +// erThread has been created and not yet joined or detached, so T2ER_Uninit() must still join it. +static bool erThreadJoinable = false; +// A detached worker may outlive T2ER_Uninit(), so the sync objects must not be destroyed under it. +static bool erThreadDetached = false; +static bool erSyncObjectsInitialized = false; + // Seam so unit tests can inject a pthread_cond_wait failure; defaults to the real call. int (*t2erCondWait)(pthread_cond_t *cond, pthread_mutex_t *mutex) = pthread_cond_wait; @@ -383,24 +389,32 @@ T2ERROR T2ER_Init() T2Error("Failed to create Event Receiver Queue\n"); return T2ERROR_FAILURE; } - int pthread_ret = 0; - pthread_ret = pthread_mutex_init(&sTDMutex, NULL); - if(pthread_ret != 0) - { - T2Error("%s Mutex init for sTDMutex failed with error code: %d\n", __FUNCTION__, pthread_ret); - return T2ERROR_FAILURE; - } - pthread_ret = pthread_mutex_init(&erMutex, NULL); - if(pthread_ret != 0) + // Re-initializing a live mutex or condition variable is undefined, so only initialize once. + if(!erSyncObjectsInitialized) { - T2Error("%s Mutex init for erMutex failed with error code: %d\n", __FUNCTION__, pthread_ret); - return T2ERROR_FAILURE; - } - pthread_ret = pthread_cond_init(&erCond, NULL); - if(pthread_ret != 0) - { - T2Error("%s Mutex init for erMutex failed with error code: %d\n", __FUNCTION__, pthread_ret); - return T2ERROR_FAILURE; + int pthread_ret = 0; + pthread_ret = pthread_mutex_init(&sTDMutex, NULL); + if(pthread_ret != 0) + { + T2Error("%s Mutex init for sTDMutex failed with error code: %d\n", __FUNCTION__, pthread_ret); + return T2ERROR_FAILURE; + } + pthread_ret = pthread_mutex_init(&erMutex, NULL); + if(pthread_ret != 0) + { + pthread_mutex_destroy(&sTDMutex); + T2Error("%s Mutex init for erMutex failed with error code: %d\n", __FUNCTION__, pthread_ret); + return T2ERROR_FAILURE; + } + pthread_ret = pthread_cond_init(&erCond, NULL); + if(pthread_ret != 0) + { + pthread_mutex_destroy(&erMutex); + pthread_mutex_destroy(&sTDMutex); + T2Error("%s Mutex init for erMutex failed with error code: %d\n", __FUNCTION__, pthread_ret); + return T2ERROR_FAILURE; + } + erSyncObjectsInitialized = true; } EREnabled = true; @@ -446,9 +460,14 @@ T2ERROR T2ER_StartDispatchThread() return T2ERROR_FAILURE; } stopDispatchThread = false; - if(pthread_create(&erThread, NULL, T2ER_EventDispatchThread, NULL) != 0) // pthread_create failed so return after unlock as already stopDispatchThread is locked. + if(pthread_create(&erThread, NULL, T2ER_EventDispatchThread, NULL) != 0) { T2Error("%s T2ER_EventDispatchThread creation failed\n", __FUNCTION__); + stopDispatchThread = true; // no worker exists, keep the flag consistent so a retry is possible + } + else + { + erThreadJoinable = true; } if(pthread_mutex_unlock(&sTDMutex) != 0) @@ -554,6 +573,15 @@ T2ERROR T2ER_StopDispatchThread() return T2ERROR_FAILURE; } ret = pthread_detach(erThread); + if(ret != 0) + { + T2Error("%s pthread_detach for erThread failed with error code %d\n", __FUNCTION__, ret); + } + else + { + erThreadJoinable = false; + erThreadDetached = true; + } pthread_mutex_unlock(&sTDMutex); flushCacheFromFile(); @@ -572,66 +600,68 @@ void T2ER_Uninit() EREnabled = false; pthread_t threadToJoin; + bool joinThread = false; + bool destroySyncObjects = false; + if(pthread_mutex_lock(&sTDMutex) != 0) // mutex lock failed so return from T2ER_Uninit { T2Error("%s pthread_mutex_lock for sTDMutex failed\n", __FUNCTION__); return; } - if(!stopDispatchThread) + stopDispatchThread = true; + joinThread = erThreadJoinable; // true even if the worker already exited on its own, it still needs reaping + threadToJoin = erThread; + erThreadJoinable = false; + destroySyncObjects = erSyncObjectsInitialized && !erThreadDetached; + if(pthread_mutex_unlock(&sTDMutex) != 0) //mutex unlock failed so return from T2ER_Uninit { - stopDispatchThread = true; - threadToJoin = erThread; // Save thread handle while holding lock - if(pthread_mutex_unlock(&sTDMutex) != 0) //mutex unlock failed so return from T2ER_Uninit - { - T2Error("%s pthread_mutex_unlock for sTDMutex failed\n", __FUNCTION__); - return; - } + T2Error("%s pthread_mutex_unlock for sTDMutex failed\n", __FUNCTION__); + return; + } - if(pthread_mutex_lock(&erMutex) != 0) //mutex lock failed so return from T2ER_Uninit - { - T2Error("%s pthread_mutex_lock for erMutex failed\n", __FUNCTION__); - return; - } - int ret = pthread_cond_signal(&erCond); - if(ret != 0) + if(joinThread) + { + if(pthread_mutex_lock(&erMutex) == 0) { - T2Error("%s pthread_cond_signal for erCond failed with error code %d\n", __FUNCTION__, ret); + int ret = pthread_cond_signal(&erCond); + if(ret != 0) + { + T2Error("%s pthread_cond_signal for erCond failed with error code %d\n", __FUNCTION__, ret); + } + if(pthread_mutex_unlock(&erMutex) != 0) + { + T2Error("%s pthread_mutex_unlock for erMutex failed\n", __FUNCTION__); + } } - if(pthread_mutex_unlock(&erMutex) != 0) + else { - T2Error("%s pthread_mutex_unlock for erMutex failed\n", __FUNCTION__); - return; + T2Error("%s pthread_mutex_lock for erMutex failed\n", __FUNCTION__); } if(pthread_join(threadToJoin, NULL) != 0) { T2Error("%s erThread join failed\n", __FUNCTION__); + destroySyncObjects = false; // worker may still be using them } + } + if(destroySyncObjects) + { if(pthread_mutex_destroy(&erMutex) != 0) { T2Error("%s pthread_mutex_destroy for erMutex failed\n", __FUNCTION__); - return; - } - if(pthread_mutex_destroy(&sTDMutex) != 0) - { - T2Error("%s pthread_mutex_destroy for sTDMutex failed\n", __FUNCTION__); - return; } if(pthread_cond_destroy(&erCond) != 0) { T2Error("%s pthread_cond_destroy for erCond failed\n", __FUNCTION__); - return; } - } - else - { - if(pthread_mutex_unlock(&sTDMutex) != 0) + if(pthread_mutex_destroy(&sTDMutex) != 0) { - T2Error("%s pthread_mutex_unlock for sTDMutex failed\n", __FUNCTION__); - return; + T2Error("%s pthread_mutex_destroy for sTDMutex failed\n", __FUNCTION__); } + erSyncObjectsInitialized = false; } + T2Debug("T2ER Event Dispatch Thread successfully terminated\n"); t2_queue_destroy(eQueue, freeT2Event); eQueue = NULL; diff --git a/source/test/bulkdata/profileTest.cpp b/source/test/bulkdata/profileTest.cpp index 95b6c18fe..f76e377c1 100644 --- a/source/test/bulkdata/profileTest.cpp +++ b/source/test/bulkdata/profileTest.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include @@ -1394,30 +1393,6 @@ TEST_F(ProfileTest, EventDispatchThread_NoEventsWait) { } */ -static pthread_mutex_t *g_erCondWaitMutex = nullptr; - -static int erCondWaitAlwaysFails(pthread_cond_t *cond, pthread_mutex_t *mutex) -{ - (void) cond; - g_erCondWaitMutex = mutex; // erMutex is static in t2eventreceiver.c, capture it from the wait call - return EINVAL; -} - -TEST_F(ProfileTest, EventDispatchThread_CondWaitFailureReleasesErMutex) { - int (*originalCondWait)(pthread_cond_t *, pthread_mutex_t *) = t2erCondWait; - g_erCondWaitMutex = nullptr; - t2erCondWait = erCondWaitAlwaysFails; - - void *result = T2ER_EventDispatchThread(nullptr); - - t2erCondWait = originalCondWait; - - ASSERT_EQ(result, nullptr); - ASSERT_NE(g_erCondWaitMutex, nullptr); - ASSERT_EQ(pthread_mutex_trylock(g_erCondWaitMutex), 0) << "erMutex was still held when the dispatch thread exited"; - pthread_mutex_unlock(g_erCondWaitMutex); -} - /* TEST_F(ProfileTest, InitAlreadyInitialized) { EXPECT_CALL(*g_rbusMock, rbus_registerLogHandler(_)) diff --git a/source/test/bulkdata/t2markersTest.cpp b/source/test/bulkdata/t2markersTest.cpp index 4a056304e..136a92657 100644 --- a/source/test/bulkdata/t2markersTest.cpp +++ b/source/test/bulkdata/t2markersTest.cpp @@ -20,6 +20,8 @@ #include #include #include +#include +#include #include #include @@ -311,3 +313,43 @@ TEST_F(t2markersTestFixture, T2ER_Uninit_after_stop_dispatch_thread) T2ER_Uninit(); } +static pthread_mutex_t *g_erCondWaitMutex = nullptr; + +static int erCondWaitAlwaysFails(pthread_cond_t *cond, pthread_mutex_t *mutex) +{ + (void) cond; + g_erCondWaitMutex = mutex; // erMutex is static in t2eventreceiver.c, capture it from the wait call + return EINVAL; +} + +//Dispatch thread must release erMutex when pthread_cond_wait fails - CID 52591 +TEST_F(t2markersTestFixture, T2ER_EventDispatchThread_cond_wait_failure_releases_erMutex) +{ + EXPECT_CALL(*g_t2markersMock, isRbusEnabled()) + .Times(::testing::AnyNumber()) + .WillRepeatedly(Return(true)); + EXPECT_CALL(*g_t2markersMock, registerForTelemetryEvents(_)) + .Times(::testing::AnyNumber()) + .WillRepeatedly(Return(T2ERROR_SUCCESS)); + // T2ER_Init() is what runs pthread_mutex_init/pthread_cond_init on erMutex, sTDMutex and erCond + ASSERT_EQ(T2ERROR_SUCCESS, T2ER_Init()); + + int (*originalCondWait)(pthread_cond_t *, pthread_mutex_t *) = t2erCondWait; + g_erCondWaitMutex = nullptr; + t2erCondWait = erCondWaitAlwaysFails; + + void *result = T2ER_EventDispatchThread(nullptr); + + t2erCondWait = originalCondWait; + + ASSERT_EQ(result, nullptr); + ASSERT_NE(g_erCondWaitMutex, nullptr); + ASSERT_EQ(0, pthread_mutex_trylock(g_erCondWaitMutex)) << "erMutex was still held when the dispatch thread exited"; + pthread_mutex_unlock(g_erCondWaitMutex); + + // stopDispatchThread must have been reset, so the dispatcher is restartable after the failure + EXPECT_EQ(T2ERROR_SUCCESS, T2ER_StartDispatchThread()); + EXPECT_EQ(T2ERROR_SUCCESS, T2ER_StopDispatchThread()); + T2ER_Uninit(); +} + From bda961b2e9e33f03ad929637542ceb24e90d23de Mon Sep 17 00:00:00 2001 From: tabbas651 Date: Wed, 30 Sep 2026 12:58:09 -0400 Subject: [PATCH 4/4] Chnages are make it minimalize --- source/bulkdata/t2eventreceiver.c | 144 +++++++++---------------- source/bulkdata/t2eventreceiver.h | 5 - source/test/bulkdata/t2markersTest.cpp | 42 -------- 3 files changed, 51 insertions(+), 140 deletions(-) diff --git a/source/bulkdata/t2eventreceiver.c b/source/bulkdata/t2eventreceiver.c index 236542789..426dfc2a6 100644 --- a/source/bulkdata/t2eventreceiver.c +++ b/source/bulkdata/t2eventreceiver.c @@ -42,15 +42,6 @@ static pthread_mutex_t erMutex; static pthread_cond_t erCond; static pthread_mutex_t sTDMutex; -// erThread has been created and not yet joined or detached, so T2ER_Uninit() must still join it. -static bool erThreadJoinable = false; -// A detached worker may outlive T2ER_Uninit(), so the sync objects must not be destroyed under it. -static bool erThreadDetached = false; -static bool erSyncObjectsInitialized = false; - -// Seam so unit tests can inject a pthread_cond_wait failure; defaults to the real call. -int (*t2erCondWait)(pthread_cond_t *cond, pthread_mutex_t *mutex) = pthread_cond_wait; - T2ERROR ReportProfiles_storeMarkerEvent(char *profileName, T2Event *eventInfo); /** @@ -273,7 +264,7 @@ void* T2ER_EventDispatchThread(void *arg) while(t2_queue_count(eQueue) == 0 && shouldContinue) { T2Debug("Event Queue size is 0, Waiting events from T2ER_Push\n"); - int ret = t2erCondWait(&erCond, &erMutex); + int ret = pthread_cond_wait(&erCond, &erMutex); if(ret != 0) // pthread cond wait failed return after unlock { T2Error("%s pthread_cond_wait failed with error code: %d\n", __FUNCTION__, ret); @@ -281,15 +272,6 @@ void* T2ER_EventDispatchThread(void *arg) { T2Error("%s pthread_mutex_unlock for erMutex failed\n", __FUNCTION__); } - if(pthread_mutex_lock(&sTDMutex) == 0) - { - stopDispatchThread = true; - pthread_mutex_unlock(&sTDMutex); - } - else - { - T2Error("%s pthread_mutex_lock for sTDMutex failed\n", __FUNCTION__); - } return NULL; } T2Debug("Received signal from T2ER_Push\n"); @@ -389,32 +371,24 @@ T2ERROR T2ER_Init() T2Error("Failed to create Event Receiver Queue\n"); return T2ERROR_FAILURE; } - // Re-initializing a live mutex or condition variable is undefined, so only initialize once. - if(!erSyncObjectsInitialized) + int pthread_ret = 0; + pthread_ret = pthread_mutex_init(&sTDMutex, NULL); + if(pthread_ret != 0) { - int pthread_ret = 0; - pthread_ret = pthread_mutex_init(&sTDMutex, NULL); - if(pthread_ret != 0) - { - T2Error("%s Mutex init for sTDMutex failed with error code: %d\n", __FUNCTION__, pthread_ret); - return T2ERROR_FAILURE; - } - pthread_ret = pthread_mutex_init(&erMutex, NULL); - if(pthread_ret != 0) - { - pthread_mutex_destroy(&sTDMutex); - T2Error("%s Mutex init for erMutex failed with error code: %d\n", __FUNCTION__, pthread_ret); - return T2ERROR_FAILURE; - } - pthread_ret = pthread_cond_init(&erCond, NULL); - if(pthread_ret != 0) - { - pthread_mutex_destroy(&erMutex); - pthread_mutex_destroy(&sTDMutex); - T2Error("%s Mutex init for erMutex failed with error code: %d\n", __FUNCTION__, pthread_ret); - return T2ERROR_FAILURE; - } - erSyncObjectsInitialized = true; + T2Error("%s Mutex init for sTDMutex failed with error code: %d\n", __FUNCTION__, pthread_ret); + return T2ERROR_FAILURE; + } + pthread_ret = pthread_mutex_init(&erMutex, NULL); + if(pthread_ret != 0) + { + T2Error("%s Mutex init for erMutex failed with error code: %d\n", __FUNCTION__, pthread_ret); + return T2ERROR_FAILURE; + } + pthread_ret = pthread_cond_init(&erCond, NULL); + if(pthread_ret != 0) + { + T2Error("%s Mutex init for erMutex failed with error code: %d\n", __FUNCTION__, pthread_ret); + return T2ERROR_FAILURE; } EREnabled = true; @@ -460,14 +434,9 @@ T2ERROR T2ER_StartDispatchThread() return T2ERROR_FAILURE; } stopDispatchThread = false; - if(pthread_create(&erThread, NULL, T2ER_EventDispatchThread, NULL) != 0) + if(pthread_create(&erThread, NULL, T2ER_EventDispatchThread, NULL) != 0) // pthread_create failed so return after unlock as already stopDispatchThread is locked. { T2Error("%s T2ER_EventDispatchThread creation failed\n", __FUNCTION__); - stopDispatchThread = true; // no worker exists, keep the flag consistent so a retry is possible - } - else - { - erThreadJoinable = true; } if(pthread_mutex_unlock(&sTDMutex) != 0) @@ -573,15 +542,6 @@ T2ERROR T2ER_StopDispatchThread() return T2ERROR_FAILURE; } ret = pthread_detach(erThread); - if(ret != 0) - { - T2Error("%s pthread_detach for erThread failed with error code %d\n", __FUNCTION__, ret); - } - else - { - erThreadJoinable = false; - erThreadDetached = true; - } pthread_mutex_unlock(&sTDMutex); flushCacheFromFile(); @@ -600,68 +560,66 @@ void T2ER_Uninit() EREnabled = false; pthread_t threadToJoin; - bool joinThread = false; - bool destroySyncObjects = false; - if(pthread_mutex_lock(&sTDMutex) != 0) // mutex lock failed so return from T2ER_Uninit { T2Error("%s pthread_mutex_lock for sTDMutex failed\n", __FUNCTION__); return; } - stopDispatchThread = true; - joinThread = erThreadJoinable; // true even if the worker already exited on its own, it still needs reaping - threadToJoin = erThread; - erThreadJoinable = false; - destroySyncObjects = erSyncObjectsInitialized && !erThreadDetached; - if(pthread_mutex_unlock(&sTDMutex) != 0) //mutex unlock failed so return from T2ER_Uninit + if(!stopDispatchThread) { - T2Error("%s pthread_mutex_unlock for sTDMutex failed\n", __FUNCTION__); - return; - } - - if(joinThread) - { - if(pthread_mutex_lock(&erMutex) == 0) + stopDispatchThread = true; + threadToJoin = erThread; // Save thread handle while holding lock + if(pthread_mutex_unlock(&sTDMutex) != 0) //mutex unlock failed so return from T2ER_Uninit { - int ret = pthread_cond_signal(&erCond); - if(ret != 0) - { - T2Error("%s pthread_cond_signal for erCond failed with error code %d\n", __FUNCTION__, ret); - } - if(pthread_mutex_unlock(&erMutex) != 0) - { - T2Error("%s pthread_mutex_unlock for erMutex failed\n", __FUNCTION__); - } + T2Error("%s pthread_mutex_unlock for sTDMutex failed\n", __FUNCTION__); + return; } - else + + if(pthread_mutex_lock(&erMutex) != 0) //mutex lock failed so return from T2ER_Uninit { T2Error("%s pthread_mutex_lock for erMutex failed\n", __FUNCTION__); + return; + } + int ret = pthread_cond_signal(&erCond); + if(ret != 0) + { + T2Error("%s pthread_cond_signal for erCond failed with error code %d\n", __FUNCTION__, ret); + } + if(pthread_mutex_unlock(&erMutex) != 0) + { + T2Error("%s pthread_mutex_unlock for erMutex failed\n", __FUNCTION__); + return; } if(pthread_join(threadToJoin, NULL) != 0) { T2Error("%s erThread join failed\n", __FUNCTION__); - destroySyncObjects = false; // worker may still be using them } - } - if(destroySyncObjects) - { if(pthread_mutex_destroy(&erMutex) != 0) { T2Error("%s pthread_mutex_destroy for erMutex failed\n", __FUNCTION__); + return; + } + if(pthread_mutex_destroy(&sTDMutex) != 0) + { + T2Error("%s pthread_mutex_destroy for sTDMutex failed\n", __FUNCTION__); + return; } if(pthread_cond_destroy(&erCond) != 0) { T2Error("%s pthread_cond_destroy for erCond failed\n", __FUNCTION__); + return; } - if(pthread_mutex_destroy(&sTDMutex) != 0) + } + else + { + if(pthread_mutex_unlock(&sTDMutex) != 0) { - T2Error("%s pthread_mutex_destroy for sTDMutex failed\n", __FUNCTION__); + T2Error("%s pthread_mutex_unlock for sTDMutex failed\n", __FUNCTION__); + return; } - erSyncObjectsInitialized = false; } - T2Debug("T2ER Event Dispatch Thread successfully terminated\n"); t2_queue_destroy(eQueue, freeT2Event); eQueue = NULL; diff --git a/source/bulkdata/t2eventreceiver.h b/source/bulkdata/t2eventreceiver.h index 057ff40b2..172f34f19 100644 --- a/source/bulkdata/t2eventreceiver.h +++ b/source/bulkdata/t2eventreceiver.h @@ -20,13 +20,8 @@ #ifndef _T2EVENTRECEIVER_H_ #define _T2EVENTRECEIVER_H_ -#include - #include "telemetry2_0.h" -// Seam so unit tests can inject a pthread_cond_wait failure; defaults to the real call. -extern int (*t2erCondWait)(pthread_cond_t *cond, pthread_mutex_t *mutex); - typedef struct _T2Event { char* name; diff --git a/source/test/bulkdata/t2markersTest.cpp b/source/test/bulkdata/t2markersTest.cpp index 136a92657..4a056304e 100644 --- a/source/test/bulkdata/t2markersTest.cpp +++ b/source/test/bulkdata/t2markersTest.cpp @@ -20,8 +20,6 @@ #include #include #include -#include -#include #include #include @@ -313,43 +311,3 @@ TEST_F(t2markersTestFixture, T2ER_Uninit_after_stop_dispatch_thread) T2ER_Uninit(); } -static pthread_mutex_t *g_erCondWaitMutex = nullptr; - -static int erCondWaitAlwaysFails(pthread_cond_t *cond, pthread_mutex_t *mutex) -{ - (void) cond; - g_erCondWaitMutex = mutex; // erMutex is static in t2eventreceiver.c, capture it from the wait call - return EINVAL; -} - -//Dispatch thread must release erMutex when pthread_cond_wait fails - CID 52591 -TEST_F(t2markersTestFixture, T2ER_EventDispatchThread_cond_wait_failure_releases_erMutex) -{ - EXPECT_CALL(*g_t2markersMock, isRbusEnabled()) - .Times(::testing::AnyNumber()) - .WillRepeatedly(Return(true)); - EXPECT_CALL(*g_t2markersMock, registerForTelemetryEvents(_)) - .Times(::testing::AnyNumber()) - .WillRepeatedly(Return(T2ERROR_SUCCESS)); - // T2ER_Init() is what runs pthread_mutex_init/pthread_cond_init on erMutex, sTDMutex and erCond - ASSERT_EQ(T2ERROR_SUCCESS, T2ER_Init()); - - int (*originalCondWait)(pthread_cond_t *, pthread_mutex_t *) = t2erCondWait; - g_erCondWaitMutex = nullptr; - t2erCondWait = erCondWaitAlwaysFails; - - void *result = T2ER_EventDispatchThread(nullptr); - - t2erCondWait = originalCondWait; - - ASSERT_EQ(result, nullptr); - ASSERT_NE(g_erCondWaitMutex, nullptr); - ASSERT_EQ(0, pthread_mutex_trylock(g_erCondWaitMutex)) << "erMutex was still held when the dispatch thread exited"; - pthread_mutex_unlock(g_erCondWaitMutex); - - // stopDispatchThread must have been reset, so the dispatcher is restartable after the failure - EXPECT_EQ(T2ERROR_SUCCESS, T2ER_StartDispatchThread()); - EXPECT_EQ(T2ERROR_SUCCESS, T2ER_StopDispatchThread()); - T2ER_Uninit(); -} -