Create a CMake flag to enable KVS Threadpool (#1806)

* create a cmake flag to enable kvs threadpool and pass it to the SDK * create a cmake flag to enable kvs threadpool and pass it to the SDK * missed one spot * add debug logs to fix tests * fix tests attempt 1 - initialize min and max for the threadpool * silly syntax error * setting min value for threadpool in test fixture * fix clang format * fixing client info initializations in other tests * More initializations for tests * Add sleep at end of tests to avoid false flags of memory leaks as threads end asynchronously * add tsan supp * tsan sup -2 * missing # in details in supp * more sleeps to allow threadpool to close out all threads * add suppressions to threadpool * add suppressions to threadpool push * race:pthread_mutex_unlock * add test name instead of threadpool function names * rename the cmake var to control only signaling threadpool * try without . in the test name * use mutex instead of race * remove mutex:lock * add back mutex:lock * deadlock:iceServerConfigRefreshConnectedAuthExpiration * deadlock:createSignalingSync * Update producer dependency to include modified threadpool teardown * Remove previous suppressions * cleanup + rename * update git tag --------- Co-authored-by: James Delaplane <[email protected]>
awslabs · Sep 20, 2023 · 8ce0c98 · 8ce0c98
1 parent f64a708
commit 8ce0c98
Show file tree

Hide file tree

Showing 11 changed files with 163 additions and 11 deletions.
diff --git a/CMake/Dependencies/libkvsCommonLws-CMakeLists.txt b/CMake/Dependencies/libkvsCommonLws-CMakeLists.txt
@@ -6,7 +6,7 @@ include(ExternalProject)
 
 ExternalProject_Add(libkvsCommonLws-download
     GIT_REPOSITORY    https://github.com/awslabs/amazon-kinesis-video-streams-producer-c.git
-    GIT_TAG           178109a5dbfc5288ba5cf7fab1dc1afd5e2e182b
+    GIT_TAG           fd562a16e41b1972df14c230b6be7a7d3f75bc6b
     PREFIX            ${CMAKE_CURRENT_BINARY_DIR}/build
     CMAKE_ARGS        
       -DCMAKE_INSTALL_PREFIX=${OPEN_SRC_INSTALL_PREFIX}

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -18,6 +18,7 @@ option(BUILD_LIBSRTP_HOST_PLATFORM "If buildng LibSRTP what is the current platf
 option(BUILD_LIBSRTP_DESTINATION_PLATFORM "If buildng LibSRTP what is the destination platform" OFF)
 option(BUILD_SAMPLE "Build available samples" ON)
 option(ENABLE_DATA_CHANNEL "Enable support for data channel" ON)
+option(ENABLE_KVS_THREADPOOL "Enable support for KVS thread pool in signaling" ON)
 option(INSTRUMENTED_ALLOCATORS "Enable memory instrumentation" OFF)
 
 # Developer Flags
@@ -98,6 +99,10 @@ message(STATUS "dependencies install path is ${OPEN_SRC_INSTALL_PREFIX}")
 add_definitions(-DKVS_CA_CERT_PATH="${CMAKE_SOURCE_DIR}/certs/cert.pem")
 add_definitions(-DCMAKE_DETECTED_CACERT_PATH)
 
+if (ENABLE_KVS_THREADPOOL)
+  add_definitions(-DENABLE_KVS_THREADPOOL)
+endif()
+
 if(USE_OPENSSL)
   add_definitions(-DKVS_USE_OPENSSL)
 elseif(USE_MBEDTLS)

diff --git a/README.md b/README.md
@@ -360,8 +360,7 @@ To build on a 32-bit Raspbian GNU/Linux 11 on 64-bit hardware, the OpenSSL libra
 
 ### Threadpool for Signaling Channel messages
 The threadpool is enabled by default, and starts with 3 threads that it can increase up to 5 if all 3 are actively in use. To change these values to better match the resources of your use case
-please edit samples/Samples.h defines `KVS_SIGNALING_THREADPOOL_MIN` and `KVS_SIGNALING_THREADPOOL_MAX`. You can also disable the threadpool to instead create and detach each thread
-to handle signaling messages by commenting out `KVS_USE_SIGNALING_CHANNEL_THREADPOOL`.
+please edit samples/Samples.h defines `KVS_SIGNALING_THREADPOOL_MIN` and `KVS_SIGNALING_THREADPOOL_MAX`. You can also disable the threadpool to instead create and detach each thread to handle signaling messages by disabling the flag `-DENABLE_KVS_THREADPOOL` while building with cmake.
 
 ## Documentation
 All Public APIs are documented in our [Include.h](https://github.com/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/blob/master/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h), we also generate a [Doxygen](https://awslabs.github.io/amazon-kinesis-video-streams-webrtc-sdk-c/) each commit for easier navigation.

diff --git a/samples/Samples.h b/samples/Samples.h
@@ -56,9 +56,6 @@ extern "C" {
 #define KVS_SIGNALING_THREADPOOL_MIN 3
 #define KVS_SIGNALING_THREADPOOL_MAX 5
 
-// comment out this line to disable the feature
-#define KVS_USE_SIGNALING_CHANNEL_THREADPOOL 1
-
 /* Uncomment the following line in order to enable IoT credentials checks in the provided samples */
 // #define IOT_CORE_ENABLE_CREDENTIALS  1
 

diff --git a/src/source/Signaling/LwsApiCalls.c b/src/source/Signaling/LwsApiCalls.c
@@ -2011,7 +2011,7 @@ STATUS receiveLwsMessage(PSignalingClient pSignalingClient, PCHAR pMessage, UINT
         DLOGW("Failed to validate the ICE server configuration received with an Offer");
     }
 
-#ifdef KVS_USE_SIGNALING_CHANNEL_THREADPOOL
+#ifdef ENABLE_KVS_THREADPOOL
     CHK_STATUS(threadpoolPush(pSignalingClient->pThreadpool, receiveLwsMessageWrapper, (PVOID) pSignalingMessageWrapper));
 #else
     // Issue the callback on a separate thread

diff --git a/src/source/Signaling/Signaling.c b/src/source/Signaling/Signaling.c
@@ -37,9 +37,11 @@ STATUS createSignalingSync(PSignalingClientInfoInternal pClientInfo, PChannelInf
     CHK_STATUS(createValidateChannelInfo(pChannelInfo, &pSignalingClient->pChannelInfo));
     CHK_STATUS(validateSignalingCallbacks(pSignalingClient, pCallbacks));
     CHK_STATUS(validateSignalingClientInfo(pSignalingClient, pClientInfo));
-#ifdef KVS_USE_SIGNALING_CHANNEL_THREADPOOL
+#ifdef ENABLE_KVS_THREADPOOL
+    DLOGD("Going to crate the threadpool for signaling");
     CHK_STATUS(threadpoolCreate(&pSignalingClient->pThreadpool, pClientInfo->signalingClientInfo.signalingMessagesMinimumThreads,
                                 pClientInfo->signalingClientInfo.signalingMessagesMaximumThreads));
+    DLOGD("Successfully created the threadpool for signaling");
 #endif
 
     pSignalingClient->version = SIGNALING_CLIENT_CURRENT_VERSION;
@@ -225,7 +227,7 @@ STATUS freeSignaling(PSignalingClient* ppSignalingClient)
 
     hashTableFree(pSignalingClient->diagnostics.pEndpointToClockSkewHashMap);
 
-#ifdef KVS_USE_SIGNALING_CHANNEL_THREADPOOL
+#ifdef ENABLE_KVS_THREADPOOL
     threadpoolFree(pSignalingClient->pThreadpool);
 #endif
 

diff --git a/src/source/Signaling/Signaling.h b/src/source/Signaling/Signaling.h
@@ -339,7 +339,7 @@ typedef struct {
     UINT64 deleteTime;
     UINT64 connectTime;
 
-#ifdef KVS_USE_SIGNALING_CHANNEL_THREADPOOL
+#ifdef ENABLE_KVS_THREADPOOL
     PThreadpool pThreadpool;
 #endif
     UINT64 offerTime;