From eb9bbda71012035a9489cb80e9b814121873d62a Mon Sep 17 00:00:00 2001 From: Mushegh Malkhasyan Date: Wed, 1 May 2024 19:54:13 -0700 Subject: [PATCH 1/4] Multiple updates include: * Adding support for Visual Studio Test Explorer via Google Test Adapter so the tests can be run/debugged in VS * Fixing multiple warnings - some have major impact on Windows platform/compiler which doesn't automatically zero structs * Fixing the flaky and explicitly broken unit tests --- CMakeLists.txt | 1 + src/client/src/Stream.c | 3 --- src/state/src/State.c | 2 +- .../src/ExponentialBackoffRetryStrategy.c | 2 +- tst/CMakeLists.txt | 3 ++- tst/client/ClientApiTest.cpp | 5 ++-- tst/client/ClientTestFixture.h | 3 ++- tst/client/StreamApiFunctionalityTest.cpp | 17 +++++++------ tst/mkvgen/MkvgenApiTest.cpp | 4 ++-- tst/utils/ExponentialBackoffUtilsTest.cpp | 9 +++---- tst/utils/StackQueue.cpp | 6 ++--- tst/utils/StringSearch.cpp | 2 +- tst/utils/Thread.cpp | 19 +++++++-------- tst/utils/Threadpool.cpp | 10 ++++---- tst/utils/ThreadsafeBlockingQueue.cpp | 24 +++++++++---------- tst/utils/TimerQueue.cpp | 12 ++++++---- 16 files changed, 61 insertions(+), 61 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2d842c7c7..f7ea3a765 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -207,5 +207,6 @@ install( DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig") if(BUILD_TEST) + enable_testing() add_subdirectory(tst) endif() diff --git a/src/client/src/Stream.c b/src/client/src/Stream.c index e17c6d3d6..3ea9deac0 100644 --- a/src/client/src/Stream.c +++ b/src/client/src/Stream.c @@ -1893,9 +1893,6 @@ STATUS putEventMetadata(PKinesisVideoStream pKinesisVideoStream, UINT32 event, P UINT32 packagedSizes[MAX_FRAGMENT_METADATA_COUNT] = {0}; PSerializedMetadata serializedNodes[MAX_FRAGMENT_METADATA_COUNT] = {0}; UINT8 neededNodes = 0; - PSerializedMetadata pExistingSerializedMetadata; - StackQueueIterator iterator; - UINT64 data; CHK(pKinesisVideoStream != NULL, STATUS_NULL_ARG); pKinesisVideoClient = pKinesisVideoStream->pKinesisVideoClient; diff --git a/src/state/src/State.c b/src/state/src/State.c index 2adf8ee9a..4a8999dc1 100644 --- a/src/state/src/State.c +++ b/src/state/src/State.c @@ -296,7 +296,7 @@ STATUS checkForStateTransition(PStateMachine pStateMachine, PBOOL pTransitionRea ENTERS(); STATUS retStatus = STATUS_SUCCESS; PStateMachineState pState = NULL; - UINT64 nextState, time; + UINT64 nextState; UINT64 customData; PStateMachineImpl pStateMachineImpl = (PStateMachineImpl) pStateMachine; UINT64 errorStateTransitionWaitTime = 0; diff --git a/src/utils/src/ExponentialBackoffRetryStrategy.c b/src/utils/src/ExponentialBackoffRetryStrategy.c index f9d822a2a..e7685e03c 100644 --- a/src/utils/src/ExponentialBackoffRetryStrategy.c +++ b/src/utils/src/ExponentialBackoffRetryStrategy.c @@ -23,7 +23,7 @@ STATUS normalizeExponentialBackoffConfig(PExponentialBackoffRetryStrategyConfig pExponentialBackoffRetryStrategyConfig->jitterType, pExponentialBackoffRetryStrategyConfig->jitterFactor); // Seed rand to generate random number for jitter - SRAND(GETTIME()); + SRAND((UINT32) GETTIME()); CleanUp: LEAVES(); diff --git a/tst/CMakeLists.txt b/tst/CMakeLists.txt index 403c10629..29c759031 100644 --- a/tst/CMakeLists.txt +++ b/tst/CMakeLists.txt @@ -1,6 +1,7 @@ cmake_minimum_required(VERSION 3.6.3) set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/CMake;${CMAKE_MODULE_PATH}") include(Utilities) +include(GoogleTest) project(pic_project_tests LANGUAGES CXX) set(CMAKE_CXX_STANDARD 11) @@ -33,4 +34,4 @@ if(UNIX AND NOT APPLE) target_link_libraries(kvspic_test rt) endif() -add_test(${PROJECT_NAME} ${PROJECT_NAME}) \ No newline at end of file +add_test(TARGET kvspic_test EXTRA_ARGS --arg1 "${CMAKE_CURRENT_SOURCE_DIR}") diff --git a/tst/client/ClientApiTest.cpp b/tst/client/ClientApiTest.cpp index 8ed78b387..218e2f8b0 100644 --- a/tst/client/ClientApiTest.cpp +++ b/tst/client/ClientApiTest.cpp @@ -416,6 +416,7 @@ TEST_F(ClientApiTest, kinesisVideoClientCreateSync_Valid_Timeout) TEST_F(ClientApiTest, kinesisVideoClientCreateSync_Store_Alloc) { CLIENT_HANDLE clientHandle, failedClientHandle; + UNUSED_PARAM(failedClientHandle); mClientSyncMode = TRUE; mDeviceInfo.clientInfo.createClientTimeout = 20 * HUNDREDS_OF_NANOS_IN_A_MILLISECOND; @@ -472,7 +473,7 @@ TEST_F(ClientApiTest, getKinesisVideoMetrics_Valid) ClientMetrics kinesisVideoClientMetrics; // Testing all versions for(UINT64 i = 0; i <= CLIENT_METRICS_CURRENT_VERSION; i++) { - kinesisVideoClientMetrics.version = i; + kinesisVideoClientMetrics.version = (UINT32) i; EXPECT_EQ(STATUS_SUCCESS, getKinesisVideoMetrics(mClientHandle, &kinesisVideoClientMetrics)); } } @@ -505,7 +506,7 @@ TEST_F(ClientApiTest, getStreamMetrics_Valid) EXPECT_EQ(STATUS_SUCCESS, createKinesisVideoStream(mClientHandle, &mStreamInfo, &streamHandle)); // Testing all versions for(UINT64 i = 0; i <= STREAM_METRICS_CURRENT_VERSION; i++) { - streamMetrics.version = i; + streamMetrics.version = (UINT32) i; EXPECT_EQ(STATUS_SUCCESS, getKinesisVideoStreamMetrics(streamHandle, &streamMetrics)); } diff --git a/tst/client/ClientTestFixture.h b/tst/client/ClientTestFixture.h index 6ee6fcf0d..8ab95f821 100644 --- a/tst/client/ClientTestFixture.h +++ b/tst/client/ClientTestFixture.h @@ -371,7 +371,7 @@ class ClientTestBase : public ::testing::Test { void initTestMembers() { - UINT32 logLevel = 0; + UINT32 logLevel = LOG_LEVEL_WARN; STATUS retStatus = STATUS_SUCCESS; auto logLevelStr = GETENV("AWS_KVS_LOG_LEVEL"); if (logLevelStr != NULL) { @@ -382,6 +382,7 @@ class ClientTestBase : public ::testing::Test { // Zero things out mClientHandle = INVALID_CLIENT_HANDLE_VALUE; MEMSET(&mDeviceInfo, 0x00, SIZEOF(DeviceInfo)); + MEMSET(&mStreamInfo, 0x00, SIZEOF(StreamInfo)); MEMSET(&mClientHandle, 0x00, SIZEOF(ClientCallbacks)); MEMSET(mDeviceName, 0x00, MAX_DEVICE_NAME_LEN); MEMSET(mStreamName, 0x00, MAX_STREAM_NAME_LEN); diff --git a/tst/client/StreamApiFunctionalityTest.cpp b/tst/client/StreamApiFunctionalityTest.cpp index 43e3161b8..ad7974cc2 100644 --- a/tst/client/StreamApiFunctionalityTest.cpp +++ b/tst/client/StreamApiFunctionalityTest.cpp @@ -99,10 +99,10 @@ TEST_F(StreamApiFunctionalityTest, putFrame_DescribeStreamNotExisting_CreateNotA TEST_F(StreamApiFunctionalityTest, streamFormatChange_stateCheck) { - UINT32 i; - BYTE tempBuffer[10000]; - UINT64 timestamp; - Frame frame; + UINT32 i = 0; + BYTE tempBuffer[10000] = {0}; + UINT64 timestamp = 0; + Frame frame = {0}; BYTE cpd[100]; CreateStream(); @@ -161,13 +161,12 @@ TEST_F(StreamApiFunctionalityTest, streamFormatChange_stateCheck) TEST_F(StreamApiFunctionalityTest, setNalAdaptionFlags_stateCheck) { - UINT32 i; - BYTE tempBuffer[10000]; - UINT64 timestamp; - Frame frame; + UINT32 i = 0; + BYTE tempBuffer[10000] = {0}; + UINT64 timestamp = 0; + Frame frame = {0}; UINT32 nalFlags = NAL_ADAPTATION_ANNEXB_NALS | NAL_ADAPTATION_ANNEXB_CPD_NALS; PKinesisVideoStream pKinesisVideoStream; - PStreamMkvGenerator pStreamMkvGenerator; // The default would be flags NONE CreateStream(); diff --git a/tst/mkvgen/MkvgenApiTest.cpp b/tst/mkvgen/MkvgenApiTest.cpp index 86c6b9e45..cbcd8d689 100644 --- a/tst/mkvgen/MkvgenApiTest.cpp +++ b/tst/mkvgen/MkvgenApiTest.cpp @@ -484,7 +484,7 @@ TEST_F(MkvgenApiTest, mkvgenGenerateTagsChain_OriginalAPICheck) TEST_F(MkvgenApiTest, mkvgenGenerateTagsChain_MaxStringsCheck) { UINT32 size = 100000; - srand(GETTIME()); + SRAND((UINT32) GETTIME()); CHAR tagName[MKV_MAX_TAG_NAME_LEN + 2] = {0}; CHAR tagValue[MKV_MAX_TAG_VALUE_LEN + 2] = {0}; CHAR temp; @@ -529,7 +529,7 @@ TEST_F(MkvgenApiTest, mkvgenIncreaseTagsTagSize_FunctionalityTest) { UINT32 size = 1000, randomSize = 0, encodedSize = 0; PBYTE tagsMkvHolder = (PBYTE) MEMCALLOC(1, size); - srand(GETTIME()); + SRAND((UINT32) GETTIME()); EXPECT_EQ(STATUS_INVALID_ARG, mkvgenIncreaseTagsTagSize(NULL, 0)); diff --git a/tst/utils/ExponentialBackoffUtilsTest.cpp b/tst/utils/ExponentialBackoffUtilsTest.cpp index afb835689..dde32a7d7 100644 --- a/tst/utils/ExponentialBackoffUtilsTest.cpp +++ b/tst/utils/ExponentialBackoffUtilsTest.cpp @@ -59,9 +59,10 @@ class ExponentialBackoffUtilsTest : public UtilTestBase { EXPECT_EQ(expectedExponentialBackoffStatus, pExponentialBackoffRetryStrategyState->status); // Record the actual wait time for validation EXPECT_EQ(retryWaitTimeToVerify, pExponentialBackoffRetryStrategyState->lastRetryWaitTime); - EXPECT_TRUE(inRange(retryWaitTimeToVerify/HUNDREDS_OF_NANOS_IN_A_MILLISECOND, acceptableWaitTimeRange)); + EXPECT_TRUE(inRange((DOUBLE) retryWaitTimeToVerify / HUNDREDS_OF_NANOS_IN_A_MILLISECOND, acceptableWaitTimeRange)); - UINT64 lastRetrySystemTimeMilliSec = pExponentialBackoffRetryStrategyState->lastRetrySystemTime/(HUNDREDS_OF_NANOS_IN_A_MILLISECOND * 1.0); + UINT64 lastRetrySystemTimeMilliSec = + (UINT64) ((DOUBLE) pExponentialBackoffRetryStrategyState->lastRetrySystemTime / (HUNDREDS_OF_NANOS_IN_A_MILLISECOND * 1.0)); UINT64 currentTimeMilliSec = GETTIME()/HUNDREDS_OF_NANOS_IN_A_MILLISECOND; UINT64 diffInMilliSec = currentTimeMilliSec - lastRetrySystemTimeMilliSec; @@ -245,7 +246,7 @@ TEST_F(ExponentialBackoffUtilsTest, testExponentialBackoffBlockingWait_Bounded) UINT64 retryWaitTimeToVerify; UINT32 actualRetryCount = 0; - for (int retryCount = 0; retryCount < maxTestRetryCount; retryCount++) { + for (UINT32 retryCount = 0; retryCount < maxTestRetryCount; retryCount++) { retryWaitTimeToVerify = 0; EXPECT_EQ(STATUS_SUCCESS, getExponentialBackoffRetryStrategyWaitTime(&kvsRetryStrategy, &retryWaitTimeToVerify)); EXPECT_EQ(STATUS_SUCCESS, getExponentialBackoffRetryCount(&kvsRetryStrategy, &actualRetryCount)); @@ -302,7 +303,7 @@ TEST_F(ExponentialBackoffUtilsTest, testExponentialBackoffBlockingWait_FullJitte UINT64 retryWaitTimeToVerify; UINT32 actualRetryCount = 0; - for (int retryCount = 0; retryCount < maxTestRetryCount; retryCount++) { + for (UINT32 retryCount = 0; retryCount < maxTestRetryCount; retryCount++) { retryWaitTimeToVerify = 0; EXPECT_EQ(STATUS_SUCCESS, getExponentialBackoffRetryStrategyWaitTime(&kvsRetryStrategy, &retryWaitTimeToVerify)); EXPECT_EQ(STATUS_SUCCESS, getExponentialBackoffRetryCount(&kvsRetryStrategy, &actualRetryCount)); diff --git a/tst/utils/StackQueue.cpp b/tst/utils/StackQueue.cpp index cfe2e058f..eb9c73629 100644 --- a/tst/utils/StackQueue.cpp +++ b/tst/utils/StackQueue.cpp @@ -178,7 +178,7 @@ TEST_F(StackQueueFunctionalityTest, StackQueueEnqueueAfterValidation) UINT64 checkIndex[5]; UINT64 indexShift[5] = {0}; - srand(GETTIME()); + SRAND((UINT32) GETTIME()); // Create the list EXPECT_EQ(STATUS_SUCCESS, stackQueueCreate(&pStackQueue)); @@ -195,7 +195,7 @@ TEST_F(StackQueueFunctionalityTest, StackQueueEnqueueAfterValidation) for (UINT64 j = 0; j < 5; j++) { checkIndex[j] = rand()%count; - EXPECT_EQ(STATUS_SUCCESS, stackQueueEnqueueAfterIndex(pStackQueue, checkIndex[j], j)); + EXPECT_EQ(STATUS_SUCCESS, stackQueueEnqueueAfterIndex(pStackQueue, (UINT32) checkIndex[j], j)); } for (UINT64 init = 0; init < 5; init++) { @@ -209,7 +209,7 @@ TEST_F(StackQueueFunctionalityTest, StackQueueEnqueueAfterValidation) } for (UINT64 k = 0; k < 5; k++) { - EXPECT_EQ(STATUS_SUCCESS, stackQueueGetAt(pStackQueue, checkIndex[k] + indexShift[k], &data)); + EXPECT_EQ(STATUS_SUCCESS, stackQueueGetAt(pStackQueue, (UINT32) (checkIndex[k] + indexShift[k]), &data)); EXPECT_EQ(k, data); } diff --git a/tst/utils/StringSearch.cpp b/tst/utils/StringSearch.cpp index b209c580f..fd51cc350 100644 --- a/tst/utils/StringSearch.cpp +++ b/tst/utils/StringSearch.cpp @@ -29,7 +29,7 @@ auto randStr = [](SIZE_T n) -> std::string { std::string s; // not ideal, but it's important to always randomize rand - SRAND(GETTIME()); + SRAND((UINT32) GETTIME()); for (int i = 0; i < n; i++) { // 8 bits contain 255 characters + 1 null character s += (CHAR)(RAND() % 255) + 1; diff --git a/tst/utils/Thread.cpp b/tst/utils/Thread.cpp index 533c6e504..111fa3309 100644 --- a/tst/utils/Thread.cpp +++ b/tst/utils/Thread.cpp @@ -9,7 +9,7 @@ MUTEX gThreadMutex; UINT64 gThreadCount; struct sleep_times { - BOOL threadSleepTime; + UINT64 threadSleepTime; BOOL threadVisited; BOOL threadCleared; }; @@ -42,9 +42,9 @@ PVOID testThreadRoutine(PVOID arg) TEST_F(ThreadFunctionalityTest, ThreadCreateAndReleaseSimpleCheck) { UINT64 index; - TID threads[TEST_THREAD_COUNT]; + TID threads[TEST_THREAD_COUNT] = {0}; gThreadMutex = MUTEX_CREATE(FALSE); - struct sleep_times st[TEST_THREAD_COUNT]; + struct sleep_times st[TEST_THREAD_COUNT] = {0}; // Create the threads for (index = 0; index < TEST_THREAD_COUNT; index++) { @@ -74,9 +74,9 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndReleaseSimpleCheck) TEST_F(ThreadFunctionalityTest, ThreadCreateAndCancel) { UINT64 index; - TID threads[TEST_THREAD_COUNT]; + TID threads[TEST_THREAD_COUNT] = {0}; gThreadMutex = MUTEX_CREATE(FALSE); - struct sleep_times st[TEST_THREAD_COUNT]; + struct sleep_times st[TEST_THREAD_COUNT] = {0}; // Create the threads for (index = 0; index < TEST_THREAD_COUNT; index++) { @@ -121,11 +121,11 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndCancel) TEST_F(ThreadFunctionalityTest, ThreadCreateAndReleaseSimpleCheckWithStack) { UINT64 index; - TID threads[TEST_THREAD_COUNT]; + TID threads[TEST_THREAD_COUNT] = {0}; gThreadMutex = MUTEX_CREATE(FALSE); - srand(GETTIME()); + SRAND((UINT32) GETTIME()); SIZE_T threadStack = 64 * 1024; - struct sleep_times st[TEST_THREAD_COUNT]; + struct sleep_times st[TEST_THREAD_COUNT] = {0}; gThreadCount = 0; @@ -156,11 +156,8 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndReleaseSimpleCheckWithStack) TEST_F(ThreadFunctionalityTest, NegativeTest) { - UINT64 index; - TID threads[TEST_THREAD_COUNT]; gThreadMutex = MUTEX_CREATE(FALSE); SIZE_T threadStack = 16 * 1024; - struct sleep_times st[TEST_THREAD_COUNT]; gThreadCount = 0; EXPECT_NE(STATUS_SUCCESS, THREAD_CREATE_WITH_PARAMS(NULL, testThreadRoutine, threadStack, NULL)); diff --git a/tst/utils/Threadpool.cpp b/tst/utils/Threadpool.cpp index 2b6959ccc..ab1fdda98 100644 --- a/tst/utils/Threadpool.cpp +++ b/tst/utils/Threadpool.cpp @@ -65,7 +65,7 @@ TEST_F(ThreadpoolFunctionalityTest, BasicTryAddTest) gTerminateCount = 0; gTerminateMutex = MUTEX_CREATE(FALSE); EXPECT_EQ(STATUS_SUCCESS, semaphoreEmptyCreate(10, &gTerminateSemaphore)); - srand(GETTIME()); + SRAND((UINT32) GETTIME()); EXPECT_EQ(STATUS_SUCCESS, threadpoolCreate(&pThreadpool, 1, 1)); // sleep for a little. Create asynchronously detaches threads, so using TryAdd too soon can @@ -100,7 +100,7 @@ TEST_F(ThreadpoolFunctionalityTest, BasicPushTest) PThreadpool pThreadpool = NULL; BOOL terminate = FALSE; UINT32 count = 0; - srand(GETTIME()); + SRAND((UINT32) GETTIME()); EXPECT_EQ(STATUS_SUCCESS, threadpoolCreate(&pThreadpool, 1, 2)); EXPECT_EQ(STATUS_SUCCESS, threadpoolPush(pThreadpool, exitOnTeardownTask, &terminate)); EXPECT_EQ(STATUS_SUCCESS, threadpoolPush(pThreadpool, exitOnTeardownTask, &terminate)); @@ -133,7 +133,7 @@ TEST_F(ThreadpoolFunctionalityTest, GetThreadCountTest) PThreadpool pThreadpool = NULL; UINT32 count = 0; BOOL terminate = FALSE; - srand(GETTIME()); + SRAND((UINT32) GETTIME()); const UINT32 max = 10; // accepted race condition where min is 1, threadpoolPush can create a new thread // before the first thread is ready to accept tasks @@ -187,7 +187,7 @@ TEST_F(ThreadpoolFunctionalityTest, ThreadsExitGracefullyAfterThreadpoolFreeTest PThreadpool pThreadpool = NULL; UINT32 count = 0; BOOL terminate = FALSE; - srand(GETTIME()); + SRAND((UINT32) GETTIME()); const UINT32 max = 10; UINT32 min = rand() % (max / 2) + 2; EXPECT_EQ(STATUS_SUCCESS, threadpoolCreate(&pThreadpool, min, max)); @@ -249,7 +249,7 @@ TEST_F(ThreadpoolFunctionalityTest, MultithreadUseTest) PThreadpool pThreadpool = NULL; ThreadpoolUser user; UINT32 count = 0; - srand(GETTIME()); + SRAND((UINT32) GETTIME()); const UINT32 max = 10; UINT32 min = rand() % (max / 2) + 2; TID thread1, thread2; diff --git a/tst/utils/ThreadsafeBlockingQueue.cpp b/tst/utils/ThreadsafeBlockingQueue.cpp index b2579e3aa..845889112 100644 --- a/tst/utils/ThreadsafeBlockingQueue.cpp +++ b/tst/utils/ThreadsafeBlockingQueue.cpp @@ -16,8 +16,8 @@ TEST_F(ThreadsafeBlockingQueueFunctionalityTest, createEnqueueDestroyTest) { PSafeBlockingQueue pSafeQueue = NULL; UINT64 totalItems = 0; - srand(GETTIME()); - totalItems = rand()%(UINT16_MAX) + 1; + SRAND((UINT32) GETTIME()); + totalItems = (UINT64) RAND() % (UINT16_MAX) + 1; EXPECT_EQ(STATUS_SUCCESS, safeBlockingQueueCreate(&pSafeQueue)); for(UINT64 i = 0; i < totalItems; i++) { EXPECT_EQ(STATUS_SUCCESS, safeBlockingQueueEnqueue(pSafeQueue, i)); @@ -30,9 +30,9 @@ TEST_F(ThreadsafeBlockingQueueFunctionalityTest, queueIsEmptyTest) PSafeBlockingQueue pSafeQueue = NULL; UINT64 totalItems = 0, totalLoops = 0, item = 0; BOOL empty = FALSE; - srand(GETTIME()); - totalItems = rand()%(UINT16_MAX) + 1; - totalLoops = rand()%64; + SRAND((UINT32) GETTIME()); + totalItems = (UINT64) RAND() % (UINT16_MAX) + 1; + totalLoops = RAND() % 64; EXPECT_EQ(STATUS_SUCCESS, safeBlockingQueueCreate(&pSafeQueue)); for(UINT64 j = 0; j < totalLoops; j++) { EXPECT_EQ(STATUS_SUCCESS, safeBlockingQueueIsEmpty(pSafeQueue, &empty)); @@ -62,7 +62,7 @@ TEST_F(ThreadsafeBlockingQueueFunctionalityTest, queueCountCorrectTest) UINT64 totalItems = 0, totalLoops = 0, item = 0, itemsToQueue = 0;; UINT32 items = 0; BOOL empty = FALSE; - srand(GETTIME()); + SRAND((UINT32) GETTIME()); totalLoops = rand()%64; EXPECT_EQ(STATUS_SUCCESS, safeBlockingQueueCreate(&pSafeQueue)); //queue and dequeue a random number of items in a loop, and check the value @@ -132,12 +132,12 @@ void* readingThread(void* ptr) { TEST_F(ThreadsafeBlockingQueueFunctionalityTest, multithreadQueueDequeueTest) { PSafeBlockingQueue pSafeQueue = NULL; - SafeQueueUser user; + SafeQueueUser user = {0}; UINT64 totalThreads = 0, item = 0, threadCount = 0; BOOL empty = FALSE; TID threads[8] = {0}; - srand(GETTIME()); - totalThreads = rand()%7 + 2; + SRAND((UINT32) GETTIME()); + totalThreads = (UINT64) RAND() % 7 + 2; //make it even totalThreads -= totalThreads%2; EXPECT_EQ(STATUS_SUCCESS, safeBlockingQueueCreate(&pSafeQueue)); @@ -159,12 +159,12 @@ TEST_F(ThreadsafeBlockingQueueFunctionalityTest, multithreadQueueDequeueTest) TEST_F(ThreadsafeBlockingQueueFunctionalityTest, multithreadTeardownTest) { PSafeBlockingQueue pSafeQueue = NULL; - SafeQueueUser user; + SafeQueueUser user = {0}; UINT64 totalThreads = 0, item = 0, threadCount = 0; BOOL empty = FALSE; TID threads[8] = {0}; - srand(GETTIME()); - totalThreads = rand()%7 + 2; + SRAND((UINT32) GETTIME()); + totalThreads = (UINT64) RAND() % 7 + 2; //make it even totalThreads -= totalThreads%2; EXPECT_EQ(STATUS_SUCCESS, safeBlockingQueueCreate(&pSafeQueue)); diff --git a/tst/utils/TimerQueue.cpp b/tst/utils/TimerQueue.cpp index 8d918b554..ec9a23d43 100644 --- a/tst/utils/TimerQueue.cpp +++ b/tst/utils/TimerQueue.cpp @@ -538,7 +538,10 @@ TEST_F(TimerQueueFunctionalityTest, timerCancelDirectlyOneLeaveAnotherStart) TEST_F(TimerQueueFunctionalityTest, miniStressTest) { - UINT64 period = 1 * HUNDREDS_OF_NANOS_IN_A_MILLISECOND; + // This test is highly sensitive to hardware platform capabilities and the version and the type of the underlying OS. + // For example, many server OS-es have 30ms+ schedulers and the timer functionality is based on the OS scheduler. + const UINT64 periodInMs = 100; + UINT64 period = periodInMs * HUNDREDS_OF_NANOS_IN_A_MILLISECOND; TIMER_QUEUE_HANDLE handle = INVALID_TIMER_QUEUE_HANDLE_VALUE; UINT32 timerIds[DEFAULT_TIMER_QUEUE_TIMER_COUNT], i; @@ -555,7 +558,7 @@ TEST_F(TimerQueueFunctionalityTest, miniStressTest) } // Let it fire for a while - THREAD_SLEEP(1000 * HUNDREDS_OF_NANOS_IN_A_MILLISECOND); + THREAD_SLEEP(HUNDREDS_OF_NANOS_IN_A_SECOND); // Cancel all the timers for (i = 0; i < DEFAULT_TIMER_QUEUE_TIMER_COUNT; i++) { @@ -564,7 +567,7 @@ TEST_F(TimerQueueFunctionalityTest, miniStressTest) // There should be a number of timers * duration in millis invokes but the actual number is timing // dependent and will be lower. We will check for half. - EXPECT_LT(1000 * DEFAULT_TIMER_QUEUE_TIMER_COUNT / 2, ATOMIC_LOAD(&invokeCount)); + EXPECT_LT((1000 / periodInMs) * DEFAULT_TIMER_QUEUE_TIMER_COUNT / 2, ATOMIC_LOAD(&invokeCount)); // Free the timer queue EXPECT_EQ(STATUS_SUCCESS, timerQueueFree(&handle)); @@ -794,7 +797,6 @@ TEST_F(TimerQueueFunctionalityTest, kickTimerQueueTest) { TIMER_QUEUE_HANDLE handle = INVALID_TIMER_QUEUE_HANDLE_VALUE; UINT32 timerId; - UINT64 curTime; // Make sure we don't check for the timer in the test callback checkTimerId = FALSE; @@ -810,7 +812,7 @@ TEST_F(TimerQueueFunctionalityTest, kickTimerQueueTest) EXPECT_EQ(STATUS_SUCCESS, timerQueueKick(handle, timerId)); EXPECT_NE(STATUS_SUCCESS, timerQueueKick(INVALID_TIMER_QUEUE_HANDLE_VALUE, timerId)); - EXPECT_NE(STATUS_SUCCESS, timerQueueKick(handle, 0)); + EXPECT_NE(STATUS_SUCCESS, timerQueueKick(handle, MAX_UINT32)); //let kick happen THREAD_SLEEP(100 * HUNDREDS_OF_NANOS_IN_A_MILLISECOND); From b69d1b9ca43c98695c71b1f2c8c27dcf12149b5b Mon Sep 17 00:00:00 2001 From: Mushegh Malkhasyan Date: Tue, 7 May 2024 11:31:42 -0700 Subject: [PATCH 2/4] Attempting to fix the Thread test TSAN run failures --- tst/utils/Thread.cpp | 58 ++++++++++++++------------------------------ 1 file changed, 18 insertions(+), 40 deletions(-) diff --git a/tst/utils/Thread.cpp b/tst/utils/Thread.cpp index 111fa3309..93061756e 100644 --- a/tst/utils/Thread.cpp +++ b/tst/utils/Thread.cpp @@ -5,8 +5,7 @@ class ThreadFunctionalityTest : public UtilTestBase { #define TEST_THREAD_COUNT 500 -MUTEX gThreadMutex; -UINT64 gThreadCount; +volatile SIZE_T gThreadCount = 0; struct sleep_times { UINT64 threadSleepTime; @@ -16,26 +15,20 @@ struct sleep_times { PVOID testThreadRoutine(PVOID arg) { - MUTEX_LOCK(gThreadMutex); - gThreadCount++; + ATOMIC_INCREMENT(&gThreadCount); struct sleep_times* st = (struct sleep_times*) arg; // Mark as visited st->threadVisited = TRUE; - MUTEX_UNLOCK(gThreadMutex); - UINT64 sleepTime = st->threadSleepTime; // Just sleep for some time THREAD_SLEEP(sleepTime); - MUTEX_LOCK(gThreadMutex); - // Mark as cleared st->threadCleared = TRUE; - gThreadCount--; - MUTEX_UNLOCK(gThreadMutex); + ATOMIC_DECREMENT(&gThreadCount); return NULL; } @@ -43,9 +36,10 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndReleaseSimpleCheck) { UINT64 index; TID threads[TEST_THREAD_COUNT] = {0}; - gThreadMutex = MUTEX_CREATE(FALSE); struct sleep_times st[TEST_THREAD_COUNT] = {0}; + ATOMIC_STORE(&gThreadCount, 0); + // Create the threads for (index = 0; index < TEST_THREAD_COUNT; index++) { st[index].threadVisited = FALSE; @@ -59,25 +53,22 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndReleaseSimpleCheck) EXPECT_EQ(STATUS_SUCCESS, THREAD_JOIN(threads[index], NULL)); } - MUTEX_LOCK(gThreadMutex); - EXPECT_EQ(0, gThreadCount); - MUTEX_UNLOCK(gThreadMutex); - + EXPECT_EQ(0, ATOMIC_LOAD(&gThreadCount)); + for (index = 0; index < TEST_THREAD_COUNT; index++) { EXPECT_TRUE(st[index].threadVisited) << "Thread didn't visit index " << index; EXPECT_TRUE(st[index].threadCleared) << "Thread didn't clear index " << index; } - - MUTEX_FREE(gThreadMutex); } TEST_F(ThreadFunctionalityTest, ThreadCreateAndCancel) { UINT64 index; TID threads[TEST_THREAD_COUNT] = {0}; - gThreadMutex = MUTEX_CREATE(FALSE); struct sleep_times st[TEST_THREAD_COUNT] = {0}; + ATOMIC_STORE(&gThreadCount, 0); + // Create the threads for (index = 0; index < TEST_THREAD_COUNT; index++) { @@ -95,8 +86,10 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndCancel) #endif } - // wait 2 seconds to make sure all threads are executed - THREAD_SLEEP(2 * HUNDREDS_OF_NANOS_IN_A_SECOND); + // Wait for the threads to finish the initial phase + while (ATOMIC_LOAD(&gThreadCount) != TEST_THREAD_COUNT) { + THREAD_SLEEP(5 * HUNDREDS_OF_NANOS_IN_A_MILLISECOND); + } // Cancel all the threads for (index = 0; index < TEST_THREAD_COUNT; index++) { @@ -105,29 +98,23 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndCancel) // Validate that threads have been killed and didn't finish successfully - MUTEX_LOCK(gThreadMutex); - EXPECT_EQ(TEST_THREAD_COUNT, gThreadCount); + EXPECT_EQ(TEST_THREAD_COUNT, ATOMIC_LOAD(&gThreadCount)); for (index = 0; index < TEST_THREAD_COUNT; index++) { EXPECT_TRUE(st[index].threadVisited) << "Thread didn't visit index " << index; EXPECT_FALSE(st[index].threadCleared) << "Thread shouldn't have cleared index " << index; } - - MUTEX_UNLOCK(gThreadMutex); - - MUTEX_FREE(gThreadMutex); } TEST_F(ThreadFunctionalityTest, ThreadCreateAndReleaseSimpleCheckWithStack) { UINT64 index; TID threads[TEST_THREAD_COUNT] = {0}; - gThreadMutex = MUTEX_CREATE(FALSE); SRAND((UINT32) GETTIME()); SIZE_T threadStack = 64 * 1024; struct sleep_times st[TEST_THREAD_COUNT] = {0}; - gThreadCount = 0; + ATOMIC_STORE(&gThreadCount, 0); // Create the threads for (index = 0; index < TEST_THREAD_COUNT; index++) { @@ -142,30 +129,21 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndReleaseSimpleCheckWithStack) EXPECT_EQ(STATUS_SUCCESS, THREAD_JOIN(threads[index], NULL)); } - MUTEX_LOCK(gThreadMutex); - EXPECT_EQ(0, gThreadCount); - MUTEX_UNLOCK(gThreadMutex); + EXPECT_EQ(0, ATOMIC_LOAD(&gThreadCount)); for (index = 0; index < TEST_THREAD_COUNT; index++) { EXPECT_TRUE(st[index].threadVisited) << "Thread didn't visit index " << index; EXPECT_TRUE(st[index].threadCleared) << "Thread didn't clear index " << index; } - - MUTEX_FREE(gThreadMutex); } TEST_F(ThreadFunctionalityTest, NegativeTest) { - gThreadMutex = MUTEX_CREATE(FALSE); SIZE_T threadStack = 16 * 1024; - gThreadCount = 0; + ATOMIC_STORE(&gThreadCount, 0); EXPECT_NE(STATUS_SUCCESS, THREAD_CREATE_WITH_PARAMS(NULL, testThreadRoutine, threadStack, NULL)); EXPECT_NE(STATUS_SUCCESS, THREAD_CREATE(NULL, testThreadRoutine, NULL)); - MUTEX_LOCK(gThreadMutex); - EXPECT_EQ(0, gThreadCount); - MUTEX_UNLOCK(gThreadMutex); - - MUTEX_FREE(gThreadMutex); + EXPECT_EQ(0, ATOMIC_LOAD(&gThreadCount)); } From e118dbca26e729df6c243f271a487f49af725ed4 Mon Sep 17 00:00:00 2001 From: Mushegh Malkhasyan Date: Fri, 17 May 2024 15:10:32 -0700 Subject: [PATCH 3/4] Fixing pthread build on Windows WSL2. Fixing TSAN thread test. --- tst/CMakeLists.txt | 1 + tst/utils/Thread.cpp | 22 ++++++++++------------ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/tst/CMakeLists.txt b/tst/CMakeLists.txt index 29c759031..d7f0cff41 100644 --- a/tst/CMakeLists.txt +++ b/tst/CMakeLists.txt @@ -8,6 +8,7 @@ set(CMAKE_CXX_STANDARD 11) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC") +SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pthread") if (OPEN_SRC_INSTALL_PREFIX) find_package(GTest REQUIRED PATHS ${OPEN_SRC_INSTALL_PREFIX}) diff --git a/tst/utils/Thread.cpp b/tst/utils/Thread.cpp index 93061756e..5ab00de8d 100644 --- a/tst/utils/Thread.cpp +++ b/tst/utils/Thread.cpp @@ -7,16 +7,16 @@ class ThreadFunctionalityTest : public UtilTestBase { volatile SIZE_T gThreadCount = 0; -struct sleep_times { +typedef struct { UINT64 threadSleepTime; BOOL threadVisited; BOOL threadCleared; -}; +} sleep_times; PVOID testThreadRoutine(PVOID arg) { ATOMIC_INCREMENT(&gThreadCount); - struct sleep_times* st = (struct sleep_times*) arg; + sleep_times* st = (sleep_times*) arg; // Mark as visited st->threadVisited = TRUE; @@ -36,7 +36,7 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndReleaseSimpleCheck) { UINT64 index; TID threads[TEST_THREAD_COUNT] = {0}; - struct sleep_times st[TEST_THREAD_COUNT] = {0}; + sleep_times st[TEST_THREAD_COUNT] = {0}; ATOMIC_STORE(&gThreadCount, 0); @@ -65,7 +65,7 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndCancel) { UINT64 index; TID threads[TEST_THREAD_COUNT] = {0}; - struct sleep_times st[TEST_THREAD_COUNT] = {0}; + sleep_times st[TEST_THREAD_COUNT] = {0}; ATOMIC_STORE(&gThreadCount, 0); @@ -78,12 +78,6 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndCancel) st[index].threadSleepTime = 20 * HUNDREDS_OF_NANOS_IN_A_SECOND; EXPECT_EQ(STATUS_SUCCESS, THREAD_CREATE(&threads[index], testThreadRoutine, (PVOID)&st[index])); -#if !(defined _WIN32 || defined _WIN64 || defined __CYGWIN__) - // We should detach thread for non-windows platforms only - // Windows implementation would cancel the handle and the - // consequent thread cancel would fail - EXPECT_EQ(STATUS_SUCCESS, THREAD_DETACH(threads[index])); -#endif } // Wait for the threads to finish the initial phase @@ -96,6 +90,10 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndCancel) EXPECT_EQ(STATUS_SUCCESS, THREAD_CANCEL(threads[index])); } + // Await for the threads to finish + for (index = 0; index < TEST_THREAD_COUNT; index++) { + EXPECT_EQ(STATUS_SUCCESS, THREAD_JOIN(threads[index], NULL)); + } // Validate that threads have been killed and didn't finish successfully EXPECT_EQ(TEST_THREAD_COUNT, ATOMIC_LOAD(&gThreadCount)); @@ -112,7 +110,7 @@ TEST_F(ThreadFunctionalityTest, ThreadCreateAndReleaseSimpleCheckWithStack) TID threads[TEST_THREAD_COUNT] = {0}; SRAND((UINT32) GETTIME()); SIZE_T threadStack = 64 * 1024; - struct sleep_times st[TEST_THREAD_COUNT] = {0}; + sleep_times st[TEST_THREAD_COUNT] = {0}; ATOMIC_STORE(&gThreadCount, 0); From f612dcca1934f9f4fb60d204d49a7aad4009664d Mon Sep 17 00:00:00 2001 From: Mushegh Malkhasyan Date: Mon, 20 May 2024 18:03:25 -0700 Subject: [PATCH 4/4] clang-format the include file as the version of the tooling has changed on the backend --- .../include/com/amazonaws/kinesis/video/mkvgen/Include.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/mkvgen/include/com/amazonaws/kinesis/video/mkvgen/Include.h b/src/mkvgen/include/com/amazonaws/kinesis/video/mkvgen/Include.h index 788f9e4d7..7171d9ac1 100644 --- a/src/mkvgen/include/com/amazonaws/kinesis/video/mkvgen/Include.h +++ b/src/mkvgen/include/com/amazonaws/kinesis/video/mkvgen/Include.h @@ -324,10 +324,7 @@ typedef struct __Frame { * * The initializer will zero all the fields and set the EoFr flag in flags. */ -#define EOFR_FRAME_INITIALIZER \ - { \ - FRAME_CURRENT_VERSION, 0, FRAME_FLAG_END_OF_FRAGMENT, 0, 0, 0, 0, NULL, 0 \ - } +#define EOFR_FRAME_INITIALIZER {FRAME_CURRENT_VERSION, 0, FRAME_FLAG_END_OF_FRAGMENT, 0, 0, 0, 0, NULL, 0} /** * The representation of mkv video element