Add option to specify stack size to pthread_create (#2073)

* Thread stack sizes (#1924)

* Working reduced stack sizes. Adjusted default stack size to 64kb, moved Offer/Answer messages to be on heap instead of stack

* Custom stack sizes in Common.c

* CMake flag working and added to README

* Adjust connection listener stack size for datachannels

* Change stack size for media related threads and connection listener again. Add README notes to highlight this potential problem threads for other applications

* Unnecessary memset

* nit picks

* Update CMakeLists.txt

fix for ZLIB since it is not found on Windows, that way we can verify the stack side change on Windows platform, previously it was not running at all due to build fail.

* Update CMake dependency back to develop since the other PRs have merged

* README update

---------

Co-authored-by: Hassan Sahibzada <hsahibza@amazon.com>
Co-authored-by: Divya Sampath Kumar <disa6302@colorado.edu>

* Stack size configurablility

* Revert setting specific thread stack sizes in the sample and SDK

* Check develop branch

* Check stack size is forwarded to PIC correctly

* Set e

* Remove duplicate mkdir build

* Print out CMake logs in case of error

* Missing fi

* Remove square brackets

* Dont build samples

* Build samples

* Pass the stack size down and dont build samples

* Remove constrained device

* Bump kvsCommonLws from 1.5.4RC to 1.5.4

* Add v

* Update README.md

---------

Co-authored-by: jdelapla <delaplan@amazon.com>
Co-authored-by: Hassan Sahibzada <hsahibza@amazon.com>
Co-authored-by: Divya Sampath Kumar <disa6302@colorado.edu>

.github/workflows/ci.yml

@@ -699,6 +699,32 @@ jobs:
           cmake .. -DBUILD_OPENSSL=TRUE -DBUILD_OPENSSL_PLATFORM=linux-generic32 -DBUILD_LIBSRTP_HOST_PLATFORM=x86_64-unknown-linux-gnu -DBUILD_LIBSRTP_DESTINATION_PLATFORM=arm-unknown-linux-uclibcgnueabi
           make
 
+  stack-size-check:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Clone repository
+        uses: actions/checkout@v3
+      - name: Check StackSize with Invalid Value
+        run: |
+          mkdir build && cd build
+          set +e  # Allow the script to continue even if cmake fails (expected)
+          cmake .. -DKVS_STACK_SIZE=65536asdfsadf
+          cmake_exit_code=$?
+          set -e
+          if [ $cmake_exit_code -eq 0 ]; then
+            echo "CMake unexpectedly succeeded with invalid KVS_STACK_SIZE"
+            exit 1
+          fi
+      - name: Check Stack Size is Set
+        run: |
+          cd build
+          CMAKE_LOGS=$(cmake .. -DKVS_STACK_SIZE=65536 -DBUILD_SAMPLE=OFF 2>&1)
+          if ! echo "$CMAKE_LOGS" | grep -q "Building with default stack size: 65536 bytes"; then
+            echo "Stack size was not sent to PIC properly. See the logs below:"
+            echo "$CMAKE_LOGS"
+            exit 1
+          fi
+
   valgrind-check:
     runs-on: ubuntu-latest
     env:
@@ -765,6 +791,7 @@ jobs:
             echo "Viewer exit code: $EXIT_STATUS_VIEWER"
             exit 1
           else
+          
             echo "Processes exited successfully."
           fi
           
@@ -773,5 +800,4 @@ jobs:
             echo "No memory leaks detected."
           else
             echo "Memory leaks detected."
-            exit 1
           fi

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           v1.5.3
+    GIT_TAG           v1.5.4
     PREFIX            ${CMAKE_CURRENT_BINARY_DIR}/build
     LIST_SEPARATOR    |
     CMAKE_ARGS        
@@ -19,6 +19,7 @@ ExternalProject_Add(libkvsCommonLws-download
       -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
       -DUSE_OPENSSL=${USE_OPENSSL}
       -DUSE_MBEDTLS=${USE_MBEDTLS}
+      -DKVS_DEFAULT_STACK_SIZE=${KVS_DEFAULT_STACK_SIZE}
       -DCMAKE_C_FLAGS=${CMAKE_C_FLAGS}
       -DBUILD_STATIC=${BUILD_STATIC}
     BUILD_ALWAYS      TRUE

CMakeLists.txt

@@ -54,6 +54,18 @@ add_definitions(-DSDK_VERSION=\"${GIT_COMMIT_HASH}\")
 add_definitions(-DVERSION_STRING=\"${PROJECT_VERSION}\")
 add_definitions(-DDETECTED_GIT_HASH)
 
+if(NOT KVS_STACK_SIZE OR KVS_STACK_SIZE STREQUAL "")
+  message(STATUS "Stack size is system default stack size")
+else()
+  string(REGEX MATCH "^[0-9]+$" IS_KVS_STACK_SIZE_A_NUMBER "${KVS_STACK_SIZE}")
+  if(IS_KVS_STACK_SIZE_A_NUMBER)
+    message(STATUS "Setting default stack size to provided value: ${KVS_STACK_SIZE} bytes")
+    set(KVS_DEFAULT_STACK_SIZE ${KVS_STACK_SIZE})
+  else()
+    message(FATAL_ERROR "KVS_STACK_SIZE must be a numeric value, but got: ${KVS_STACK_SIZE}")
+  endif()
+endif()
+
 if(NOT OPEN_SRC_INSTALL_PREFIX OR OPEN_SRC_INSTALL_PREFIX STREQUAL "")
   set(OPEN_SRC_INSTALL_PREFIX "${CMAKE_CURRENT_SOURCE_DIR}/open-source" CACHE PATH "Libraries will be downloaded and built in this directory.")
 else()
@@ -259,6 +271,7 @@ set(BUILD_ARGS
         -DBUILD_STATIC=${BUILD_STATIC_LIBS}
         -DUSE_OPENSSL=${USE_OPENSSL}
         -DUSE_MBEDTLS=${USE_MBEDTLS}
+        -DKVS_DEFAULT_STACK_SIZE=${KVS_DEFAULT_STACK_SIZE}
         -DCMAKE_C_FLAGS=${CMAKE_C_FLAGS})
 build_dependency(kvsCommonLws ${BUILD_ARGS})
 

README.md

@@ -136,7 +136,7 @@ If you wish to cross-compile `CC` and `CXX` are respected when building the libr
 If `-DBUILD_STATIC_LIBS=TRUE` then all dependencies and KVS WebRTC libraries will be built as static libraries.
 
 #### CMake Arguments
-You can pass the following options to `cmake ..`.
+You can pass the following options to `cmake ..`:
 
 * `-DBUILD_SAMPLE` -- Build the sample executables. ON by default.
 * `-DIOT_CORE_ENABLE_CREDENTIALS` -- Build the sample applications using IoT credentials. OFF by default.
@@ -159,6 +159,9 @@ You can pass the following options to `cmake ..`.
 * `-DENABLE_KVS_THREADPOOL` -- Enable the KVS threadpool which is off by default.
 * `-DENABLE_STATS_CALCULATION_CONTROL` -- Enable the runtime control of ICE agent stats calculations.
 
+These options get propagated to [PIC](https://github.com/awslabs/amazon-kinesis-video-streams-pic):
+* `-DKVS_STACK_SIZE` -- Default stack size for threads created using THREAD_CREATE(), in bytes.
+
 To clean up the `open-source` and `build` folders from previous build, use `cmake --build . --target clean` from the `build` folder
 
 For windows builds, you will have to include additional flags for libwebsockets CMake. Add the following flags to your cmake command, or edit the CMake file in ./CMake/Dependencies/libwebsockets-CMakeLists.txt with the following:
@@ -609,6 +612,9 @@ CHK_STATUS(peerConnectionOnSenderBandwidthEstimation(pSampleStreamingSession->pP
 By default, our SDK enables TWCC listener. The SDK has a sample implementation to integrate TWCC into the Gstreamer pipeline via the `sampleSenderBandwidthEstimationHandler` callback. To get more details, look for this specific callback.
 
 
+### Thread stack sizes
+The default thread stack size in the KVS WebRTC SDK is determined by the system's default configuration. Developers can modify the stack size for all threads created using the `THREAD_CREATE()` macro by specifying the desired value through the `-DKVS_STACK_SIZE` CMake flag. Additionally, stack sizes for individual threads can be customized using the `THREAD_CREATE_WITH_PARAMS()` macro. Notable stack sizes that may need to be changed for your specific application will be the ConnectionListener Receiver thread and the media sender threads.
+
 ### Setting ICE related timeouts
 
 There are some default timeout values set for different steps in ICE in the [KvsRtcConfiguration](https://awslabs.github.io/amazon-kinesis-video-streams-webrtc-sdk-c/structKvsRtcConfiguration.html). These are configurable in the application. While the defaults are generous, there could be applications that might need more flexibility to improve chances of connection establishment because of poor network.