From 0def1b1a1094fc57752f241250e9a1aed71bbffd Mon Sep 17 00:00:00 2001 From: Sam Umbach Date: Mon, 19 May 2025 08:39:46 -0400 Subject: [PATCH 01/25] [build] Update for compatibility with CMake 4.x (#3167) --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 847b6f23f..847564d03 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,7 +7,7 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. # -cmake_minimum_required (VERSION 2.8.12 FATAL_ERROR) +cmake_minimum_required (VERSION 3.5 FATAL_ERROR) set (SRT_VERSION 1.5.4) set (CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/scripts") From 19113e65d9c66ed32746a1d49c0a82a11ad92126 Mon Sep 17 00:00:00 2001 From: yaruno Date: Mon, 19 May 2025 14:41:09 +0200 Subject: [PATCH 02/25] [build] Update cxx11-macos.yaml (#3170) * Update for compatibility with CMake 4.x * Update cxx11-macos.yaml Downgrade googletest to 1.16.x as latest one requires C++ 17 * Update cxx11-macos.yaml try to download the exact version of google test formula from homebrew history * Update cxx11-macos.yaml downgrade googletest to 1.12.1 --------- Co-authored-by: Sam Umbach --- .github/workflows/cxx11-macos.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/cxx11-macos.yaml b/.github/workflows/cxx11-macos.yaml index e3e6b1f8a..4732c2abb 100644 --- a/.github/workflows/cxx11-macos.yaml +++ b/.github/workflows/cxx11-macos.yaml @@ -13,7 +13,9 @@ jobs: steps: - name: GoogleTest - run: brew install googletest + run: | + curl -o googletest.rb https://raw.githubusercontent.com/Homebrew/homebrew-core/23e7fb4dc0cc73facc3772815741e1deb87d6406/Formula/googletest.rb + brew install -s googletest.rb - uses: actions/checkout@v3 - name: configure run: | From 21370a7e8f63a4816ffabe768726b68662c27731 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Wed, 21 May 2025 15:36:37 +0200 Subject: [PATCH 03/25] [core] Fixed problems found by thread and memory sanitizers (#3166) * Fixed some thread-related problems * Fixed a leak in group objects * Fixed incorrect conditional for a group closure fix * Update srtcore/group.cpp - comment typo --------- Co-authored-by: Mikolaj Malecki --- srtcore/api.cpp | 15 ++++++++++++++- srtcore/core.cpp | 2 +- srtcore/core.h | 1 + srtcore/group.cpp | 22 +++++++++++++++------- test/test_bonding.cpp | 2 +- 5 files changed, 32 insertions(+), 10 deletions(-) diff --git a/srtcore/api.cpp b/srtcore/api.cpp index 1108c0a1d..14d7a4652 100644 --- a/srtcore/api.cpp +++ b/srtcore/api.cpp @@ -318,6 +318,15 @@ void srt::CUDTUnited::closeAllSockets() { j->second->m_tsClosureTimeStamp = steady_clock::time_point(); } + +#if ENABLE_BONDING + for (groups_t::iterator j = m_Groups.begin(); j != m_Groups.end(); ++j) + { + SRTSOCKET id = j->second->m_GroupID; + m_ClosedGroups[id] = j->second; + } + m_Groups.clear(); +#endif } HLOGC(inlog.Debug, log << "GC: GLOBAL EXIT - releasing all CLOSED sockets."); @@ -591,7 +600,11 @@ int srt::CUDTUnited::newConnection(const SRTSOCKET listen, } // exceeding backlog, refuse the connection request - if (ls->m_QueuedSockets.size() >= ls->m_uiBackLog) + + enterCS(ls->m_AcceptLock); + size_t backlog = ls->m_QueuedSockets.size(); + leaveCS(ls->m_AcceptLock); + if (backlog >= ls->m_uiBackLog) { w_error = SRT_REJ_BACKLOG; LOGC(cnlog.Note, log << "newConnection: listen backlog=" << ls->m_uiBackLog << " EXCEEDED"); diff --git a/srtcore/core.cpp b/srtcore/core.cpp index f3812cc4a..d2118c555 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -11924,7 +11924,7 @@ int64_t srt::CUDT::socketStartTime(SRTSOCKET u) if (!s) return APIError(MJ_NOTSUP, MN_SIDINVAL); - return count_microseconds(s->core().m_stats.tsStartTime.time_since_epoch()); + return count_microseconds(s->core().socketStartTime().time_since_epoch()); } bool srt::CUDT::runAcceptHook(CUDT *acore, const sockaddr* peer, const CHandShake& hs, const CPacket& hspkt) diff --git a/srtcore/core.h b/srtcore/core.h index ed250c641..6f7bd6b19 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -747,6 +747,7 @@ class CUDT time_point socketStartTime() { + sync::ScopedLock lk (m_StatsLock); return m_stats.tsStartTime; } diff --git a/srtcore/group.cpp b/srtcore/group.cpp index 4c292cb89..dc3c1a9be 100644 --- a/srtcore/group.cpp +++ b/srtcore/group.cpp @@ -862,17 +862,25 @@ void CUDTGroup::getOpt(SRT_SOCKOPT optname, void* pw_optval, int& w_optlen) { // Can't have m_GroupLock locked while calling getOpt on a member socket // because the call will acquire m_ControlLock leading to a lock-order-inversion. + SRTSOCKET firstsocket = SRT_INVALID_SOCK; enterCS(m_GroupLock); gli_t gi = m_Group.begin(); - CUDTSocket* const ps = (gi != m_Group.end()) ? gi->ps : NULL; - CUDTUnited::SocketKeeper sk(CUDT::uglobal(), ps); + if (gi != m_Group.end()) + firstsocket = gi->ps->core().id(); leaveCS(m_GroupLock); - if (sk.socket) + // CUDTUnited::m_GlobControlLock can't be acquired with m_GroupLock either. + // We have also no guarantee that after leaving m_GroupLock the socket isn't + // going to be deleted. Hence use the safest method by extracting through the id. + if (firstsocket != SRT_INVALID_SOCK) { - // Return the value from the first member socket, if any is present - // Note: Will throw exception if the request is wrong. - sk.socket->core().getOpt(optname, (pw_optval), (w_optlen)); - is_set_on_socket = true; + CUDTUnited::SocketKeeper sk(CUDT::uglobal(), firstsocket); + if (sk.socket) + { + // Return the value from the first member socket, if any is present + // Note: Will throw exception if the request is wrong. + sk.socket->core().getOpt(optname, (pw_optval), (w_optlen)); + is_set_on_socket = true; + } } } diff --git a/test/test_bonding.cpp b/test/test_bonding.cpp index e077ea1e0..6307fd21f 100644 --- a/test/test_bonding.cpp +++ b/test/test_bonding.cpp @@ -1284,7 +1284,7 @@ TEST(Bonding, BackupPrioritySelection) g_nconnected = 0; g_nfailed = 0; - volatile bool recvd = false; + sync::atomic recvd { false }; // 1. sockaddr_in bind_sa; From d6d0e5a964f6f9d5a52acf475d4016bc5c5bb9eb Mon Sep 17 00:00:00 2001 From: cl-ment Date: Fri, 6 Jun 2025 14:32:04 +0200 Subject: [PATCH 04/25] [build] + [core] compilation of SRT on platforms that do not support the pthread_condattr_setclock function. (#3152) * [core],[build] check phtread_condattr_setclock (Android < 21) * Update CMakelists.txt with #3154 to fix travis buildbreak. --------- Co-authored-by: Lilinxiong Zhouliuya --- CMakeLists.txt | 15 +++++++++++++++ srtcore/sync_posix.cpp | 16 +++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 847564d03..a5547454f 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,6 +11,7 @@ cmake_minimum_required (VERSION 3.5 FATAL_ERROR) set (SRT_VERSION 1.5.4) set (CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/scripts") +include(CheckSymbolExists) include(haiUtil) # needed for set_version_variables # CMake version 3.0 introduced the VERSION option of the project() command # to specify a project version as well as the name. @@ -961,8 +962,22 @@ elseif (MICROSOFT) else () find_package(Threads REQUIRED) set(PTHREAD_LIBRARY ${CMAKE_THREAD_LIBS_INIT}) + message(STATUS "Pthread library: ${PTHREAD_LIBRARY}") endif() +# To avoid the need for other judgments when ENABLE_STDCXX_SYNC is OFF in the future, this is a separate conditional statement. +if (NOT ENABLE_STDCXX_SYNC AND ENABLE_MONOTONIC_CLOCK) + list(PREPEND CMAKE_REQUIRED_LIBRARIES "${PTHREAD_LIBRARY}") + unset(CMAKE_REQUIRED_QUIET) + check_symbol_exists(pthread_condattr_setclock "pthread.h" HAVE_PTHREAD_CONDATTR_SETCLOCK) + message(STATUS "Checking pthread_condattr_setclock: '${HAVE_PTHREAD_CONDATTR_SETCLOCK}'") + if ("${HAVE_PTHREAD_CONDATTR_SETCLOCK}" STREQUAL "1") + message(STATUS "FOUND pthread_condattr_setclock - pthread_cond_timedwait will use monotonic clock") + add_definitions(-DHAVE_PTHREAD_CONDATTR_SETCLOCK=1) + else () + message(WARNING "NOT FOUND pthread_cond_timedwait. pthread_cond_timedwait will use system clock. Recommended -DENABLE_STDCXX_SYNC=ON") + endif () +endif () # This is required in some projects that add some other sources # to the SRT library to be compiled together (aka "virtual library"). if (DEFINED SRT_EXTRA_LIB_INC) diff --git a/srtcore/sync_posix.cpp b/srtcore/sync_posix.cpp index 1668a9479..164913097 100644 --- a/srtcore/sync_posix.cpp +++ b/srtcore/sync_posix.cpp @@ -288,13 +288,23 @@ Condition::~Condition() {} void Condition::init() { pthread_condattr_t* attr = NULL; -#if SRT_SYNC_CLOCK == SRT_SYNC_CLOCK_GETTIME_MONOTONIC +#if SRT_SYNC_CLOCK == SRT_SYNC_CLOCK_GETTIME_MONOTONIC && HAVE_PTHREAD_CONDATTR_SETCLOCK pthread_condattr_t CondAttribs; pthread_condattr_init(&CondAttribs); - pthread_condattr_setclock(&CondAttribs, CLOCK_MONOTONIC); + if (pthread_condattr_setclock(&CondAttribs, CLOCK_MONOTONIC) != 0) + { + pthread_condattr_destroy(&CondAttribs); + LOGC(inlog.Fatal, log << "IPE: pthread_condattr_setclock failed to set up a monotonic clock for a CV"); + } attr = &CondAttribs; #endif const int res = pthread_cond_init(&m_cv, attr); +#if SRT_SYNC_CLOCK == SRT_SYNC_CLOCK_GETTIME_MONOTONIC && HAVE_PTHREAD_CONDATTR_SETCLOCK + if (attr != NULL) + { + pthread_condattr_destroy(attr); + } +#endif if (res != 0) throw std::runtime_error("pthread_cond_init monotonic failed"); } @@ -312,7 +322,7 @@ void Condition::wait(UniqueLock& lock) bool Condition::wait_for(UniqueLock& lock, const steady_clock::duration& rel_time) { timespec timeout; -#if SRT_SYNC_CLOCK == SRT_SYNC_CLOCK_GETTIME_MONOTONIC +#if SRT_SYNC_CLOCK == SRT_SYNC_CLOCK_GETTIME_MONOTONIC && HAVE_PTHREAD_CONDATTR_SETCLOCK clock_gettime(CLOCK_MONOTONIC, &timeout); const uint64_t now_us = timeout.tv_sec * uint64_t(1000000) + (timeout.tv_nsec / 1000); #else From 0c570a10e30686552be0f6b9b19ce6a64daed598 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20G=C3=A9rouville?= Date: Wed, 21 May 2025 11:14:16 +0200 Subject: [PATCH 05/25] [build] Remove SonarQube --- .github/workflows/cxx11-ubuntu.yaml | 11 -------- README.md | 4 --- sonar-project.properties | 40 ----------------------------- 3 files changed, 55 deletions(-) delete mode 100644 sonar-project.properties diff --git a/.github/workflows/cxx11-ubuntu.yaml b/.github/workflows/cxx11-ubuntu.yaml index 5566ed801..7d30f4a1a 100644 --- a/.github/workflows/cxx11-ubuntu.yaml +++ b/.github/workflows/cxx11-ubuntu.yaml @@ -10,12 +10,8 @@ jobs: build: name: ubuntu runs-on: ubuntu-20.04 - env: - BUILD_WRAPPER_OUT_DIR: sonar-output # Directory where build-wrapper output will be placed steps: - uses: actions/checkout@v3 - - name: Install sonar-scanner and build-wrapper - uses: sonarsource/sonarcloud-github-c-cpp@v2 - name: configure run: | mkdir _build && cd _build @@ -29,10 +25,3 @@ jobs: run: | source ./scripts/collect-gcov.sh bash <(curl -s https://codecov.io/bash) - - name: Run SonarCloud Scan for C and C++ - if: ${{ !github.event.pull_request.head.repo.fork }} - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - # Consult https://docs.sonarcloud.io/advanced-setup/ci-based-analysis/sonarscanner-cli/ for more information and options. - run: sonar-scanner --define sonar.cfamily.compile-commands="_build/compile_commands.json" diff --git a/README.md b/README.md index b6465a347..0e18ff4a5 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,6 @@ [![License: MPLv2.0][license-badge]](./LICENSE) [![Latest release][release-badge]][github releases] -[![Quality Gate Status][sonarcloud-badge]][sonarcloud-project] [![codecov][codecov-badge]][codecov-project] [![Build Status Linux and macOS][travis-badge]][travis] [![Build Status Windows][appveyor-badge]][appveyor] @@ -226,9 +225,6 @@ By contributing code to the SRT project, you agree to license your contribution [ConanCenter-package]: https://conan.io/center/recipes/srt [ConanCenter-badge]: https://img.shields.io/conan/v/srt -[sonarcloud-project]: https://sonarcloud.io/project/overview?id=srt -[sonarcloud-badge]: https://sonarcloud.io/api/project_badges/measure?project=srt&metric=alert_status - [codecov-project]: https://codecov.io/gh/haivision/srt [codecov-badge]: https://codecov.io/gh/haivision/srt/branch/master/graph/badge.svg diff --git a/sonar-project.properties b/sonar-project.properties deleted file mode 100644 index 4bdaccdb2..000000000 --- a/sonar-project.properties +++ /dev/null @@ -1,40 +0,0 @@ -# must be unique in a given SonarQube instance -sonar.projectKey=srt -sonar.organization=haivision - -# --- optional properties --- - -# This is the name and version displayed in the SonarCloud UI. -#sonar.projectName=srt -#sonar.projectVersion=1.0 - -# Path is relative to the sonar-project.properties file. Replace "\" by "/" on Windows. -#sonar.sources=. - -# Encoding of the source code. Default is default system encoding -#sonar.sourceEncoding=UTF-8 - -# ===================================================== -# Meta-data for the project -# ===================================================== - -sonar.links.homepage=https://github.com/Haivision/srt -sonar.links.scm=https://github.com/Haivision/srt -sonar.links.issue=https://github.com/Haivision/srt/issues - - -# ===================================================== -# Properties that will be shared amongst all modules -# ===================================================== - -# SQ standard properties -sonar.sources=srtcore/,apps/,common/,examples/,haicrypt/,scripts/,testing/ -sonar.tests=test/ - -# Properties specific to the C/C++ analyzer: -#sonar.cfamily.build-wrapper-output=_build/sonar-output -# This field is deprecated. compile-commands should be used -# instead, and this property is set in the action specification -# Should be something like: -#sonar.cfamily.compile-commands=_build/compile_commands.json -sonar.cfamily.gcov.reportsPath=. From 5d80411f0c2df2ebaa2177875309974b0233cc6c Mon Sep 17 00:00:00 2001 From: Sergei Ignatov Date: Thu, 17 Jul 2025 16:33:38 +1000 Subject: [PATCH 06/25] [build] Update iOS build scripts (#3182) * Update iOS build scripts ~ Generate Xcode project files ~ Support build for tvOS ~ Package all variants into XCFramework bundle * Fix iOS / iOS-cxxsyncON --- docs/build/build-android.md | 2 +- docs/build/build-iOS.md | 36 ++++++-------------- scripts/build-ios/README.md | 3 ++ scripts/build-ios/mksrt-xcf.sh | 60 ++++++++++++++++++++++++++++++++++ scripts/build-ios/mkssl-xcf.sh | 22 +++++++++++++ scripts/iOS.cmake | 58 +++++++++++++++++++++++--------- 6 files changed, 139 insertions(+), 42 deletions(-) create mode 100644 scripts/build-ios/README.md create mode 100755 scripts/build-ios/mksrt-xcf.sh create mode 100755 scripts/build-ios/mkssl-xcf.sh diff --git a/docs/build/build-android.md b/docs/build/build-android.md index 7c13c9404..da3541407 100644 --- a/docs/build/build-android.md +++ b/docs/build/build-android.md @@ -1,6 +1,6 @@ # Building SRT on Android -**NOTE:** The scripts have been moved to [scripts/build-android](../../scripts/build-android/) folder. +**NOTE:** The scripts are located in [scripts/build-android](../../scripts/build-android/) folder. ## Install the NDK and CMake diff --git a/docs/build/build-iOS.md b/docs/build/build-iOS.md index a1fd48ba9..c6d58844f 100644 --- a/docs/build/build-iOS.md +++ b/docs/build/build-iOS.md @@ -1,44 +1,28 @@ -# Building SRT on iOS +# Building SRT on iOS/tvOS + +**NOTE:** The scripts are located in [scripts/build-ios](../../scripts/build-ios/) folder. ## Prerequisites * Xcode should be installed. Check in terminal whether `xcode-select -p` points to **/Applications/Xcode.app/Contents/Developer** * Install Homebrew according to instructions on [https://brew.sh/](https://brew.sh/) -* Install CMake and pkg-config with Homebrew: +* Install CMake with Homebrew: ``` brew install cmake -brew install pkg-config ``` ## Building OpenSSL -There is [OpenSSL for iPhone](https://github.com/x2on/OpenSSL-for-iPhone) project which have all necessary to build OpenSSL for our needs. It fetches OpenSSL code by itself, so you don't need to download it separately. So simply clone it and build with command: +There is [OpenSSL-Universal](https://github.com/krzyzanowskim/OpenSSL) project which has all necessary to build OpenSSL for our needs. It fetches OpenSSL code by itself, so you don't need to download it separately. Build it with command: ``` -./build-libssl.sh --archs="arm64" +./mkssl-xcf.sh ``` -Results (both libraries and headers) will be placed in bin/<SDK_VERSION>-<ARCH>.sdk directory (for example, *bin/iPhoneOS11.2-arm64.sdk*). We assume you set **IOS_OPENSSL** variable to this path (e.g. `export IOS_OPENSSL="/Users/johndoe/sources/OpenSSL-for-iPhone/bin/iPhoneOS11.2-arm64.sdk"`). - ## Building SRT code -Now you can build SRT providing path to OpenSSL library and toolchain file for iOS - +Now you can build SRT with command: ``` -./configure --cmake-prefix-path=$IOS_OPENSSL --use-openssl-pc=OFF --cmake-toolchain-file=scripts/iOS.cmake -make +./mksrt-xcf.sh ``` -Optionally you may add following iOS-specifc settings to configure: - -* `--ios-disable-bitcode=1` - disable embedding bitcode to library. -* `--ios-arch=armv7|armv7s|arm64` - specify if you want to build for specific architecture (arm64 by default) -* `--ios-platform=OS|SIMULATOR|SIMULATOR64` - specify for build simulator code -* `--cmake-ios-developer-root=<path>`- specify path for platform directory; {XCODE_ROOT}/Platforms/iPhoneOS.platform/Developer by default -* `--cmake-ios-sdk-root=` - by default searches for latest SDK version within {CMAKE_IOS_DEVELOPER_ROOT}/SDKs, set if you want to use another SDK version - -Note that resulting .dylib file has install path @executable_path/Frameworks/libsrt.1.dylib, so if you need to place it in some other place with your application, you may change it with *install_name_tool* command: ``install_name_tool -id "" ``, for example ``install_name_tool -id "@executable_path/Frameworks/libsrt.1.3.0.dylib" libsrt.1.3.0.dylib`` - ## Adding to Xcode project -In Xcode project settings in General tab, add libsrt to **Linked Frameworks and Libraries** section - click Plus sign, then click "Add Other" and find libsrt.1.dylib - -Click plus sign in **Embedded binaries** section and choose Frameworks/libsrt.1.dylib +Results (libcrypto.xcframework and libsrt.xcframework) will be placed in [scripts/build-ios](../../scripts/build-ios/) folder. -In **Build settings** tab find **Header Search Paths** setting -and add paths to SRT library sources (you should add srt, srt/common and srt/common directories). +Follow [these steps](https://developer.apple.com/documentation/xcode/creating-a-static-framework#Embed-your-framework-in-an-app) to embed frameworks in your app. Choose the Do Not Embed option from the Embed value list. diff --git a/scripts/build-ios/README.md b/scripts/build-ios/README.md new file mode 100644 index 000000000..63986286e --- /dev/null +++ b/scripts/build-ios/README.md @@ -0,0 +1,3 @@ +## Scripts for building SRT for iOS/tvOS + +See [Building SRT for iOS/tvOS](../../docs/build/build-iOS.md) for the instructions. diff --git a/scripts/build-ios/mksrt-xcf.sh b/scripts/build-ios/mksrt-xcf.sh new file mode 100755 index 000000000..59022cdc1 --- /dev/null +++ b/scripts/build-ios/mksrt-xcf.sh @@ -0,0 +1,60 @@ +#!/bin/zsh + +# Determine the path of the executing script +BASE_DIR=$(readlink -f $0 | xargs dirname) + +srt="../.." +ssl=$BASE_DIR/OpenSSL +out=$BASE_DIR/libsrt +target=13.0 +tv_target=14.0 +tvos_sdk=`xcrun -sdk appletvos --show-sdk-version` +lib=libsrt.a + +ios_archs=('arm64' 'arm64' 'x86_64' 'arm64' 'arm64' 'x86_64') +ios_platforms=('OS' 'ARM_SIMULATOR' 'SIMULATOR64' 'TV' 'TV_ARM_SIMULATOR' 'TV_SIMULATOR' ) +ios_targets=('iOS-arm64' 'iOS-simArm' 'iOS-simIntel64' 'tvOS' 'tvOS_simArm' 'tvOS_simIntel64') +ssl_folders=('iphoneos' 'iphonesimulator' 'iphonesimulator' 'appletvos' 'appletvsimulator' 'appletvsimulator') +rm -rf $out + +cd $srt + +for idx in {1..$#ios_archs} +do + ios_arch=${ios_archs[$idx]} + ios_platform=${ios_platforms[$idx]} + dest=${ios_targets[$idx]} + ssl_path=${ssl}/${ssl_folders[$idx]} + + git clean -fd -e scripts + echo '********************************************************************************' + echo SSL_PATH $ssl_path ARCH $ios_arch PLATFORM $ios_platform + + CMAKE_GENERATOR=Xcode ./configure \ + --enable-shared=OFF --enable-apps=OFF \ + --cmake-toolchain-file=scripts/iOS.cmake --ios-arch=$ios_arch --ios-platform=$ios_platform \ + --cmake-xcode-attribute-iphoneos-deployment-target=$target \ + --cmake-xcode-attribute-tvos-deployment-target=$tv_target \ + --cmake-prefix-path=$ssl_path --use-openssl-pc=OFF \ + --cmake_install_prefix=$out/$dest + + cmake --build . --target install --config Release + echo '********************************************************************************' +done + +git clean -fd -e scripts + +lipo -create $out/iOS-simArm/lib/$lib $out/iOS-simIntel64/lib/$lib -output $out/libsrt-ios-sim.a +lipo -create $out/tvOS_simArm/lib/$lib $out/tvOS_simIntel64/lib/$lib -output $out/libsrt-tv-sim.a + +srt_headers=$out/iOS-arm64/include +rm -rf $BASE_DIR/libsrt.xcframework + +echo '####### create XCFramework bundle #######' + +xcodebuild -create-xcframework \ + -library $out/iOS-arm64/lib/$lib -headers $srt_headers \ + -library $out/libsrt-ios-sim.a -headers $srt_headers \ + -library $out/tvOS/lib/$lib -headers $srt_headers \ + -library $out/libsrt-tv-sim.a -headers $srt_headers \ + -output $BASE_DIR/libsrt.xcframework diff --git a/scripts/build-ios/mkssl-xcf.sh b/scripts/build-ios/mkssl-xcf.sh new file mode 100755 index 000000000..c7283d75d --- /dev/null +++ b/scripts/build-ios/mkssl-xcf.sh @@ -0,0 +1,22 @@ +#!/bin/zsh + +# Determine the path of the executing script +BASE_DIR=$(readlink -f $0 | xargs dirname) + +REPO_DIR=OpenSSL +SSL_DIR=$BASE_DIR/$REPO_DIR + +if [ ! -d $REPO_DIR ] +then + git clone https://github.com/krzyzanowskim/OpenSSL +fi +cd $REPO_DIR +make build + +rm -rf $BASE_DIR/libcrypto.xcframework +xcodebuild -create-xcframework \ + -library $SSL_DIR/iphoneos/lib/libcrypto.a \ + -library $SSL_DIR/iphonesimulator/lib/libcrypto.a \ + -library $SSL_DIR/appletvos/lib/libcrypto.a \ + -library $SSL_DIR/appletvsimulator/lib/libcrypto.a \ + -output $BASE_DIR/libcrypto.xcframework diff --git a/scripts/iOS.cmake b/scripts/iOS.cmake index 1f2a04b50..ed3743dd6 100644 --- a/scripts/iOS.cmake +++ b/scripts/iOS.cmake @@ -8,8 +8,10 @@ # This decides if SDKS will be selected from the iPhoneOS.platform or iPhoneSimulator.platform folders # OS - the default, used to build for iPhone and iPad physical devices, which have an arm arch. # SIMULATOR - used to build for the Simulator platforms, which have an x86 arch. +# TV - used to build for tvOS devices +# TV_SIMULATOR - used to build for tvOS Simulator platforms, which have an x86 arch. # -# IOS_ARCH = arm64 (default for OS), armv7, armv7s, i386 (default for SIMULATOR), x86_64 (default for SIMULATOR64) +# IOS_ARCH = arm64 (default for OS and TV), armv7, armv7s, i386 (default for SIMULATOR), x86_64 (default for SIMULATOR64 and TV_SIMULATOR) # # CMAKE_IOS_DEVELOPER_ROOT = automatic(default) or /path/to/platform/Developer folder # By default this location is automatcially chosen based on the IOS_PLATFORM value above. @@ -19,8 +21,6 @@ # By default this location is automatcially chosen based on the CMAKE_IOS_DEVELOPER_ROOT value. # In this case it will always be the most up-to-date SDK found in the CMAKE_IOS_DEVELOPER_ROOT path. # If set manually, this will force the use of a specific SDK version -# -# IOS_DISABLE_BITCODE - set to 1 if you want to disable bitcode generation # Standard settings set (CMAKE_SYSTEM_NAME Darwin) @@ -29,14 +29,16 @@ set (UNIX True) set (APPLE True) set (IOS True) +# With Xcode 16, Apple has deprecated bitcode for all platforms +set(CMAKE_XCODE_ATTRIBUTE_ENABLE_BITCODE "NO") + # Required as of cmake 2.8.10 set (CMAKE_OSX_DEPLOYMENT_TARGET "" CACHE STRING "Force unset of the deployment target for iOS" FORCE) # Determine the cmake host system version so we know where to find the iOS SDKs find_program (CMAKE_UNAME uname /bin /usr/bin /usr/local/bin) if (CMAKE_UNAME) - execute_process(COMMAND uname -r - OUTPUT_VARIABLE CMAKE_HOST_SYSTEM_VERSION) + execute_process(COMMAND uname -r OUTPUT_VARIABLE CMAKE_HOST_SYSTEM_VERSION) string (REGEX REPLACE "^([0-9]+)\\.([0-9]+).*$" "\\1" DARWIN_MAJOR_VERSION "${CMAKE_HOST_SYSTEM_VERSION}") endif (CMAKE_UNAME) @@ -49,10 +51,6 @@ set (CMAKE_C_OSX_CURRENT_VERSION_FLAG "-current_version ") set (CMAKE_CXX_OSX_COMPATIBILITY_VERSION_FLAG "${CMAKE_C_OSX_COMPATIBILITY_VERSION_FLAG}") set (CMAKE_CXX_OSX_CURRENT_VERSION_FLAG "${CMAKE_C_OSX_CURRENT_VERSION_FLAG}") -if (NOT DEFINED IOS_DISABLE_BITCODE) - set (EMBED_OPTIONS "-fembed-bitcode") -endif(NOT DEFINED IOS_DISABLE_BITCODE) - if (CMAKE_BUILD_TYPE STREQUAL "Debug" OR ENABLE_DEBUG) set(IOS_DEBUG_OPTIONS "-glldb -gmodules") else() @@ -102,6 +100,12 @@ if (${IOS_PLATFORM} STREQUAL OS) # This causes the installers to properly locate the output libraries set (CMAKE_XCODE_EFFECTIVE_PLATFORMS "-iphoneos") +elseif (${IOS_PLATFORM} STREQUAL TV) + set (IOS_PLATFORM_LOCATION "AppleTVOS.platform") + unset(CMAKE_IOS_DEVELOPER_ROOT CACHE) + unset(CMAKE_IOS_SDK_ROOT CACHE) + # This causes the installers to properly locate the output libraries + set (CMAKE_XCODE_EFFECTIVE_PLATFORMS "-appletvos") elseif (${IOS_PLATFORM} STREQUAL SIMULATOR) set (SIMULATOR true) set (IOS_PLATFORM_LOCATION "iPhoneSimulator.platform") @@ -114,15 +118,35 @@ elseif (${IOS_PLATFORM} STREQUAL SIMULATOR64) # This causes the installers to properly locate the output libraries set (CMAKE_XCODE_EFFECTIVE_PLATFORMS "-iphonesimulator") +elseif (${IOS_PLATFORM} STREQUAL ARM_SIMULATOR) + set (SIMULATOR true) + set (IOS_PLATFORM_LOCATION "iPhoneSimulator.platform") + + # This causes the installers to properly locate the output libraries + set (CMAKE_XCODE_EFFECTIVE_PLATFORMS "-iphonesimulator") +elseif (${IOS_PLATFORM} STREQUAL TV_SIMULATOR) + set (SIMULATOR true) + set (IOS_PLATFORM_LOCATION "AppleTVSimulator.platform") + unset(CMAKE_IOS_DEVELOPER_ROOT CACHE) + unset(CMAKE_IOS_SDK_ROOT CACHE) + + # This causes the installers to properly locate the output libraries + set (CMAKE_XCODE_EFFECTIVE_PLATFORMS "-appletvsimulator") +elseif (${IOS_PLATFORM} STREQUAL TV_ARM_SIMULATOR) + set (SIMULATOR true) + set (IOS_PLATFORM_LOCATION "AppleTVSimulator.platform") + unset(CMAKE_IOS_DEVELOPER_ROOT CACHE) + unset(CMAKE_IOS_SDK_ROOT CACHE) + + # This causes the installers to properly locate the output libraries + set (CMAKE_XCODE_EFFECTIVE_PLATFORMS "-appletvsimulator") else (${IOS_PLATFORM} STREQUAL OS) - message (FATAL_ERROR "Unsupported IOS_PLATFORM value selected. Please choose OS or SIMULATOR") + message (FATAL_ERROR "Unsupported IOS_PLATFORM value selected. Please choose OS, TV or SIMULATOR") endif (${IOS_PLATFORM} STREQUAL OS) # Setup iOS developer location unless specified manually with CMAKE_IOS_DEVELOPER_ROOT if (NOT DEFINED CMAKE_IOS_DEVELOPER_ROOT) - execute_process(COMMAND /usr/bin/xcode-select -print-path - OUTPUT_VARIABLE CMAKE_XCODE_DEVELOPER_DIR) - string(STRIP "${CMAKE_XCODE_DEVELOPER_DIR}" CMAKE_XCODE_DEVELOPER_DIR) # FIXED: remove new line character, otherwise it complain no iOS SDK's found in default search path + execute_process(COMMAND /usr/bin/xcode-select -print-path OUTPUT_VARIABLE CMAKE_XCODE_DEVELOPER_DIR OUTPUT_STRIP_TRAILING_WHITESPACE) set (CMAKE_IOS_DEVELOPER_ROOT "${CMAKE_XCODE_DEVELOPER_DIR}/Platforms/${IOS_PLATFORM_LOCATION}/Developer") endif (NOT DEFINED CMAKE_IOS_DEVELOPER_ROOT) set (CMAKE_IOS_DEVELOPER_ROOT ${CMAKE_IOS_DEVELOPER_ROOT} CACHE PATH "Location of iOS Platform") @@ -152,6 +176,10 @@ if (NOT DEFINED IOS_ARCH) set (IOS_ARCH i386) elseif (${IOS_PLATFORM} STREQUAL SIMULATOR64) set (IOS_ARCH x86_64) + elseif (${IOS_PLATFORM} STREQUAL ARM_SIMULATOR) + set (IOS_ARCH arm64) + elseif (${IOS_PLATFORM} STREQUAL TV_ARM_SIMULATOR) + set (IOS_ARCH arm64) endif (${IOS_PLATFORM} STREQUAL OS) endif(NOT DEFINED IOS_ARCH) set (CMAKE_OSX_ARCHITECTURES ${IOS_ARCH} CACHE STRING "Build architecture for iOS") @@ -169,8 +197,8 @@ set (CMAKE_SYSTEM_FRAMEWORK_PATH ${CMAKE_IOS_SDK_ROOT}/Developer/Library/Frameworks ) -# only search the iOS sdks, not the remainder of the host filesystem (except for programs, so that we can still find Python if needed) -set (CMAKE_FIND_ROOT_PATH_MODE_PROGRAM BOTH) +# only search the iOS sdks, not the remainder of the host filesystem +set (CMAKE_FIND_ROOT_PATH_MODE_PROGRAM ONLY) set (CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY) set (CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY) From e9160f9228fdea4629e25a2f94b3a2ec2669b764 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Tue, 19 Aug 2025 11:45:07 +0200 Subject: [PATCH 07/25] [MAINT] Added optional test discovery. Added improvements for attributes. Enabled std shared mutex if compiling with C++17 (#3190) * [MAINT] Added optional test discovery. Added improvements for attributes * Enabled C++17 version of shared mutex when compiling in C++17 mode * Added UT discovery option to configure * Changed enabled std::shared_mutex only if compiling with stdc++-sync * Fixed problem with not updated sync.cpp SharedMutex * Fixed review findings --------- Co-authored-by: Mikolaj Malecki --- CMakeLists.txt | 5 +++- configure-data.tcl | 1 + srtcore/srt.h | 4 +++ srtcore/srt_attr_defs.h | 61 +++++++++++++++++++++-------------------- srtcore/sync.cpp | 9 +++++- srtcore/sync.h | 12 ++++++-- test/test_sync.cpp | 3 ++ 7 files changed, 62 insertions(+), 33 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a5547454f..0acd39b41 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -164,6 +164,7 @@ option(ENABLE_PKTINFO "Enable using IP_PKTINFO to allow the listener extracting option(ENABLE_RELATIVE_LIBPATH "Should application contain relative library paths, like ../lib" OFF) option(ENABLE_GETNAMEINFO "In-logs sockaddr-to-string should do rev-dns" OFF) option(ENABLE_UNITTESTS "Enable unit tests" OFF) +option(ENABLE_UNITTESTS_DISCOVERY "Do unit test discovery when unit tests enabled" ON) option(ENABLE_ENCRYPTION "Enable encryption in SRT" ON) option(ENABLE_AEAD_API_PREVIEW "Enable AEAD API preview in SRT" Off) option(ENABLE_MAXREXMITBW "Enable SRTO_MAXREXMITBW (v1.6.0 API preview)" Off) @@ -1585,7 +1586,9 @@ if (ENABLE_UNITTESTS AND ENABLE_CXX11) #set_tests_properties(test-srt PROPERTIES RUN_SERIAL TRUE) else() set_tests_properties(${tests_srt} PROPERTIES RUN_SERIAL TRUE) - gtest_discover_tests(test-srt) + if (ENABLE_UNITTESTS_DISCOVERY) + gtest_discover_tests(test-srt) + endif() endif() enable_testing() diff --git a/configure-data.tcl b/configure-data.tcl index 3510a9645..d7b9d4937 100644 --- a/configure-data.tcl +++ b/configure-data.tcl @@ -49,6 +49,7 @@ set cmake_options { enable-relative-libpath "Should applications contain relative library paths, like ../lib (default: OFF)" enable-getnameinfo "In-logs sockaddr-to-string should do rev-dns (default: OFF)" enable-unittests "Enable Unit Tests (will download Google UT) (default: OFF)" + enable-unittests-discovery "Enable UT Discovery (will run when building) (default: ON)" enable-encryption "Should encryption features be enabled (default: ON)" enable-c++-deps "Extra library dependencies in srt.pc for C language (default: ON)" use-static-libstdc++ "Should use static rather than shared libstdc++ (default: OFF)" diff --git a/srtcore/srt.h b/srtcore/srt.h index 71ac2c3af..2d8ba3997 100644 --- a/srtcore/srt.h +++ b/srtcore/srt.h @@ -109,17 +109,21 @@ written by #define SRT_ATR_DEPRECATED #define SRT_ATR_DEPRECATED_PX [[deprecated]] +#define SRT_ATR_NODISCARD [[nodiscard]] // GNUG is GNU C/C++; this syntax is also supported by Clang #elif defined(__GNUC__) #define SRT_ATR_DEPRECATED_PX #define SRT_ATR_DEPRECATED __attribute__((deprecated)) +#define SRT_ATR_NODISCARD __attribute__((warn_unused_result)) #elif defined(_MSC_VER) #define SRT_ATR_DEPRECATED_PX __declspec(deprecated) #define SRT_ATR_DEPRECATED // no postfix-type modifier +#define SRT_ATR_NODISCARD _Check_return_ #else #define SRT_ATR_DEPRECATED_PX #define SRT_ATR_DEPRECATED +#define SRT_ATR_NODISCARD #endif #ifdef __cplusplus diff --git a/srtcore/srt_attr_defs.h b/srtcore/srt_attr_defs.h index 726c4a03b..1c489658f 100644 --- a/srtcore/srt_attr_defs.h +++ b/srtcore/srt_attr_defs.h @@ -17,26 +17,17 @@ used by SRT library internally. // ATTRIBUTES: // -// SRT_ATR_UNUSED: declare an entity ALLOWED to be unused (prevents warnings) -// ATR_DEPRECATED: declare an entity deprecated (compiler should warn when used) // ATR_NOEXCEPT: The true `noexcept` from C++11, or nothing if compiling in pre-C++11 mode // ATR_NOTHROW: In C++11: `noexcept`. In pre-C++11: `throw()`. Required for GNU libstdc++. // ATR_CONSTEXPR: In C++11: `constexpr`. Otherwise empty. // ATR_OVERRIDE: In C++11: `override`. Otherwise empty. // ATR_FINAL: In C++11: `final`. Otherwise empty. -#ifdef __GNUG__ -#define ATR_DEPRECATED __attribute__((deprecated)) -#else -#define ATR_DEPRECATED -#endif -#if HAVE_CXX11 -#define SRT_ATR_ALIGNAS(n) alignas(n) -#elif HAVE_GCC -#define SRT_ATR_ALIGNAS(n) __attribute__((aligned(n))) +#ifdef __GNUG__ +#define HAVE_GCC 1 #else -#define SRT_ATR_ALIGNAS(n) +#define HAVE_GCC 0 #endif #if defined(__cplusplus) && __cplusplus > 199711L @@ -44,18 +35,15 @@ used by SRT library internally. // For gcc 4.7, claim C++11 is supported, as long as experimental C++0x is on, // however it's only the "most required C++11 support". #if defined(__GXX_EXPERIMENTAL_CXX0X__) && __GNUC__ == 4 && __GNUC_MINOR__ >= 7 // 4.7 only! -#define ATR_NOEXCEPT -#define ATR_NOTHROW throw() -#define ATR_CONSTEXPR -#define ATR_OVERRIDE -#define ATR_FINAL #else #define HAVE_FULL_CXX11 1 -#define ATR_NOEXCEPT noexcept -#define ATR_NOTHROW noexcept -#define ATR_CONSTEXPR constexpr -#define ATR_OVERRIDE override -#define ATR_FINAL final + +#if __cplusplus >= 201703L +#define HAVE_CXX17 1 +#else +#define HAVE_CXX17 0 +#endif + #endif #elif defined(_MSC_VER) && _MSC_VER >= 1800 // Microsoft Visual Studio supports C++11, but not fully, @@ -65,26 +53,41 @@ used by SRT library internally. #define HAVE_CXX11 1 #if defined(_MSC_FULL_VER) && _MSC_FULL_VER >= 190023026 #define HAVE_FULL_CXX11 1 + +#if __cplusplus >= 201703L +#define HAVE_CXX17 1 +#else +#define HAVE_CXX17 0 +#endif + +#endif +#else +#define HAVE_CXX11 0 +#define HAVE_CXX17 0 +#endif // __cplusplus + +#if HAVE_FULL_CXX11 #define ATR_NOEXCEPT noexcept #define ATR_NOTHROW noexcept #define ATR_CONSTEXPR constexpr #define ATR_OVERRIDE override #define ATR_FINAL final #else +// These are both for HAVE_CXX11 == 1 and 0. #define ATR_NOEXCEPT #define ATR_NOTHROW throw() #define ATR_CONSTEXPR #define ATR_OVERRIDE #define ATR_FINAL #endif + +#if HAVE_CXX11 +#define SRT_ATR_ALIGNAS(n) alignas(n) +#elif HAVE_GCC +#define SRT_ATR_ALIGNAS(n) __attribute__((aligned(n))) #else -#define HAVE_CXX11 0 -#define ATR_NOEXCEPT -#define ATR_NOTHROW throw() -#define ATR_CONSTEXPR -#define ATR_OVERRIDE -#define ATR_FINAL -#endif // __cplusplus +#define SRT_ATR_ALIGNAS(n) +#endif #if !HAVE_CXX11 && defined(REQUIRE_CXX11) && REQUIRE_CXX11 == 1 #error "The currently compiled application required C++11, but your compiler doesn't support it." diff --git a/srtcore/sync.cpp b/srtcore/sync.cpp index bfe153657..96cb1a9d9 100644 --- a/srtcore/sync.cpp +++ b/srtcore/sync.cpp @@ -357,6 +357,11 @@ int srt::sync::genRandomInt(int minVal, int maxVal) #endif // HAVE_CXX11 } +#if defined(ENABLE_STDCXX_SYNC) && HAVE_CXX17 + +// Shared mutex imp not required - aliased from C++17 + +#else //////////////////////////////////////////////////////////////////////////////// // @@ -451,4 +456,6 @@ int srt::sync::SharedMutex::getReaderCount() const { ScopedLock lk(m_Mutex); return m_iCountRead; -} \ No newline at end of file +} +#endif // C++17 for shared_mutex + diff --git a/srtcore/sync.h b/srtcore/sync.h index 45285828c..7e11c29cf 100644 --- a/srtcore/sync.h +++ b/srtcore/sync.h @@ -12,6 +12,7 @@ #define INC_SRT_SYNC_H #include "platform_sys.h" +#include "srt_attr_defs.h" #include #include @@ -21,6 +22,9 @@ #include #include #include +#if HAVE_CXX17 +#include +#endif #define SRT_SYNC_CLOCK SRT_SYNC_CLOCK_STDCXX_STEADY #define SRT_SYNC_CLOCK_STR "STDCXX_STEADY" #else @@ -54,7 +58,6 @@ #include "srt.h" #include "utilities.h" -#include "srt_attr_defs.h" namespace srt @@ -491,11 +494,15 @@ inline void releaseCond(Condition& cv) { cv.destroy(); } // /////////////////////////////////////////////////////////////////////////////// +#if defined(ENABLE_STDCXX_SYNC) && HAVE_CXX17 +using SharedMutex = std::shared_mutex; +#else + /// Implementation of a read-write mutex. /// This allows multiple readers at a time, or a single writer. /// TODO: The class can be improved if needed to give writer a preference /// by adding additional m_iWritersWaiting member variable (counter). -/// TODO: The m_iCountRead could be made atomic to make unlok_shared() faster and lock-free. +/// TODO: The m_iCountRead could be made atomic to make unlock_shared() faster and lock-free. class SharedMutex { public: @@ -526,6 +533,7 @@ class SharedMutex int m_iCountRead; bool m_bWriterLocked; }; +#endif /// A version of std::scoped_lock (or lock_guard for C++11). /// We could have used the srt::sync::ScopedLock making it a template-based class. diff --git a/test/test_sync.cpp b/test/test_sync.cpp index d06b5e5f5..9fb1b3db3 100644 --- a/test/test_sync.cpp +++ b/test/test_sync.cpp @@ -609,6 +609,8 @@ TEST(SyncThread, Joinable) EXPECT_FALSE(foo.joinable()); } +#if !HAVE_CXX17 + /*****************************************************************************/ /* * SharedMutex @@ -693,6 +695,7 @@ TEST(SharedMutex, LockedReadCount) EXPECT_TRUE(mut.try_lock()); } +#endif /*****************************************************************************/ /* From 7497397023cddd72a54ea9ed79b60e96f1085640 Mon Sep 17 00:00:00 2001 From: Mikolaj Malecki Date: Tue, 19 Aug 2025 18:39:37 +0200 Subject: [PATCH 08/25] [core] Fixed bug in srt_epoll_uwait: wrong number of sockets returned --- srtcore/epoll.cpp | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/srtcore/epoll.cpp b/srtcore/epoll.cpp index 8cd8440c7..c8c3c74b7 100644 --- a/srtcore/epoll.cpp +++ b/srtcore/epoll.cpp @@ -522,22 +522,17 @@ int srt::CEPoll::uwait(const int eid, SRT_EPOLL_EVENT* fdsSet, int fdsSize, int6 throw CUDTException(MJ_NOTSUP, MN_INVAL); } - int total = 0; // This is a list, so count it during iteration - CEPollDesc::enotice_t::iterator i = ed.enotice_begin(); - while (i != ed.enotice_end()) + CEPollDesc::enotice_t::iterator i = ed.enotice_begin(), inext; + int pos = 0; // This is a list, so count it during iteration + for (inext = i ; i != ed.enotice_end() && pos < fdsSize ; ++pos, i = inext) { - int pos = total; // previous past-the-end position - ++total; - - if (total > fdsSize) - break; + ++inext; // deletion-safe list loop fdsSet[pos] = *i; - - ed.checkEdge(i++); // NOTE: potentially deletes `i` + ed.checkEdge(i); // NOTE: potentially deletes `i` } - if (total) - return total; + if (pos) // pos is increased by 1 towards the last used position + return pos; } if ((msTimeOut >= 0) && (count_microseconds(srt::sync::steady_clock::now() - entertime) >= msTimeOut * int64_t(1000))) From d720b8c9a47c265f60eaf5a7bf6a1990423d039f Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Tue, 26 Aug 2025 09:36:12 +0200 Subject: [PATCH 09/25] [core] Rework m_bListening to lock-free and m_GlobControlLock to use shared mutex (#3189) * [MAINT] Added optional test discovery. Added improvements for attributes * Enabled C++17 version of shared mutex when compiling in C++17 mode * [core] Rework m_bListening to lock-free and m_GlobControlLock to use shared mutex * Added UT discovery option to configure * Changed enabled std::shared_mutex only if compiling with stdc++-sync * Fixed problem with not updated sync.cpp SharedMutex * Removed dead code * Fixed review findings * Demoted some locks on m_GlobControlLock to shared. Added false positive TSan info * Merged changes from #3200 * Added a fix that should repair the problem reported in 3196. Still needs rechecks * Fixed incorrect shared lock applied for a case when a new group is to be created (required exclusive) --------- Co-authored-by: Mikolaj Malecki --- srtcore/api.cpp | 65 +++++++++------ srtcore/api.h | 2 +- srtcore/core.cpp | 200 +++++++++++++++++++++++++++++----------------- srtcore/core.h | 1 - srtcore/group.cpp | 6 +- srtcore/queue.cpp | 22 +++-- srtcore/queue.h | 5 +- srtcore/sync.h | 26 +++--- 8 files changed, 206 insertions(+), 121 deletions(-) diff --git a/srtcore/api.cpp b/srtcore/api.cpp index 14d7a4652..da929ef5a 100644 --- a/srtcore/api.cpp +++ b/srtcore/api.cpp @@ -281,7 +281,8 @@ void srt::CUDTUnited::closeAllSockets() HLOGC(inlog.Debug, log << "GC: GLOBAL EXIT - releasing all pending sockets. Acquring control lock..."); { - ScopedLock glock(m_GlobControlLock); + // Pre-closing: run over all open sockets and close them. + SharedLock glock(m_GlobControlLock); for (sockets_t::iterator i = m_Sockets.begin(); i != m_Sockets.end(); ++i) { @@ -297,6 +298,16 @@ void srt::CUDTUnited::closeAllSockets() s->removeFromGroup(false); } #endif + } + } + + { + ExclusiveLock glock(m_GlobControlLock); + + for (sockets_t::iterator i = m_Sockets.begin(); i != m_Sockets.end(); ++i) + { + CUDTSocket* s = i->second; + m_ClosedSockets[i->first] = s; // remove from listener's queue @@ -518,7 +529,7 @@ SRTSOCKET srt::CUDTUnited::newSocket(CUDTSocket** pps) HLOGC(smlog.Debug, log << CONID(ns->m_SocketID) << "newSocket: mapping socket " << ns->m_SocketID); // protect the m_Sockets structure. - ScopedLock cs(m_GlobControlLock); + ExclusiveLock cs(m_GlobControlLock); m_Sockets[ns->m_SocketID] = ns; } catch (...) @@ -536,6 +547,12 @@ SRTSOCKET srt::CUDTUnited::newSocket(CUDTSocket** pps) return ns->m_SocketID; } +// XXX NOTE: TSan reports here false positive against the call +// to CRcvQueue::removeListener. This here will apply shared +// lock on m_GlobControlLock in the call of locateSocket, while +// having applied a shared lock on CRcvQueue::m_pListener in +// CRcvQueue::worker_ProcessConnectionRequest. As this thread +// locks both mutexes as shared, it doesn't form a deadlock. int srt::CUDTUnited::newConnection(const SRTSOCKET listen, const sockaddr_any& peer, const CPacket& hspkt, @@ -675,7 +692,7 @@ int srt::CUDTUnited::newConnection(const SRTSOCKET listen, HLOGC(cnlog.Debug, log << "newConnection: incoming " << peer.str() << ", mapping socket " << ns->m_SocketID); { - ScopedLock cg(m_GlobControlLock); + ExclusiveLock cg(m_GlobControlLock); m_Sockets[ns->m_SocketID] = ns; } @@ -724,7 +741,7 @@ int srt::CUDTUnited::newConnection(const SRTSOCKET listen, { // protect the m_PeerRec structure (and group existence) - ScopedLock glock(m_GlobControlLock); + ExclusiveLock glock(m_GlobControlLock); try { HLOGC(cnlog.Debug, log << "newConnection: mapping peer " << ns->m_PeerID @@ -898,7 +915,7 @@ int srt::CUDTUnited::newConnection(const SRTSOCKET listen, // connect() in UDT code) may fail, in which case this socket should not be // further processed and should be removed. { - ScopedLock cg(m_GlobControlLock); + ExclusiveLock cg(m_GlobControlLock); #if ENABLE_BONDING if (ns->m_GroupOf) @@ -973,7 +990,7 @@ int srt::CUDTUnited::installConnectHook(const SRTSOCKET u, srt_connect_callback_ SRT_SOCKSTATUS srt::CUDTUnited::getStatus(const SRTSOCKET u) { // protects the m_Sockets structure - ScopedLock cg(m_GlobControlLock); + SharedLock cg(m_GlobControlLock); sockets_t::const_iterator i = m_Sockets.find(u); @@ -1247,7 +1264,7 @@ SRTSOCKET srt::CUDTUnited::accept(const SRTSOCKET listen, sockaddr* pw_addr, int { // Put a lock to protect the group against accidental deletion // in the meantime. - ScopedLock glock(m_GlobControlLock); + SharedLock glock(m_GlobControlLock); // Check again; it's unlikely to happen, but // it's a theoretically possible scenario if (s->m_GroupOf) @@ -1527,7 +1544,7 @@ int srt::CUDTUnited::groupConnect(CUDTGroup* pg, SRT_SOCKGROUPCONFIG* targets, i } { - ScopedLock cs(m_GlobControlLock); + ExclusiveLock cs(m_GlobControlLock); if (m_Sockets.count(sid) == 0) { HLOGC(aclog.Debug, log << "srt_connect_group: socket @" << sid << " deleted in process"); @@ -1633,7 +1650,7 @@ int srt::CUDTUnited::groupConnect(CUDTGroup* pg, SRT_SOCKGROUPCONFIG* targets, i targets[tii].errorcode = e.getErrorCode(); targets[tii].id = CUDT::INVALID_SOCK; - ScopedLock cl(m_GlobControlLock); + ExclusiveLock cl(m_GlobControlLock); ns->removeFromGroup(false); m_Sockets.erase(ns->m_SocketID); // Intercept to delete the socket on failure. @@ -1645,7 +1662,7 @@ int srt::CUDTUnited::groupConnect(CUDTGroup* pg, SRT_SOCKGROUPCONFIG* targets, i LOGC(aclog.Fatal, log << "groupConnect: IPE: UNKNOWN EXCEPTION from connectIn"); targets[tii].errorcode = SRT_ESYSOBJ; targets[tii].id = CUDT::INVALID_SOCK; - ScopedLock cl(m_GlobControlLock); + ExclusiveLock cl(m_GlobControlLock); ns->removeFromGroup(false); m_Sockets.erase(ns->m_SocketID); // Intercept to delete the socket on failure. @@ -2019,7 +2036,7 @@ void srt::CUDTUnited::deleteGroup(CUDTGroup* g) { using srt_logging::gmlog; - srt::sync::ScopedLock cg(m_GlobControlLock); + srt::sync::ExclusiveLock cg(m_GlobControlLock); return deleteGroup_LOCKED(g); } @@ -2110,7 +2127,7 @@ int srt::CUDTUnited::close(CUDTSocket* s) HLOGC(smlog.Debug, log << "@" << u << "U::close done. GLOBAL CLOSE: " << s->core().CONID() << "Acquiring GLOBAL control lock"); - ScopedLock manager_cg(m_GlobControlLock); + ExclusiveLock manager_cg(m_GlobControlLock); // since "s" is located before m_GlobControlLock, locate it again in case // it became invalid // XXX This is very weird; if we state that the CUDTSocket object @@ -2181,7 +2198,7 @@ int srt::CUDTUnited::close(CUDTSocket* s) // Done the other way, but still done. You can stop waiting. bool isgone = false; { - ScopedLock manager_cg(m_GlobControlLock); + SharedLock manager_cg(m_GlobControlLock); isgone = m_ClosedSockets.count(u) == 0; } if (!isgone) @@ -2583,7 +2600,7 @@ int srt::CUDTUnited::epoll_release(const int eid) srt::CUDTSocket* srt::CUDTUnited::locateSocket(const SRTSOCKET u, ErrorHandling erh) { - ScopedLock cg(m_GlobControlLock); + SharedLock cg(m_GlobControlLock); CUDTSocket* s = locateSocket_LOCKED(u); if (!s) { @@ -2611,7 +2628,7 @@ srt::CUDTSocket* srt::CUDTUnited::locateSocket_LOCKED(SRTSOCKET u) #if ENABLE_BONDING srt::CUDTGroup* srt::CUDTUnited::locateAcquireGroup(SRTSOCKET u, ErrorHandling erh) { - ScopedLock cg(m_GlobControlLock); + SharedLock cg(m_GlobControlLock); const groups_t::iterator i = m_Groups.find(u); if (i == m_Groups.end()) @@ -2628,7 +2645,7 @@ srt::CUDTGroup* srt::CUDTUnited::locateAcquireGroup(SRTSOCKET u, ErrorHandling e srt::CUDTGroup* srt::CUDTUnited::acquireSocketsGroup(CUDTSocket* s) { - ScopedLock cg(m_GlobControlLock); + SharedLock cg(m_GlobControlLock); CUDTGroup* g = s->m_GroupOf; if (!g) return NULL; @@ -2642,7 +2659,7 @@ srt::CUDTGroup* srt::CUDTUnited::acquireSocketsGroup(CUDTSocket* s) srt::CUDTSocket* srt::CUDTUnited::locateAcquireSocket(SRTSOCKET u, ErrorHandling erh) { - ScopedLock cg(m_GlobControlLock); + SharedLock cg(m_GlobControlLock); CUDTSocket* s = locateSocket_LOCKED(u); if (!s) @@ -2665,7 +2682,7 @@ bool srt::CUDTUnited::acquireSocket(CUDTSocket* s) // directly from m_Sockets, or even better, has been acquired // by some other functionality already, which is only about to // be released earlier than you need. - ScopedLock cg(m_GlobControlLock); + SharedLock cg(m_GlobControlLock); s->apiAcquire(); // Keep the lock so that no one changes anything in the meantime. // If the socket m_Status == SRTS_CLOSED (set by setClosed()), then @@ -2681,7 +2698,7 @@ bool srt::CUDTUnited::acquireSocket(CUDTSocket* s) srt::CUDTSocket* srt::CUDTUnited::locatePeer(const sockaddr_any& peer, const SRTSOCKET id, int32_t isn) { - ScopedLock cg(m_GlobControlLock); + SharedLock cg(m_GlobControlLock); map >::iterator i = m_PeerRec.find(CUDTSocket::getPeerSpec(id, isn)); if (i == m_PeerRec.end()) @@ -2705,7 +2722,7 @@ srt::CUDTSocket* srt::CUDTUnited::locatePeer(const sockaddr_any& peer, const SRT void srt::CUDTUnited::checkBrokenSockets() { - ScopedLock cg(m_GlobControlLock); + ExclusiveLock cg(m_GlobControlLock); #if ENABLE_BONDING vector delgids; @@ -3059,7 +3076,7 @@ bool srt::CUDTUnited::channelSettingsMatch(const CSrtMuxerConfig& cfgMuxer, cons void srt::CUDTUnited::updateMux(CUDTSocket* s, const sockaddr_any& reqaddr, const UDPSOCKET* udpsock /*[[nullable]]*/) { - ScopedLock cg(m_GlobControlLock); + ExclusiveLock cg(m_GlobControlLock); // If udpsock is provided, then this socket will be simply // taken for binding as a good deal. It would be nice to make @@ -3366,7 +3383,7 @@ void srt::CUDTUnited::updateMux(CUDTSocket* s, const sockaddr_any& reqaddr, cons // multiplexer wasn't found by id, the search by port number continues. bool srt::CUDTUnited::updateListenerMux(CUDTSocket* s, const CUDTSocket* ls) { - ScopedLock cg(m_GlobControlLock); + ExclusiveLock cg(m_GlobControlLock); const int port = ls->m_SelfAddr.hport(); HLOGC(smlog.Debug, @@ -3534,7 +3551,7 @@ SRTSOCKET srt::CUDT::createGroup(SRT_GROUP_TYPE gt) { try { - srt::sync::ScopedLock globlock(uglobal().m_GlobControlLock); + srt::sync::ExclusiveLock globlock(uglobal().m_GlobControlLock); return newGroup(gt).id(); // Note: potentially, after this function exits, the group // could be deleted, immediately, from a separate thread (tho @@ -3590,7 +3607,7 @@ SRTSOCKET srt::CUDT::getGroupOfSocket(SRTSOCKET socket) { // Lock this for the whole function as we need the group // to persist the call. - ScopedLock glock(uglobal().m_GlobControlLock); + SharedLock glock(uglobal().m_GlobControlLock); CUDTSocket* s = uglobal().locateSocket_LOCKED(socket); if (!s || !s->m_GroupOf) return APIError(MJ_NOTSUP, MN_INVAL, 0); diff --git a/srtcore/api.h b/srtcore/api.h index d0249ca10..d3f8427d9 100644 --- a/srtcore/api.h +++ b/srtcore/api.h @@ -407,7 +407,7 @@ class CUDTUnited groups_t m_Groups; #endif - sync::Mutex m_GlobControlLock; // used to synchronize UDT API + sync::SharedMutex m_GlobControlLock; // used to synchronize UDT API sync::Mutex m_IDLock; // used to synchronize ID generation diff --git a/srtcore/core.cpp b/srtcore/core.cpp index d2118c555..344f21ac1 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -1012,8 +1012,6 @@ void srt::CUDT::open() void srt::CUDT::setListenState() { - ScopedLock cg(m_ConnectionLock); - if (!m_bOpened) throw CUDTException(MJ_NOTSUP, MN_NONE, 0); @@ -1021,14 +1019,42 @@ void srt::CUDT::setListenState() throw CUDTException(MJ_NOTSUP, MN_ISCONNECTED, 0); // listen can be called more than once - if (m_bListening) - return; - - // if there is already another socket listening on the same port - if (m_pRcvQueue->setListener(this) < 0) - throw CUDTException(MJ_NOTSUP, MN_BUSY, 0); - - m_bListening = true; + // If two threads call srt_listen at the same time, only + // one will pass this condition; others will be rejected. + // If it was called ever once, none will pass. + for (;;) + { + if (m_bListening.compare_exchange(false, true)) + { + // if there is already another socket listening on the same port + if (!m_pRcvQueue->setListener(this)) + { + // Failed here, so + m_bListening = false; + throw CUDTException(MJ_NOTSUP, MN_BUSY, 0); + } + } + else + { + // Ok, this thread could have been blocked access, + // but still the other thread that attempted to set + // the listener could have failed. Therefore check + // again if the listener was set successfully, and + // if the listening point is still free, try again. + CUDT* current = m_pRcvQueue->getListener(); + if (current == NULL) + { + continue; + } + else if (current != this) + { + // Some other listener already set it + throw CUDTException(MJ_NOTSUP, MN_BUSY, 0); + } + // If it was you who set this, just return with no exception. + } + break; + } } size_t srt::CUDT::fillSrtHandshake(uint32_t *aw_srtdata, size_t srtlen, int msgtype, int hs_version) @@ -1734,7 +1760,7 @@ bool srt::CUDT::createSrtHandshake( if (have_group) { // NOTE: See information about mutex ordering in api.h - ScopedLock gdrg (uglobal().m_GlobControlLock); + SharedLock gdrg (uglobal().m_GlobControlLock); if (!m_parent->m_GroupOf) { // This may only happen if since last check of m_GroupOf pointer the socket was removed @@ -3188,10 +3214,19 @@ bool srt::CUDT::interpretGroup(const int32_t groupdata[], size_t data_size SRT_A return false; } - ScopedLock guard_group_existence (uglobal().m_GlobControlLock); - if (m_SrtHsSide == HSD_INITIATOR) { + // Here we'll be only using a pre-existing group, so shared is enough. + SharedLock guard_group_existence (uglobal().m_GlobControlLock); + + // Recheck broken flags after acquisition + if (m_bClosing || m_bBroken) + { + m_RejectReason = SRT_REJ_CLOSE; + LOGC(cnlog.Error, log << CONID() << "interpretGroup: closure during handshake, interrupting"); + return false; + } + // This is a connection initiator that has requested the peer to make a // mirror group and join it, then respond its mirror group id. The // `grpid` variable contains this group ID; map this as your peer @@ -3210,64 +3245,81 @@ bool srt::CUDT::interpretGroup(const int32_t groupdata[], size_t data_size SRT_A return false; } - // Group existence is guarded, so we can now lock the group as well. - ScopedLock gl(*pg->exp_groupLock()); - - // Now we know the group exists, but it might still be closed - if (pg->closing()) { - LOGC(cnlog.Error, log << CONID() << "HS/RSP: group was closed in the process, can't continue connecting"); - m_RejectReason = SRT_REJ_IPE; - return false; - } + // XXX Consider moving this to another function to avoid + // exposing group lock. - SRTSOCKET peer = pg->peerid(); - if (peer == -1) - { - // This is the first connection within this group, so this group - // has just been informed about the peer membership. Accept it. - pg->set_peerid(grpid); - HLOGC(cnlog.Debug, - log << CONID() << "HS/RSP: group $" << pg->id() << " -> peer $" << pg->peerid() - << ", copying characteristic data"); + // Group existence is guarded, so we can now lock the group as well. + ScopedLock gl(*pg->exp_groupLock()); - // The call to syncWithSocket is copying - // some interesting data from the first connected - // socket. This should be only done for the first successful connection. - pg->syncWithSocket(*this, HSD_INITIATOR); - } - // Otherwise the peer id must be the same as existing, otherwise - // this group is considered already bound to another peer group. - // (Note that the peer group is peer-specific, and peer id numbers - // may repeat among sockets connected to groups established on - // different peers). - else if (peer != grpid) - { - LOGC(cnlog.Error, - log << CONID() << "IPE: HS/RSP: group membership responded for peer $" << grpid - << " but the current socket's group $" << pg->id() << " has already a peer $" << peer); - m_RejectReason = SRT_REJ_GROUP; - return false; - } - else - { - HLOGC(cnlog.Debug, - log << CONID() << "HS/RSP: group $" << pg->id() << " ALREADY MAPPED to peer mirror $" - << pg->peerid()); + // Now we know the group exists, but it might still be closed + if (pg->closing()) + { + LOGC(cnlog.Error, log << CONID() << "HS/RSP: group was closed in the process, can't continue connecting"); + m_RejectReason = SRT_REJ_IPE; + return false; + } + + SRTSOCKET peer = pg->peerid(); + if (peer == SRT_INVALID_SOCK) + { + // This is the first connection within this group, so this group + // has just been informed about the peer membership. Accept it. + pg->set_peerid(grpid); + HLOGC(cnlog.Debug, + log << CONID() << "HS/RSP: group $" << pg->id() << " -> peer $" << pg->peerid() + << ", copying characteristic data"); + + // The call to syncWithSocket is copying + // some interesting data from the first connected + // socket. This should be only done for the first successful connection. + pg->syncWithSocket(*this, HSD_INITIATOR); + } + // Otherwise the peer id must be the same as existing, otherwise + // this group is considered already bound to another peer group. + // (Note that the peer group is peer-specific, and peer id numbers + // may repeat among sockets connected to groups established on + // different peers). + else if (peer != grpid) + { + LOGC(cnlog.Error, + log << CONID() << "IPE: HS/RSP: group membership responded for peer $" << grpid + << " but the current socket's group $" << pg->id() << " has already a peer $" << peer); + m_RejectReason = SRT_REJ_GROUP; + return false; + } + else + { + HLOGC(cnlog.Debug, + log << CONID() << "HS/RSP: group $" << pg->id() << " ALREADY MAPPED to peer mirror $" + << pg->peerid()); + } } + m_parent->m_GroupOf->debugGroup(); } else { + // Here we'll be potentially creating a new group, hence exclusive is needed. + ExclusiveLock guard_group_existence (uglobal().m_GlobControlLock); + + // Recheck broken flags after acquisition + if (m_bClosing || m_bBroken) + { + m_RejectReason = SRT_REJ_CLOSE; + LOGC(cnlog.Error, log << CONID() << "interpretGroup: closure during handshake, interrupting"); + return false; + } + // This is a connection responder that has been requested to make a // mirror group and join it. Later on, the HS response will be sent // and its group ID will be added to the HS extensions as mirror group // ID to the peer. SRTSOCKET lgid = makeMePeerOf(grpid, gtp, link_flags); - if (!lgid) + if (lgid == 0) return true; // already done - if (lgid == -1) + if (lgid == SRT_INVALID_SOCK) { // NOTE: This error currently isn't reported by makeMePeerOf, // so this is left to handle a possible error introduced in future. @@ -3288,10 +3340,9 @@ bool srt::CUDT::interpretGroup(const int32_t groupdata[], size_t data_size SRT_A f->weight = link_weight; f->agent = m_parent->m_SelfAddr; f->peer = m_PeerAddr; + m_parent->m_GroupOf->debugGroup(); } - m_parent->m_GroupOf->debugGroup(); - // That's all. For specific things concerning group // types, this will be later. return true; @@ -4843,7 +4894,7 @@ EConnectStatus srt::CUDT::postConnect(const CPacket* pResponse, bool rendezvous, { #if ENABLE_BONDING - ScopedLock cl (uglobal().m_GlobControlLock); + SharedLock cl (uglobal().m_GlobControlLock); CUDTGroup* g = m_parent->m_GroupOf; if (g) { @@ -4950,10 +5001,11 @@ EConnectStatus srt::CUDT::postConnect(const CPacket* pResponse, bool rendezvous, //int token = -1; #if ENABLE_BONDING { - ScopedLock cl (uglobal().m_GlobControlLock); + SharedLock cl (uglobal().m_GlobControlLock); CUDTGroup* g = m_parent->m_GroupOf; if (g) { + ScopedLock gl (*g->exp_groupLock()); // XXX this might require another check of group type. // For redundancy group, at least, update the status in the group. @@ -6380,7 +6432,11 @@ bool srt::CUDT::closeInternal() ATR_NOEXCEPT if (m_bListening) { m_bListening = false; - m_pRcvQueue->removeListener(this); + bool removed SRT_ATR_UNUSED = m_pRcvQueue->removeListener(this); + if (!removed) + { + LOGC(smlog.Error, log << CONID() << "CLOSING: IPE: listening=true but listener removal failed!"); + } } else if (m_bConnecting) { @@ -8213,7 +8269,7 @@ int srt::CUDT::sendCtrlAck(CPacket& ctrlpkt, int size) // can be either never set, already reset, or ever set // and possibly dangling. The re-check after lock eliminates // the dangling case. - ScopedLock glock (uglobal().m_GlobControlLock); + SharedLock glock (uglobal().m_GlobControlLock); // Note that updateLatestRcv will lock m_GroupOf->m_GroupLock, // but this is an intended order. @@ -8274,7 +8330,7 @@ int srt::CUDT::sendCtrlAck(CPacket& ctrlpkt, int size) if (group_read_seq != SRT_SEQNO_NONE && m_parent->m_GroupOf) { // See above explanation for double-checking - ScopedLock glock (uglobal().m_GlobControlLock); + SharedLock glock (uglobal().m_GlobControlLock); if (m_parent->m_GroupOf) { @@ -8431,7 +8487,7 @@ void srt::CUDT::updateSndLossListOnACK(int32_t ackdata_seqno) { // m_RecvAckLock is ordered AFTER m_GlobControlLock, so this can only // be done now that m_RecvAckLock is unlocked. - ScopedLock glock (uglobal().m_GlobControlLock); + SharedLock glock (uglobal().m_GlobControlLock); if (m_parent->m_GroupOf) { HLOGC(inlog.Debug, log << CONID() << "ACK: acking group sender buffer for #" << msgno_at_last_acked_seq); @@ -8591,7 +8647,7 @@ void srt::CUDT::processCtrlAck(const CPacket &ctrlpkt, const steady_clock::time_ #if ENABLE_BONDING if (m_parent->m_GroupOf) { - ScopedLock glock (uglobal().m_GlobControlLock); + SharedLock glock (uglobal().m_GlobControlLock); if (m_parent->m_GroupOf) { // Will apply m_GroupLock, ordered after m_GlobControlLock. @@ -8812,7 +8868,7 @@ void srt::CUDT::processCtrlAckAck(const CPacket& ctrlpkt, const time_point& tsAr if (m_config.bDriftTracer) { #if ENABLE_BONDING - ScopedLock glock(uglobal().m_GlobControlLock); // XXX not too excessive? + ExclusiveLock glock(uglobal().m_GlobControlLock); // XXX not too excessive? const bool drift_updated = #endif m_pRcvBuffer->addRcvTsbPdDriftSample(ctrlpkt.getMsgTimeStamp(), tsArrival, rtt); @@ -9358,7 +9414,7 @@ void srt::CUDT::updateAfterSrtHandshake(int hsv) if (m_parent->m_GroupOf) { - ScopedLock glock (uglobal().m_GlobControlLock); + SharedLock glock (uglobal().m_GlobControlLock); grpspec = m_parent->m_GroupOf ? " group=$" + Sprint(m_parent->m_GroupOf->id()) : string(); @@ -10512,7 +10568,7 @@ int srt::CUDT::processData(CUnit* in_unit) // reception sequence pointer stating that this link is not receiving. if (m_parent->m_GroupOf) { - ScopedLock protect_group_existence (uglobal().m_GlobControlLock); + ExclusiveLock protect_group_existence (uglobal().m_GlobControlLock); groups::SocketData* gi = m_parent->m_GroupMemberData; // This check is needed as after getting the lock the socket @@ -11720,7 +11776,7 @@ void srt::CUDT::checkTimers() #if ENABLE_BONDING if (m_parent->m_GroupOf) { - ScopedLock glock (uglobal().m_GlobControlLock); + SharedLock glock (uglobal().m_GlobControlLock); if (m_parent->m_GroupOf) { // Pass socket ID because it's about changing group socket data @@ -11752,7 +11808,7 @@ void srt::CUDT::completeBrokenConnectionDependencies(int errorcode) #if ENABLE_BONDING bool pending_broken = false; { - ScopedLock guard_group_existence (uglobal().m_GlobControlLock); + SharedLock guard_group_existence (uglobal().m_GlobControlLock); if (m_parent->m_GroupOf) { token = m_parent->m_GroupMemberData->token; @@ -11787,7 +11843,7 @@ void srt::CUDT::completeBrokenConnectionDependencies(int errorcode) // existence of the group will not be changed during // the operation. The attempt of group deletion will // have to wait until this operation completes. - ScopedLock lock(uglobal().m_GlobControlLock); + ExclusiveLock lock(uglobal().m_GlobControlLock); CUDTGroup* pg = m_parent->m_GroupOf; if (pg) { @@ -12045,7 +12101,7 @@ void srt::CUDT::processKeepalive(const CPacket& ctrlpkt, const time_point& tsArr // existence of the group will not be changed during // the operation. The attempt of group deletion will // have to wait until this operation completes. - ScopedLock lock(uglobal().m_GlobControlLock); + ExclusiveLock lock(uglobal().m_GlobControlLock); CUDTGroup* pg = m_parent->m_GroupOf; if (pg) { diff --git a/srtcore/core.h b/srtcore/core.h index 6f7bd6b19..00fd44923 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -436,7 +436,6 @@ class CUDT // immediately to free the socket void notListening() { - sync::ScopedLock cg(m_ConnectionLock); m_bListening = false; m_pRcvQueue->removeListener(this); } diff --git a/srtcore/group.cpp b/srtcore/group.cpp index dc3c1a9be..339512573 100644 --- a/srtcore/group.cpp +++ b/srtcore/group.cpp @@ -1044,7 +1044,7 @@ void CUDTGroup::close() vector ids; { - ScopedLock glob(CUDT::uglobal().m_GlobControlLock); + ExclusiveLock glob(CUDT::uglobal().m_GlobControlLock); ScopedLock g(m_GroupLock); m_bClosing = true; @@ -3409,7 +3409,7 @@ void CUDTGroup::send_CloseBrokenSockets(vector& w_wipeme) // With unlocked GroupLock, we can now lock GlobControlLock. // This is needed to prevent any of them deleted from the container // at the same time. - ScopedLock globlock(CUDT::uglobal().m_GlobControlLock); + SharedLock globlock(CUDT::uglobal().m_GlobControlLock); for (vector::iterator p = w_wipeme.begin(); p != w_wipeme.end(); ++p) { @@ -3446,7 +3446,7 @@ void CUDTGroup::sendBackup_CloseBrokenSockets(SendBackupCtx& w_sendBackupCtx) // With unlocked GroupLock, we can now lock GlobControlLock. // This is needed prevent any of them be deleted from the container // at the same time. - ScopedLock globlock(CUDT::uglobal().m_GlobControlLock); + SharedLock globlock(CUDT::uglobal().m_GlobControlLock); typedef vector::const_iterator const_iter_t; for (const_iter_t member = w_sendBackupCtx.memberStates().begin(); member != w_sendBackupCtx.memberStates().end(); ++member) diff --git a/srtcore/queue.cpp b/srtcore/queue.cpp index 6cb4faeb1..f8b782174 100644 --- a/srtcore/queue.cpp +++ b/srtcore/queue.cpp @@ -1429,7 +1429,7 @@ srt::EConnectStatus srt::CRcvQueue::worker_ProcessConnectionRequest(CUnit* unit, bool have_listener = false; { SharedLock shl(m_pListener); - CUDT* pListener = m_pListener.getPtrNoLock(); + CUDT* pListener = m_pListener.get_locked(shl); if (pListener) { @@ -1720,17 +1720,25 @@ int srt::CRcvQueue::recvfrom(int32_t id, CPacket& w_packet) return (int)w_packet.getLength(); } -int srt::CRcvQueue::setListener(CUDT* u) +bool srt::CRcvQueue::setListener(CUDT* u) { - if (!m_pListener.set(u)) - return -1; + return m_pListener.compare_exchange(NULL, u); +} - return 0; +srt::CUDT* srt::CRcvQueue::getListener() +{ + SharedLock lkl (m_pListener); + return m_pListener.get_locked(lkl); } -void srt::CRcvQueue::removeListener(const CUDT* u) +// XXX NOTE: TSan reports here false positive against the call +// to locateSocket in CUDTUnited::newConnection. This here will apply +// exclusive lock on m_pListener, while keeping shared lock on +// CUDTUnited::m_GlobControlLock in CUDTUnited::closeAllSockets. +// As the other thread locks both as shared, this is no deadlock risk. +bool srt::CRcvQueue::removeListener(CUDT* u) { - m_pListener.clearIf(u); + return m_pListener.compare_exchange(u, NULL); } void srt::CRcvQueue::registerConnector(const SRTSOCKET& id, diff --git a/srtcore/queue.h b/srtcore/queue.h index 48bedd9af..0d23790e9 100644 --- a/srtcore/queue.h +++ b/srtcore/queue.h @@ -538,8 +538,9 @@ class CRcvQueue #endif private: - int setListener(CUDT* u); - void removeListener(const CUDT* u); + bool setListener(CUDT* u); + CUDT* getListener(); + bool removeListener(CUDT* u); void registerConnector(const SRTSOCKET& id, CUDT* u, diff --git a/srtcore/sync.h b/srtcore/sync.h index 7e11c29cf..100a11eed 100644 --- a/srtcore/sync.h +++ b/srtcore/sync.h @@ -535,6 +535,15 @@ class SharedMutex }; #endif +inline void enterCS(SharedMutex& m) SRT_ATTR_EXCLUDES(m) SRT_ATTR_ACQUIRE(m) { m.lock(); } + +inline bool tryEnterCS(SharedMutex& m) SRT_ATTR_EXCLUDES(m) SRT_ATTR_TRY_ACQUIRE(true, m) { return m.try_lock(); } + +inline void leaveCS(SharedMutex& m) SRT_ATTR_REQUIRES(m) SRT_ATTR_RELEASE(m) { m.unlock(); } + +inline void setupMutex(SharedMutex&, const char*) {} +inline void releaseMutex(SharedMutex&) {} + /// A version of std::scoped_lock (or lock_guard for C++11). /// We could have used the srt::sync::ScopedLock making it a template-based class. /// But in that case all usages would have to be specificed like ScopedLock in C++03. @@ -585,26 +594,21 @@ class CSharedObjectPtr : public SharedMutex { } - bool set(T* pObj) + bool compare_exchange(T* expected, T* newobj) { ExclusiveLock lock(*this); - if (m_pObj) + if (m_pObj != expected) return false; - m_pObj = pObj; + m_pObj = newobj; return true; } - bool clearIf(const T* pObj) + T* get_locked(SharedLock& /*wholocked*/) { - ExclusiveLock lock(*this); - if (m_pObj != pObj) - return false; - m_pObj = NULL; - return true; + // XXX Here you can assert that `wholocked` locked *this. + return m_pObj; } - T* getPtrNoLock() const { return m_pObj; } - private: T* m_pObj; }; From 725e2d472115c4ba745822441b79125ec9efdd58 Mon Sep 17 00:00:00 2001 From: cl-ment Date: Wed, 27 Aug 2025 10:56:20 +0200 Subject: [PATCH 10/25] [build] Update Ubuntu runners to Lastest. (#3180) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [build] Update Ubuntu runners to 22.04. * Update to googletest-1.12.1 * Remove Unit Tests from the ABI workflow. * Use latest version of ubuntu * Remove build-wrapper. --------- Authored-by: Clément Gérouville --- .github/workflows/abi.yml | 6 +++--- .github/workflows/android.yaml | 2 +- .github/workflows/cxx11-ubuntu.yaml | 4 ++-- scripts/googletest-download.cmake | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/abi.yml b/.github/workflows/abi.yml index 2c05cc06b..51d22a516 100644 --- a/.github/workflows/abi.yml +++ b/.github/workflows/abi.yml @@ -12,7 +12,7 @@ env: jobs: build: name: ABI checks - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -22,7 +22,7 @@ jobs: run: | cd pull_request mkdir _build && cd _build - cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_UNITTESTS=ON ../ + cmake -DCMAKE_BUILD_TYPE=Debug ../ - name: build run: | sudo apt install -y abi-dumper @@ -41,7 +41,7 @@ jobs: echo $SRT_TAG_VERSION cd tag mkdir _build && cd _build - cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_UNITTESTS=ON ../ + cmake -DCMAKE_BUILD_TYPE=Debug ../ - name: build_tag run: | cd tag diff --git a/.github/workflows/android.yaml b/.github/workflows/android.yaml index 0af85fda3..5e11c75df 100644 --- a/.github/workflows/android.yaml +++ b/.github/workflows/android.yaml @@ -9,7 +9,7 @@ on: jobs: build: name: NDK-R23 - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Setup Android NDK R23 diff --git a/.github/workflows/cxx11-ubuntu.yaml b/.github/workflows/cxx11-ubuntu.yaml index 7d30f4a1a..2f362d84f 100644 --- a/.github/workflows/cxx11-ubuntu.yaml +++ b/.github/workflows/cxx11-ubuntu.yaml @@ -9,7 +9,7 @@ on: jobs: build: name: ubuntu - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - name: configure @@ -17,7 +17,7 @@ jobs: mkdir _build && cd _build cmake ../ -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DENABLE_STDCXX_SYNC=ON -DENABLE_ENCRYPTION=ON -DENABLE_UNITTESTS=ON -DENABLE_BONDING=ON -DENABLE_TESTING=ON -DENABLE_EXAMPLES=ON -DENABLE_CODE_COVERAGE=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON - name: build - run: cd _build && build-wrapper-linux-x86-64 --out-dir ${{ env.BUILD_WRAPPER_OUT_DIR }} cmake --build . + run: cd _build && cmake --build . - name: test run: | cd _build && ctest --extra-verbose diff --git a/scripts/googletest-download.cmake b/scripts/googletest-download.cmake index 7469f4f7b..36554f316 100644 --- a/scripts/googletest-download.cmake +++ b/scripts/googletest-download.cmake @@ -11,7 +11,7 @@ ExternalProject_Add( BINARY_DIR "@GOOGLETEST_DOWNLOAD_ROOT@/googletest-build" GIT_REPOSITORY https://github.com/google/googletest.git - GIT_TAG release-1.10.0 + GIT_TAG release-1.12.1 CONFIGURE_COMMAND "" BUILD_COMMAND "" INSTALL_COMMAND "" From 265d0e495f5a44e762b65ceb60b9fa0e7a698b73 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Fri, 29 Aug 2025 13:10:58 +0200 Subject: [PATCH 11/25] [core] Fixed invalid cookie contest calculation (#3212) Co-authored-by: Mikolaj Malecki --- srtcore/core.cpp | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 344f21ac1..050caaa17 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -4107,13 +4107,13 @@ void srt::CUDT::cookieContest() if (m_SrtHsSide != HSD_DRAW) return; - LOGC(cnlog.Debug, - log << CONID() << "cookieContest: agent=" << m_ConnReq.m_iCookie << " peer=" << m_ConnRes.m_iCookie); - // Here m_ConnReq.m_iCookie is a local cookie value sent in connection request to the peer. // m_ConnRes.m_iCookie is a cookie value sent by the peer in its connection request. if (m_ConnReq.m_iCookie == 0 || m_ConnRes.m_iCookie == 0) { + LOGC(cnlog.Debug, log << CONID() << "cookieContest: agent=" << m_ConnReq.m_iCookie << " peer=" << m_ConnRes.m_iCookie + << " - ERROR: zero not allowed!"); + // Note that it's virtually impossible that Agent's cookie is not ready, this // shall be considered IPE. // Not all cookies are ready, don't start the contest. @@ -4134,11 +4134,34 @@ void srt::CUDT::cookieContest() // int32_t iBetterCookie = 2002790373 (7760 27E5); // // Now 64-bit arithmetic is used to calculate the actual result of subtraction. - // The 31-st bit is then checked to check if the resulting is negative in 32-bit aritmetics. - // This way the old contest behavior is preserved, and potential compiler optimisations are avoided. - const int64_t contest = int64_t(m_ConnReq.m_iCookie) - int64_t(m_ConnRes.m_iCookie); - - if ((contest & 0xFFFFFFFF) == 0) + // + // In SRT v1.5.4 there was a version, that checked: + // - if LOWER 32-bits are 0, this is a draw + // - if bit 31 is set (AND with 0x80000000), the result is considered negative. + // This was erroneous because for 1 and 0x80000001 cookie values the + // result was always the same, regardless of the order: + // + // 0x0000000000000001 - 0xFFFFFFFF80000001 = 0x0000000080000000 + // 0xFFFFFFFF80000001 - 0x0000000080000000 = 0xFFFFFFFF80000000 + // + // >> if (contest & 0x80000000) -> results in true in both comparisons. + // + // This version takes the bare result of the 64-bit arithmetics. + const int64_t xreq = int64_t(m_ConnReq.m_iCookie); + const int64_t xres = int64_t(m_ConnRes.m_iCookie); + const int64_t contest = xreq - xres; + + // NOTE: for 1.6.0 use: + // fmtc hex64 = fmtc().uhex().fillzero().width(16); then + // << fmt(xreq, hex64) + LOGC(cnlog.Debug, log << CONID() << "cookieContest: agent=" << m_ConnReq.m_iCookie + << " peer=" << m_ConnRes.m_iCookie + << hex << uppercase << setfill('0') // PERSISTENT flags + << " X64: "<< setw(16) << xreq + << " vs. " << setw(16) << xres + << " DIFF: " << setw(16) << contest); + + if (contest == 0) { // DRAW! The only way to continue would be to force the // cookies to be regenerated and to start over. But it's @@ -4154,7 +4177,7 @@ void srt::CUDT::cookieContest() return; } - if (contest & 0x80000000) + if (contest < 0) { m_SrtHsSide = HSD_RESPONDER; return; From 5138f37d45b5724e379e27760dc7e30bcfa110f1 Mon Sep 17 00:00:00 2001 From: cl-ment Date: Fri, 29 Aug 2025 13:15:35 +0200 Subject: [PATCH 12/25] [core] Cleanup SRT state after a fork() (issue #3177) (#3179) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [core] Cleanup SRT state after a fork() (issue #3177) * Free socket memory without calling the destructor. * Remove srt_cleanupAtFork() from the API. * Make it compile on systems that don't support pthread_atfork() * Remove a typo * Avoid to send shutdown packet when cleaning up after a fork. * Close the dangling UDP sockets, Free memory. * Add TODO for freeing the Send Queue after refacttoring it. * Ensure that CThread is joinable before join() * Try fix iOS-cxxsyncOFF * Replace the mutex pointer by a mutex reference. * Refactor the Multiplexer initialization. * Fix SIGSEGV * Fix Compilation error on a Debug Log * Rollback to cleaner code for the multiplexer initialization. * Add compatibility with C++11 Sync. * Apply code review changes * Replace the resetThread() macro by an inline function. Co-authored-by: Sektor van Skijlen * Reset m_CGStopCond in CUDTUnited * Rework of resetThread() * Ensure Garbage collector is in the right state after fork. * Protect ~CMultiplexer() against NULL pointers. * Protect resetAtFork() and stop() against NULL pointers. * Added fork example to the repository * Remove french comments. --------- Co-authored-by: Clément Gérouville Co-authored-by: Sektor van Skijlen Co-authored-by: Mikolaj Malecki --- CMakeLists.txt | 10 ++- examples/fork-test/Makefile | 9 +++ examples/fork-test/README.md | 7 ++ examples/fork-test/srt_client.c | 55 +++++++++++++++ examples/fork-test/srt_server.c | 118 ++++++++++++++++++++++++++++++++ srtcore/api.cpp | 90 ++++++++++++++++++++++-- srtcore/api.h | 3 + srtcore/core.cpp | 12 ++++ srtcore/core.h | 3 + srtcore/queue.cpp | 74 +++++++++++++++----- srtcore/queue.h | 18 +++++ srtcore/sync.h | 6 +- srtcore/sync_cxx11.cpp | 18 +++++ srtcore/sync_posix.cpp | 39 +++++++---- 14 files changed, 426 insertions(+), 36 deletions(-) create mode 100644 examples/fork-test/Makefile create mode 100644 examples/fork-test/README.md create mode 100644 examples/fork-test/srt_client.c create mode 100644 examples/fork-test/srt_server.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 0acd39b41..2017139ed 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -966,10 +966,16 @@ else () message(STATUS "Pthread library: ${PTHREAD_LIBRARY}") endif() +list(PREPEND CMAKE_REQUIRED_LIBRARIES "${PTHREAD_LIBRARY}") +unset(CMAKE_REQUIRED_QUIET) + +check_symbol_exists(pthread_atfork "pthread.h" HAVE_PTHREAD_ATFORK) +if ("${HAVE_PTHREAD_ATFORK}" STREQUAL "1") + add_definitions(-DHAVE_PTHREAD_ATFORK=1) +endif () + # To avoid the need for other judgments when ENABLE_STDCXX_SYNC is OFF in the future, this is a separate conditional statement. if (NOT ENABLE_STDCXX_SYNC AND ENABLE_MONOTONIC_CLOCK) - list(PREPEND CMAKE_REQUIRED_LIBRARIES "${PTHREAD_LIBRARY}") - unset(CMAKE_REQUIRED_QUIET) check_symbol_exists(pthread_condattr_setclock "pthread.h" HAVE_PTHREAD_CONDATTR_SETCLOCK) message(STATUS "Checking pthread_condattr_setclock: '${HAVE_PTHREAD_CONDATTR_SETCLOCK}'") if ("${HAVE_PTHREAD_CONDATTR_SETCLOCK}" STREQUAL "1") diff --git a/examples/fork-test/Makefile b/examples/fork-test/Makefile new file mode 100644 index 000000000..57e9f3a94 --- /dev/null +++ b/examples/fork-test/Makefile @@ -0,0 +1,9 @@ +TARGETS=srt_server srt_client + +all: $(TARGETS) + +%: %.c + $(CC) $< `pkg-config --cflags --libs srt` -o `basename $< .c` + +clean: + rm -f $(TARGETS) diff --git a/examples/fork-test/README.md b/examples/fork-test/README.md new file mode 100644 index 000000000..59752ad02 --- /dev/null +++ b/examples/fork-test/README.md @@ -0,0 +1,7 @@ +The `srt_server` and `srt_client` apps should be compiled using +external installation of SRT (e.g. in the local directory), each +one as a single program. This is not compiled as a part of SRT. + +If you want to use a local installation, simply set `PKG_CONFIG_PATH` +environment variable to point to the local installation directory with +"lib/pkgconfig" or "lib64/pkgconfig" suffix. diff --git a/examples/fork-test/srt_client.c b/examples/fork-test/srt_client.c new file mode 100644 index 000000000..ab69d5956 --- /dev/null +++ b/examples/fork-test/srt_client.c @@ -0,0 +1,55 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#define SERVER_IP "127.0.0.1" +#define SERVER_PORT 9000 +int main() { + if (srt_startup() != 0) { + fprintf(stderr, "Error initializing SRT.\n"); + return 1; + } + SRTSOCKET client_sock = srt_create_socket(); + if (client_sock == SRT_INVALID_SOCK) { + fprintf(stderr, "Error creating a socket: %s\n", srt_getlasterror_str()); + return 1; + } + struct sockaddr_in sa; + memset(&sa, 0, sizeof sa); + sa.sin_family = AF_INET; + sa.sin_port = htons(SERVER_PORT); + inet_pton(AF_INET, SERVER_IP, &sa.sin_addr); + if (srt_connect(client_sock, (struct sockaddr*)&sa, sizeof(sa)) == SRT_ERROR) { + fprintf(stderr, "Error: srt_connect: %s\n", srt_getlasterror_str()); + return 1; + } + printf("Connected to SRT server %s:%d\n", SERVER_IP, SERVER_PORT); + const char* message = "Hello from SRT client!"; + int bytes = srt_send(client_sock, message, strlen(message)); + if (bytes == SRT_ERROR) { + fprintf(stderr, "Sending error: %s\n", srt_getlasterror_str()); + } else { + printf("Message sent: %s\n", message); + } + + while (1) + { + char buffer[1500]; + int nb = srt_recv(client_sock, buffer, sizeof(buffer)); + if (nb <= 0) + { + printf("Closed from the server !\n"); + srt_close(client_sock); + break; + } + buffer[nb] = 0; + printf("Server has sent: %s\n", buffer); + } + srt_cleanup(); + return 0; +} + diff --git a/examples/fork-test/srt_server.c b/examples/fork-test/srt_server.c new file mode 100644 index 000000000..a680f1966 --- /dev/null +++ b/examples/fork-test/srt_server.c @@ -0,0 +1,118 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#define PORT 9000 + +int run(char *command) { + pid_t pid = fork(); + if (pid < 0) { + perror("fork (intermediate)"); + exit(EXIT_FAILURE); + } + if (pid > 0) { + // Parent process + printf("[GRANDPARENT %d] waiting for grand-child process pid=%d to finish...\n", + getpid(), pid); + waitpid(pid, NULL, 0); // Wait for intermediate child + printf("[GRANDPARENT] returning\n"); + return 0; + } + // Intermediate process + //srt_cleanup(); + if (setsid() < 0) { + perror("setsid"); + exit(EXIT_FAILURE); + } + pid_t grandchild_pid = fork(); + if (grandchild_pid < 0) { + perror("fork (grandchild)"); + exit(EXIT_FAILURE); + } + if (grandchild_pid > 0) { + printf("[PARENT %d] waiting for 10s with child process pid=%d ...\n", + getpid(), grandchild_pid); + // Intermediate process exits immediately + sleep(10); + printf("[PARENT] exitting\n"); + exit(0); + } + // Grandchild process + // Redirect stdin to /dev/null + printf("[CHILD %d] Preparing descriptors...\n", getpid()); + int devnull = open("/dev/null", O_RDONLY); + if (devnull >= 0) { + dup2(devnull, STDIN_FILENO); + close(devnull); + } else { + perror("open /dev/null"); + } + // Redirect stdout to stderr + dup2(STDERR_FILENO, STDOUT_FILENO); + // Execute the command + printf("[CHILD] Executing process '%s'...\n", command); + execl("/bin/sh", "sh", "-c", command, (char *)NULL); + // If execl fails + perror("execl"); + exit(EXIT_FAILURE); +} + +int main() { + if (srt_startup() != 0) { + fprintf(stderr, "Error initializing SRT.\n"); + return 1; + } + + SRTSOCKET serv_sock = srt_create_socket(); + if (serv_sock == SRT_INVALID_SOCK) { + fprintf(stderr, "Error creating SRT socket: %s\n", srt_getlasterror_str()); + return 1; + } + struct sockaddr_in sa; + memset(&sa, 0, sizeof sa); + sa.sin_family = AF_INET; + sa.sin_port = htons(PORT); + sa.sin_addr.s_addr = INADDR_ANY; + if (srt_bind(serv_sock, (struct sockaddr*)&sa, sizeof sa) == SRT_ERROR) { + fprintf(stderr, "Error: srt_bind: %s\n", srt_getlasterror_str()); + return 1; + } + if (srt_listen(serv_sock, 5) == SRT_ERROR) { + fprintf(stderr, "Error: srt_listen: %s\n", srt_getlasterror_str()); + return 1; + } + printf("SRT server is listening on port %d...\n", PORT); + struct sockaddr_in client_addr; + int addr_len = sizeof(client_addr); + SRTSOCKET client_sock = srt_accept(serv_sock, (struct sockaddr*)&client_addr, &addr_len); + if (client_sock == SRT_INVALID_SOCK) { + fprintf(stderr, "Error: srt_accept: %s\n", srt_getlasterror_str()); + return 1; + } + printf("Client connected via SRT !\n"); + char buffer[1500]; + int bytes = srt_recv(client_sock, buffer, sizeof(buffer)); + if (bytes > 0) { + buffer[bytes] = '\0'; + printf("Message received: %s\n", buffer); + const char resp [] = "We read you!"; + srt_send(client_sock, resp, (sizeof resp)-1); + } else { + printf("Error: reading from srt_recv: %s.\n", srt_getlasterror_str()); + } + run("date > /tmp/res"); + printf("Server: sleep(1)\n"); + sleep(1); + // Nettoyage + printf("Server: closing SRT sockets\n"); + srt_close(client_sock); + srt_close(serv_sock); + printf("Server: cleanup\n"); + srt_cleanup(); + printf("Server: exit\n"); + return 0; +} diff --git a/srtcore/api.cpp b/srtcore/api.cpp index da929ef5a..22b093dcc 100644 --- a/srtcore/api.cpp +++ b/srtcore/api.cpp @@ -99,6 +99,12 @@ srt::CUDTSocket::~CUDTSocket() releaseMutex(m_ControlLock); } +void srt::CUDTSocket::resetAtFork() +{ + m_UDT.resetAtFork(); + resetCond(m_AcceptCond); +} + SRT_SOCKSTATUS srt::CUDTSocket::getStatus() { // TTL in CRendezvousQueue::updateConnStatus() will set m_bConnecting to false. @@ -275,6 +281,51 @@ void srt::CUDTUnited::stopGarbageCollector() } } +void srt::CUDTUnited::cleanupAllSockets() +{ + for (sockets_t::iterator i = m_Sockets.begin(); i != m_Sockets.end(); ++i) + { + CUDTSocket* s = i->second; + +#if ENABLE_BONDING + if (s->m_GroupOf) + { + s->removeFromGroup(false); + } +#endif + + // remove from listener's queue + sockets_t::iterator ls = m_Sockets.find(s->m_ListenSocket); + if (ls == m_Sockets.end()) + { + ls = m_ClosedSockets.find(s->m_ListenSocket); + } + if (ls != m_ClosedSockets.end()) + { + ls->second->m_QueuedSockets.erase(s->m_SocketID); + } + s->core().closeAtFork(); + s->resetAtFork(); + delete(s); + } + m_Sockets.clear(); + +#if ENABLE_BONDING + for (groups_t::iterator j = m_Groups.begin(); j != m_Groups.end(); ++j) + { + delete j->second; + } + m_Groups.clear(); +#endif + for (map::iterator i = m_mMultiplexer.begin(); i != m_mMultiplexer.end(); ++i) + { + CMultiplexer &multiplexer = i->second; + multiplexer.resetAtFork(); + } + m_mMultiplexer.clear(); +} + + void srt::CUDTUnited::closeAllSockets() { // remove all sockets and multiplexers @@ -370,6 +421,18 @@ int srt::CUDTUnited::startup() return startGarbageCollector() ? 0 : -1; } +int srt::CUDTUnited::cleanupAtFork() +{ + cleanupAllSockets(); + resetThread(&m_GCThread); + resetCond(m_GCStopCond); + m_GCStopLock.unlock(); + setupCond(m_GCStopCond, "GCStop"); + m_iInstanceCount=0; + m_bGCStatus = false; + return 0; +} + int srt::CUDTUnited::cleanup() { // IMPORTANT!!! @@ -3076,8 +3139,9 @@ bool srt::CUDTUnited::channelSettingsMatch(const CSrtMuxerConfig& cfgMuxer, cons void srt::CUDTUnited::updateMux(CUDTSocket* s, const sockaddr_any& reqaddr, const UDPSOCKET* udpsock /*[[nullable]]*/) { + const int port = reqaddr.hport(); ExclusiveLock cg(m_GlobControlLock); - + // If udpsock is provided, then this socket will be simply // taken for binding as a good deal. It would be nice to make // a sanity check to see if this UDP socket isn't already installed @@ -3087,7 +3151,6 @@ void srt::CUDTUnited::updateMux(CUDTSocket* s, const sockaddr_any& reqaddr, cons { // If not, we need to see if there exist already a multiplexer bound // to the same endpoint. - const int port = reqaddr.hport(); const CSrtConfig& cfgSocket = s->core().m_config; // This loop is going to check the attempted binding of @@ -3305,6 +3368,8 @@ void srt::CUDTUnited::updateMux(CUDTSocket* s, const sockaddr_any& reqaddr, cons } } + + // a new multiplexer is needed CMultiplexer m; configureMuxer((m), s, reqaddr.family()); @@ -3360,7 +3425,7 @@ void srt::CUDTUnited::updateMux(CUDTSocket* s, const sockaddr_any& reqaddr, cons // Rewrite the port here, as it might be only known upon return // from CChannel::open. m.m_iPort = installMuxer((s), m); - m_mMultiplexer[m.m_iID] = m; + swap(m_mMultiplexer[m.m_iID],m); } catch (const CUDTException&) { @@ -3373,7 +3438,7 @@ void srt::CUDTUnited::updateMux(CUDTSocket* s, const sockaddr_any& reqaddr, cons throw CUDTException(MJ_SYSTEMRES, MN_MEMORY, 0); } - HLOGC(smlog.Debug, log << "bind: creating new multiplexer for port " << m.m_iPort); + HLOGC(smlog.Debug, log << "bind: creating new multiplexer for port " << port); } // This function is going to find a multiplexer for the port contained @@ -3492,6 +3557,14 @@ void* srt::CUDTUnited::garbageCollect(void* p) int srt::CUDT::startup() { +#if HAVE_PTHREAD_ATFORK + static bool registered = false; + if (!registered) + { + pthread_atfork(NULL, NULL, (void (*)()) srt::CUDT::cleanupAtFork); + registered = true; + } +#endif return uglobal().startup(); } @@ -3500,6 +3573,15 @@ int srt::CUDT::cleanup() return uglobal().cleanup(); } +int srt::CUDT::cleanupAtFork() +{ + CUDTUnited &context = uglobal(); + context.cleanupAtFork(); + new (&context) CUDTUnited(); + + return context.startup(); +} + SRTSOCKET srt::CUDT::socket() { try diff --git a/srtcore/api.h b/srtcore/api.h index d3f8427d9..42b97ef32 100644 --- a/srtcore/api.h +++ b/srtcore/api.h @@ -120,6 +120,7 @@ class CUDTSocket } ~CUDTSocket(); + void resetAtFork(); void construct(); @@ -263,6 +264,7 @@ class CUDTUnited /// release the UDT library. /// @return 0 if success, otherwise -1 is returned. int cleanup(); + int cleanupAtFork(); /// Create a new UDT socket. /// @param [out] pps Variable (optional) to which the new socket will be written, if succeeded @@ -464,6 +466,7 @@ class CUDTUnited bool acquireSocket(CUDTSocket* s); bool startGarbageCollector(); void stopGarbageCollector(); + void cleanupAllSockets(); void closeAllSockets(); public: diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 050caaa17..0acef17bb 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -6513,6 +6513,12 @@ bool srt::CUDT::closeInternal() ATR_NOEXCEPT return true; } +bool srt::CUDT::closeAtFork() ATR_NOEXCEPT +{ + m_bShutdown = true; + return closeInternal(); +} + int srt::CUDT::receiveBuffer(char *data, int len) { if (!m_CongCtl->checkTransArgs(SrtCongestion::STA_BUFFER, SrtCongestion::STAD_RECV, data, len, SRT_MSGTTL_INF, false)) @@ -7939,6 +7945,12 @@ void srt::CUDT::destroySynch() m_RcvTsbPdCond.notify_all(); releaseCond(m_RcvTsbPdCond); } +void srt::CUDT::resetAtFork() +{ + resetCond(m_SendBlockCond); + resetCond(m_RecvDataCond); + resetCond(m_RcvTsbPdCond); +} void srt::CUDT::releaseSynch() { diff --git a/srtcore/core.h b/srtcore/core.h index 00fd44923..a07d17277 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -197,6 +197,7 @@ class CUDT public: //API static int startup(); static int cleanup(); + static int cleanupAtFork(); static SRTSOCKET socket(); #if ENABLE_BONDING static SRTSOCKET createGroup(SRT_GROUP_TYPE); @@ -604,6 +605,7 @@ class CUDT /// Close the opened UDT entity. bool closeInternal() ATR_NOEXCEPT; + bool closeAtFork() ATR_NOEXCEPT; void updateBrokenConnection(); void completeBrokenConnectionDependencies(int errorcode); @@ -1045,6 +1047,7 @@ class CUDT void initSynch(); void destroySynch(); + void resetAtFork(); void releaseSynch(); private: // Common connection Congestion Control setup diff --git a/srtcore/queue.cpp b/srtcore/queue.cpp index f8b782174..88c0d30f6 100644 --- a/srtcore/queue.cpp +++ b/srtcore/queue.cpp @@ -220,6 +220,11 @@ srt::CSndUList::~CSndUList() delete[] m_pHeap; } +void srt::CSndUList::resetAtFork() +{ + resetCond(m_ListCond); +} + void srt::CSndUList::update(const CUDT* u, EReschedule reschedule, sync::steady_clock::time_point ts) { ScopedLock listguard(m_ListLock); @@ -414,6 +419,17 @@ srt::CSndQueue::CSndQueue() } srt::CSndQueue::~CSndQueue() +{ + delete m_pSndUList; +} + +void srt::CSndQueue::resetAtFork() +{ + resetThread(&m_WorkerThread); + m_pSndUList->resetAtFork(); +} + +void srt::CSndQueue::stop() { m_bClosing = true; @@ -430,8 +446,6 @@ srt::CSndQueue::~CSndQueue() HLOGC(rslog.Debug, log << "SndQueue: EXIT"); m_WorkerThread.join(); } - - delete m_pSndUList; } int srt::CSndQueue::ioctlQuery(int type) const @@ -1168,15 +1182,6 @@ srt::CRcvQueue::CRcvQueue() srt::CRcvQueue::~CRcvQueue() { - m_bClosing = true; - - if (m_WorkerThread.joinable()) - { - HLOGC(rslog.Debug, log << "RcvQueue: EXIT"); - m_WorkerThread.join(); - } - releaseCond(m_BufferCond); - delete m_pUnitQueue; delete m_pRcvUList; delete m_pHash; @@ -1194,6 +1199,24 @@ srt::CRcvQueue::~CRcvQueue() } } +void srt::CRcvQueue::resetAtFork() +{ + resetThread(&m_WorkerThread); +} + +void srt::CRcvQueue::stop() +{ + m_bClosing = true; + + if (m_WorkerThread.joinable()) + { + HLOGC(rslog.Debug, log << "RcvQueue: EXIT"); + m_WorkerThread.join(); + } + releaseCond(m_BufferCond); +} + + #if ENABLE_LOGGING srt::sync::atomic srt::CRcvQueue::m_counter(0); #endif @@ -1819,16 +1842,35 @@ void srt::CRcvQueue::storePktClone(int32_t id, const CPacket& pkt) } } -void srt::CMultiplexer::destroy() +void srt::CMultiplexer::resetAtFork() { - // Reverse order of the assigned. - delete m_pRcvQueue; - delete m_pSndQueue; - delete m_pTimer; + if (m_pRcvQueue != NULL) + m_pRcvQueue->resetAtFork(); + if (m_pSndQueue != NULL) + m_pSndQueue->resetAtFork(); +} +void srt::CMultiplexer::close() +{ if (m_pChannel) { m_pChannel->close(); delete m_pChannel; + m_pChannel = NULL; } } + +void srt::CMultiplexer::stop() +{ + if (m_pRcvQueue != NULL) + m_pRcvQueue->stop(); + if (m_pSndQueue != NULL) + m_pSndQueue->stop(); +} + +void srt::CMultiplexer::destroy() +{ + // Reverse order of the assigned. + stop(); + close(); +} diff --git a/srtcore/queue.h b/srtcore/queue.h index 0d23790e9..cf3109767 100644 --- a/srtcore/queue.h +++ b/srtcore/queue.h @@ -157,6 +157,7 @@ class CSndUList }; static EReschedule rescheduleIf(bool cond) { return cond ? DO_RESCHEDULE : DONT_RESCHEDULE; } + void resetAtFork(); /// Update the timestamp of the UDT instance on the list. /// @param [in] u pointer to the UDT instance @@ -401,6 +402,7 @@ class CSndQueue ~CSndQueue(); public: + void resetAtFork(); // XXX There's currently no way to access the socket ID set for // whatever the queue is currently working for. Required to find // some way to do this, possibly by having a "reverse pointer". @@ -439,6 +441,7 @@ class CSndQueue int sockoptQuery(int level, int type) const; void setClosing() { m_bClosing = true; } + void stop(); private: static void* worker(void* param); @@ -486,6 +489,7 @@ class CRcvQueue ~CRcvQueue(); public: + void resetAtFork(); // XXX There's currently no way to access the socket ID set for // whatever the queue is currently working. Required to find // some way to do this, possibly by having a "reverse pointer". @@ -513,6 +517,7 @@ class CRcvQueue int getIPversion() { return m_iIPversion; } + void stop(); private: static void* worker(void* param); sync::CThread m_WorkerThread; @@ -599,6 +604,19 @@ struct CMultiplexer { } + ~CMultiplexer() + { + if (m_pRcvQueue != NULL) + delete m_pRcvQueue; + if (m_pSndQueue != NULL) + delete m_pSndQueue; + if (m_pTimer != NULL) + delete m_pTimer; + close(); + } + void resetAtFork(); + void close(); + void stop(); void destroy(); }; diff --git a/srtcore/sync.h b/srtcore/sync.h index 100a11eed..72da5869b 100644 --- a/srtcore/sync.h +++ b/srtcore/sync.h @@ -429,11 +429,11 @@ class Condition public: Condition(); ~Condition(); - public: /// These functions do not align with C++11 version. They are here hopefully as a temporal solution /// to avoud issues with static initialization of CV on windows. void init(); + void reset(); void destroy(); public: @@ -486,6 +486,7 @@ class Condition }; inline void setupCond(Condition& cv, const char*) { cv.init(); } +inline void resetCond(Condition& cv) { cv.reset(); } inline void releaseCond(Condition& cv) { cv.destroy(); } /////////////////////////////////////////////////////////////////////////////// @@ -1002,6 +1003,7 @@ class CThread private: pthread_t m_thread; + pid_t m_pid; }; template @@ -1033,6 +1035,8 @@ namespace this_thread #endif +inline void resetThread(CThread* th) { (void)new (th) CThread; } + /// StartThread function should be used to do CThread assignments: /// @code /// CThread a(); diff --git a/srtcore/sync_cxx11.cpp b/srtcore/sync_cxx11.cpp index bdfbf9a4e..c376b0adf 100644 --- a/srtcore/sync_cxx11.cpp +++ b/srtcore/sync_cxx11.cpp @@ -58,6 +58,24 @@ srt::sync::Condition::~Condition() {} void srt::sync::Condition::init() {} +void srt::sync::Condition::reset() +{ + // SRT attempts to safely handle `fork()` in multithreaded environments, + // even though using `fork()` in such contexts is strongly discouraged. + // This is because `fork()` only duplicates the calling thread, leaving + // synchronization primitives (like condition variables) in an + // undefined or inconsistent state in the child process. + // + // To mitigate this, SRT forcefully reinitializes these synchronization + // primitives post-fork. In POSIX, this is done by overwriting the object + // with its default-initialized state. In C++11, we achieve the same effect + // using *placement new* to reconstruct the object in place. This ensures + // the condition variable is returned to a fresh, "neutral" state, + // as if it was just created. + + new (&m_cv) std::condition_variable; +} + void srt::sync::Condition::destroy() {} void srt::sync::Condition::wait(UniqueLock& lock) diff --git a/srtcore/sync_posix.cpp b/srtcore/sync_posix.cpp index 164913097..09def5c33 100644 --- a/srtcore/sync_posix.cpp +++ b/srtcore/sync_posix.cpp @@ -309,6 +309,14 @@ void Condition::init() throw std::runtime_error("pthread_cond_init monotonic failed"); } +void Condition::reset() +{ + + pthread_cond_t temp = PTHREAD_COND_INITIALIZER; + memcpy(&m_cv, (void *) &temp, sizeof(m_cv)); + init(); +} + void Condition::destroy() { pthread_cond_destroy(&m_cv); @@ -370,6 +378,7 @@ void Condition::notify_all() srt::sync::CThread::CThread() { m_thread = pthread_t(); + m_pid = getpid(); } srt::sync::CThread::CThread(void *(*start_routine) (void *), void *arg) @@ -402,9 +411,9 @@ srt::sync::CThread& srt::sync::CThread::operator=(CThread& other) join(); #endif } - // Move thread handler from other m_thread = other.m_thread; + m_pid = other.m_pid; other.m_thread = pthread_t(); return *this; } @@ -419,24 +428,27 @@ void srt::sync::CThread::create_thread(void *(*start_routine) (void *), void *ar bool srt::sync::CThread::joinable() const { - return !pthread_equal(m_thread, pthread_t()); + return m_pid == getpid() && !pthread_equal(m_thread, pthread_t()); } void srt::sync::CThread::join() { - void *retval; - const int ret SRT_ATR_UNUSED = pthread_join(m_thread, &retval); - if (ret != 0) - { - LOGC(inlog.Error, log << "pthread_join failed with " << ret); - } + if (joinable()) + { + void *retval; + const int ret SRT_ATR_UNUSED = pthread_join(m_thread, &retval); + if (ret != 0) + { + LOGC(inlog.Error, log << "pthread_join failed with " << ret << " (" << m_thread << ")"); + } #ifdef HEAVY_LOGGING - else - { - HLOGC(inlog.Debug, log << "pthread_join SUCCEEDED"); - } + else + { + HLOGC(inlog.Debug, log << "pthread_join SUCCEEDED"); + } #endif - // After joining, joinable should be false + // After joining, joinable should be false + } m_thread = pthread_t(); return; } @@ -449,6 +461,7 @@ void srt::sync::CThread::create(void *(*start_routine) (void *), void *arg) LOGC(inlog.Error, log << "pthread_create failed with " << st); throw CThreadException(MJ_SYSTEMRES, MN_THREAD, 0); } + m_pid = getpid(); } From 9b97cfa235c1778262325e84d080aad8ac34de89 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Mon, 1 Sep 2025 14:28:01 +0200 Subject: [PATCH 13/25] [core] Fixed reentrancy problem of srt_strerror (#3214) * [core] Fixed reentrancy problem of srt_strerror * [docs] Updated documentation for srt_strerror --------- Co-authored-by: Mikolaj Malecki --- docs/API/API-functions.md | 12 ++++++------ srtcore/srt_c_api.cpp | 14 ++++++++++---- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/docs/API/API-functions.md b/docs/API/API-functions.md index 2fdb8a661..928fd6aaa 100644 --- a/docs/API/API-functions.md +++ b/docs/API/API-functions.md @@ -2841,13 +2841,13 @@ associated with the last error. The system error is: const char* srt_strerror(int code, int errnoval); ``` -Returns a string message that represents a given SRT error code and possibly the -`errno` value, if not 0. +Returns a string message that represents a given SRT error code. -**NOTE:** *This function isn't thread safe. It uses a static variable to hold the -error description. There's no problem with using it in a multithreaded environment, -as long as only one thread in the whole application calls this function at the -moment* +**NOTE:** *The `errnoval` parameter is ignored. This function's old version +was intended to get both the SRT error description and system error description, +but this requires resolution of the reentrancy problem and dynamic strings. +For getting the error description for a system error, you need to use the +`strerror` function or some of its reentrant version.* [:arrow_up:   Back to List of Functions & Structures](#srt-api-functions) diff --git a/srtcore/srt_c_api.cpp b/srtcore/srt_c_api.cpp index 031030cd7..cf82467c2 100644 --- a/srtcore/srt_c_api.cpp +++ b/srtcore/srt_c_api.cpp @@ -26,6 +26,14 @@ written by using namespace std; using namespace srt; +namespace srt +{ +// This is provided in strerror_defs.cpp, which doesn't have +// its header file. +// XXX Consider adding some static function to CUDTException. +const char* strerror_get_message(size_t major, size_t minor); +} + extern "C" { @@ -248,11 +256,9 @@ int srt_getlasterror(int* loc_errno) return CUDT::getlasterror().getErrorCode(); } -const char* srt_strerror(int code, int err) +const char* srt_strerror(int code, int /*err ignored*/) { - static srt::CUDTException e; - e = srt::CUDTException(CodeMajor(code/1000), CodeMinor(code%1000), err); - return(e.getErrorMessage()); + return strerror_get_message(CodeMajor(code/1000), CodeMinor(code%1000)); } From 52ceecdf5190885914f0f94d01be32441ccb1f4c Mon Sep 17 00:00:00 2001 From: Aleksey S P <35438102+SPAleksey@users.noreply.github.com> Date: Mon, 1 Sep 2025 17:14:14 +0400 Subject: [PATCH 14/25] Fix SIGABRT when bonding option is too long (#3210) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix SIGABRT when bonding option is too long * Move the UT to test_bonding.cpp --------- Co-authored-by: Clément Gérouville --- srtcore/socketconfig.cpp | 2 +- test/test_bonding.cpp | 9 +++++++++ test/test_utilities.cpp | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/srtcore/socketconfig.cpp b/srtcore/socketconfig.cpp index a0c8a6a47..1b94c9197 100644 --- a/srtcore/socketconfig.cpp +++ b/srtcore/socketconfig.cpp @@ -1092,7 +1092,7 @@ bool SRT_SocketOptionObject::add(SRT_SOCKOPT optname, const void* optval, size_t // needed - and it's better to not risk that alighment rules // will make these calculations result in less space than needed. const size_t headersize = sizeof(SingleOption); - const size_t payload = std::min(sizeof(uint32_t), optlen); + const size_t payload = std::max(sizeof(uint32_t), optlen); unsigned char* mem = new unsigned char[headersize + payload]; SingleOption* option = reinterpret_cast(mem); option->option = optname; diff --git a/test/test_bonding.cpp b/test/test_bonding.cpp index 6307fd21f..982145fae 100644 --- a/test/test_bonding.cpp +++ b/test/test_bonding.cpp @@ -1594,3 +1594,12 @@ TEST(Bonding, BackupPrioritySelection) } +TEST(Bonding, ApiConfig) +{ + using namespace std; + SRT_SOCKOPT_CONFIG config; + + string example = "example_long_excessively"; + + EXPECT_EQ(srt_config_add(&config, SRTO_BINDTODEVICE, (void*)example.data(), example.size()), 0); +} diff --git a/test/test_utilities.cpp b/test/test_utilities.cpp index 28c14044e..e8494e744 100644 --- a/test/test_utilities.cpp +++ b/test/test_utilities.cpp @@ -272,3 +272,4 @@ TEST(ConfigString, Setting) EXPECT_EQ(s.size(), 0U); EXPECT_TRUE(s.empty()); } + From bab403744b4d02005b03dac12a79988bc4590038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20G=C3=A9rouville?= Date: Wed, 3 Sep 2025 09:43:59 +0200 Subject: [PATCH 15/25] [API] raise libSRT version number to 1.5.5 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2017139ed..a7e979513 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,7 @@ # cmake_minimum_required (VERSION 3.5 FATAL_ERROR) -set (SRT_VERSION 1.5.4) +set (SRT_VERSION 1.5.5) set (CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/scripts") include(CheckSymbolExists) From 5c5f5b5f3ac52c1d453c98bb945b6a39309a32cb Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Wed, 15 Oct 2025 13:23:41 +0200 Subject: [PATCH 16/25] [MAINT] Add fixes for CI configuration based on latest dev (#3232) * [MAINT] Add fixes for CI configuration based on latest dev * Added fixes from another PR * Added lacking scripts and repos * Fixed error reported in CI. Fixed ABI entry --------- Co-authored-by: Mikolaj Malecki --- .github/workflows/abi.yml | 139 ++++++++++++++++++++++------ .github/workflows/android.yaml | 4 +- .github/workflows/codeql.yml | 4 +- .github/workflows/cxx03-ubuntu.yaml | 29 ++++++ .github/workflows/cxx11-macos.yaml | 16 ++-- .github/workflows/cxx11-ubuntu.yaml | 6 +- .github/workflows/cxx11-win.yaml | 8 +- .github/workflows/iOS.yaml | 10 +- .github/workflows/s390x-focal.yaml | 4 +- .gitmodules | 3 + .travis.yml | 116 +++++++++++++++-------- scripts/abi-check.tcl | 76 +++++++++++++++ scripts/build-windows.ps1 | 24 ++++- scripts/get-build-version.tcl | 47 ++++++++++ scripts/googletest-download.cmake | 4 +- srtcore/utilities.h | 17 ++-- submodules/abi-compliance-checker | 1 + 17 files changed, 402 insertions(+), 106 deletions(-) create mode 100644 .github/workflows/cxx03-ubuntu.yaml create mode 100644 .gitmodules create mode 100644 scripts/abi-check.tcl create mode 100755 scripts/get-build-version.tcl create mode 160000 submodules/abi-compliance-checker diff --git a/.github/workflows/abi.yml b/.github/workflows/abi.yml index 51d22a516..4d4d6012c 100644 --- a/.github/workflows/abi.yml +++ b/.github/workflows/abi.yml @@ -2,60 +2,141 @@ name: ABI checks on: push: - branches: [ master ] + branches: [ "master", "dev" ] pull_request: - branches: [ master ] + branches: [ "master", "dev" ] env: SRT_BASE: v1.5.0 jobs: - build: - name: ABI checks + build_pr: + name: Build current version runs-on: ubuntu-latest + outputs: + base_version: ${{ steps.build.outputs.SRT_BASE }} + tag_version: ${{ steps.build.outputs.SRT_TAG_VERSION }} steps: - uses: actions/checkout@v3 with: - path: pull_request - - name: configure + path: gitview_pr + - id: configure + name: Configure (cmake) run: | - cd pull_request + cd gitview_pr mkdir _build && cd _build cmake -DCMAKE_BUILD_TYPE=Debug ../ - - name: build + - id: build + name: Build and dump run: | sudo apt install -y abi-dumper - cd pull_request/_build && cmake --build ./ + sudo apt install -y tcl + cd gitview_pr/_build && cmake --build ./ make install DESTDIR=./installdir - SRT_TAG_VERSION=$(cat version.h |grep SRT_VERSION_MINOR |head -n1 |awk {'print $3'}) - abi-dumper libsrt.so -o libsrt-pr.dump -public-headers installdir/usr/local/include/srt/ -lver 0 - SRT_BASE="v1.$SRT_TAG_VERSION.0" - echo "SRT_BASE=$SRT_BASE" >> "$GITHUB_ENV" + SRT_TAG_VERSION=v$(../scripts/get-build-version.tcl full) + echo "SRT_TAG_VERSION=$SRT_TAG_VERSION" >> "$GITHUB_OUTPUT" + SRT_TAG=${SRT_TAG_VERSION}dev-$(git rev-parse --short HEAD) + echo "TAGGING PR BUILD: $SRT_TAG" + abi-dumper libsrt.so -o libsrt-pr.dump -public-headers installdir/usr/local/include/srt/ -lver $SRT_TAG + ls -ld libsrt-pr.dump + sha256sum libsrt-pr.dump + SRT_BASE=v$(../scripts/get-build-version.tcl base) + if [[ $SRT_TAG_VERSION == $SRT_BASE ]]; then + echo "NOT CHECKING ABI: base version is being built: $SRT_TAG (not emitting SRT_BASE)" + #echo "SRT_BASE=''" >> "$GITHUB_OUTPUT" + else + echo "WILL CHECK ABI changes $SRT_BASE - $SRT_TAG_VERSION" + echo "SRT_BASE=$SRT_BASE" >> "$GITHUB_OUTPUT" + fi + - id: upload_pr_dump + uses: actions/upload-artifact@v4 + with: + name: abidump_pr + path: gitview_pr/_build/libsrt-pr.dump + + build_base: + name: Build base version + runs-on: ubuntu-latest + needs: build_pr + if: ${{ needs.build_pr.outputs.base_version != '' }} + + env: + SRT_BASE: ${{ needs.build_pr.outputs.base_version }} + SRT_TAG_VERSION: ${{ needs.build_pr.outputs.tag_version }} + steps: - uses: actions/checkout@v3 with: - path: tag + path: gitview_base ref: ${{ env.SRT_BASE }} - - name: configure_tag + - id: configure_tag + name: Configure (cmake) run: | - echo $SRT_TAG_VERSION - cd tag + echo "TAG:$SRT_TAG_VERSION BASE:$SRT_BASE" + + #This is currently a paranoid check - the if should do the job + if [[ -z $SRT_BASE ]]; then + echo "NO BASE DEFINED. NOT BUILDING" + exit 1 + fi + cd gitview_base mkdir _build && cd _build cmake -DCMAKE_BUILD_TYPE=Debug ../ - - name: build_tag + - id: build_tag + name: Build and dump + if: ${{ success() }} run: | - cd tag + sudo apt install -y abi-dumper + sudo apt install -y tcl + cd gitview_base cd _build && cmake --build ./ make install DESTDIR=./installdir - abi-dumper libsrt.so -o libsrt-tag.dump -public-headers installdir/usr/local/include/srt/ -lver 1 + echo "TAGGING BASE BUILD: $SRT_BASE" + abi-dumper libsrt.so -o libsrt-base.dump -public-headers installdir/usr/local/include/srt/ -lver $SRT_BASE + ls -ld libsrt-base.dump + sha256sum libsrt-base.dump + - id: upload_base_dump + uses: actions/upload-artifact@v4 + with: + name: abidump_base + path: gitview_base/_build/libsrt-base.dump + + check_abi: + name: Compare ABI + runs-on: ubuntu-latest + needs: [build_pr, build_base] + env: + SRT_BASE: ${{ needs.build_pr.outputs.base_version }} + + steps: + - name: Download base dump + uses: actions/download-artifact@v4 + with: + name: abidump_base + path: . + - name: Download pr dump + uses: actions/download-artifact@v4 + with: + name: abidump_pr + path: . - name: abi-check run: | - git clone https://github.com/lvc/abi-compliance-checker.git - cd abi-compliance-checker && sudo make install && cd ../ - abi-compliance-checker -l libsrt -old tag/_build/libsrt-tag.dump -new pull_request/_build/libsrt-pr.dump - RES=$? - if (( $RES != 0 )) - then - echo "ABI/API Compatibility check failed with value $?" - exit $RES - fi + git clone https://github.com/lvc/abi-compliance-checker.git + #cd gitview_pr/submodules + #git submodule update --init abi-compliance-checker + cd abi-compliance-checker && sudo make install && cd ../ + #cd ../.. + echo "FILESYSTEM state before running abi-check at $PWD" + ls -l + sha256sum libsrt-base.dump + sha256sum libsrt-pr.dump + abi-compliance-checker -l libsrt -old libsrt-base.dump -new libsrt-pr.dump + RES=$? + if (( $RES != 0 )); then + echo "ABI/API Compatibility check failed with value $?" + exit $RES + fi + - name: Download report + uses: actions/download-artifact@v4 + with: + path: compat_reports diff --git a/.github/workflows/android.yaml b/.github/workflows/android.yaml index 5e11c75df..c94c0bb3c 100644 --- a/.github/workflows/android.yaml +++ b/.github/workflows/android.yaml @@ -2,9 +2,9 @@ name: Android on: push: - branches: [ master ] + branches: [ "master", "dev" ] pull_request: - branches: [ master ] + branches: [ "master", "dev" ] jobs: build: diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 4a337b6ca..f7f417259 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -2,9 +2,9 @@ name: "CodeQL" on: push: - branches: [ "master", "experimental/socket-groups" ] + branches: [ "master", "dev" ] pull_request: - branches: [ "master" ] + branches: [ "master", "dev" ] jobs: analyze: diff --git a/.github/workflows/cxx03-ubuntu.yaml b/.github/workflows/cxx03-ubuntu.yaml new file mode 100644 index 000000000..ce27436aa --- /dev/null +++ b/.github/workflows/cxx03-ubuntu.yaml @@ -0,0 +1,29 @@ +name: C++03 (old compat) + +on: + push: + branches: [ "master", "dev" ] + pull_request: + branches: [ "master", "dev" ] + types: [opened, synchronize, reopened] +jobs: + build: + name: ubuntu + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: configure + run: | + mkdir _build && cd _build + cmake ../ -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DENABLE_STDCXX_SYNC=OFF -DUSE_CXX_STD=c++03 -DENABLE_ENCRYPTION=ON -DENABLE_UNITTESTS=ON -DENABLE_BONDING=ON -DENABLE_TESTING=ON -DENABLE_EXAMPLES=ON -DENABLE_CODE_COVERAGE=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON + - name: build + # That below is likely SonarQube remains, which was removed earlier. + #run: cd _build && build-wrapper-linux-x86-64 --out-dir ${{ env.BUILD_WRAPPER_OUT_DIR }} cmake --build . + run: cd _build && cmake --build . + - name: test + run: | + cd _build && ctest --extra-verbose + - name: codecov + run: | + source ./scripts/collect-gcov.sh + bash <(curl -s https://codecov.io/bash) diff --git a/.github/workflows/cxx11-macos.yaml b/.github/workflows/cxx11-macos.yaml index 4732c2abb..b03b4f75f 100644 --- a/.github/workflows/cxx11-macos.yaml +++ b/.github/workflows/cxx11-macos.yaml @@ -1,10 +1,10 @@ -name: cxx11 +name: C++11 on: push: - branches: [ master ] + branches: [ "master", "dev" ] pull_request: - branches: [ master ] + branches: [ "master", "dev" ] jobs: build: @@ -14,13 +14,17 @@ jobs: steps: - name: GoogleTest run: | - curl -o googletest.rb https://raw.githubusercontent.com/Homebrew/homebrew-core/23e7fb4dc0cc73facc3772815741e1deb87d6406/Formula/googletest.rb - brew install -s googletest.rb + brew tap-new Haivision/gt-local + brew extract --version=1.12.1 --force googletest Haivision/gt-local + brew install googletest@1.12.1 + # NOTE: 1.12.1 is the last version that requires C++11; might need update later + # curl -o googletest.rb https://raw.githubusercontent.com/Homebrew/homebrew-core/23e7fb4dc0cc73facc3772815741e1deb87d6406/Formula/googletest.rb + # brew install -s googletest.rb - uses: actions/checkout@v3 - name: configure run: | mkdir _build && cd _build - cmake ../ -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DENABLE_STDCXX_SYNC=ON -DENABLE_ENCRYPTION=OFF -DENABLE_UNITTESTS=ON -DENABLE_BONDING=ON -DUSE_CXX_STD=14 + cmake ../ -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DENABLE_STDCXX_SYNC=ON -DENABLE_ENCRYPTION=OFF -DENABLE_UNITTESTS=ON -DENABLE_BONDING=ON -DUSE_CXX_STD=17 - name: build run: cd _build && cmake --build ./ - name: test diff --git a/.github/workflows/cxx11-ubuntu.yaml b/.github/workflows/cxx11-ubuntu.yaml index 2f362d84f..eda7ecca3 100644 --- a/.github/workflows/cxx11-ubuntu.yaml +++ b/.github/workflows/cxx11-ubuntu.yaml @@ -1,10 +1,10 @@ -name: cxx11 +name: C++11 on: push: - branches: [ master ] + branches: [ "master", "dev" ] pull_request: - branches: [ master ] + branches: [ "master", "dev" ] types: [opened, synchronize, reopened] jobs: build: diff --git a/.github/workflows/cxx11-win.yaml b/.github/workflows/cxx11-win.yaml index f1554053d..2947809ab 100644 --- a/.github/workflows/cxx11-win.yaml +++ b/.github/workflows/cxx11-win.yaml @@ -1,10 +1,10 @@ -name: cxx11 +name: C++11 on: push: - branches: [ master ] + branches: [ "master", "dev" ] pull_request: - branches: [ master ] + branches: [ "master", "dev" ] jobs: build: @@ -17,7 +17,7 @@ jobs: - name: configure run: | md _build && cd _build - cmake ../ -DENABLE_STDCXX_SYNC=ON -DENABLE_ENCRYPTION=OFF -DENABLE_UNITTESTS=ON -DENABLE_BONDING=ON -DUSE_CXX_STD=c++11 + cmake ../ -DENABLE_STDCXX_SYNC=ON -DENABLE_ENCRYPTION=OFF -DENABLE_UNITTESTS=ON -DENABLE_BONDING=ON -DENABLE_LOCALIF_WIN32=ON -DUSE_CXX_STD=c++11 - name: build run: cd _build && cmake --build ./ --config Release --verbose - name: test diff --git a/.github/workflows/iOS.yaml b/.github/workflows/iOS.yaml index 0fb11542b..9ae9e9124 100644 --- a/.github/workflows/iOS.yaml +++ b/.github/workflows/iOS.yaml @@ -2,9 +2,9 @@ name: iOS on: push: - branches: [ master ] + branches: [ "master", "dev" ] pull_request: - branches: [ master ] + branches: [ "master", "dev" ] jobs: build: @@ -17,9 +17,13 @@ jobs: steps: - uses: actions/checkout@v3 + - name: Install dependency packages + run: | + brew install make llvm - name: configure run: | mkdir _build && cd _build - cmake ../ -DENABLE_ENCRYPTION=OFF -DENABLE_STDCXX_SYNC=${{matrix.cxxstdsync}} -DENABLE_UNITTESTS=OFF -DENABLE_BONDING=ON --toolchain scripts/iOS.cmake + export PATH="/opt/homebrew/opt/llvm/bin:$PATH" CC=clang CXX=clang++ + cmake .. -DCMAKE_MAKE_PROGRAM=gmake -DENABLE_ENCRYPTION=OFF -DENABLE_STDCXX_SYNC=${{matrix.cxxstdsync}} -DENABLE_MONOTONIC_CLOCK=OFF -DENABLE_UNITTESTS=OFF -DUSE_CXX_STD=c++11 -DENABLE_BONDING=ON --toolchain scripts/iOS.cmake - name: build run: cd _build && cmake --build ./ diff --git a/.github/workflows/s390x-focal.yaml b/.github/workflows/s390x-focal.yaml index f1b6c7508..d91991456 100644 --- a/.github/workflows/s390x-focal.yaml +++ b/.github/workflows/s390x-focal.yaml @@ -2,9 +2,9 @@ name: QEMU to run s390x-focal on: push: - branches: [ master ] + branches: [ "master", "dev" ] pull_request: - branches: [ master ] + branches: [ "master", "dev" ] jobs: Tests: diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 000000000..0c81e9233 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "submodules/abi-compliance-checker"] + path = submodules/abi-compliance-checker + url = https://github.com/lvc/abi-compliance-checker.git diff --git a/.travis.yml b/.travis.yml index 0402fb51e..5c54148ea 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,3 +1,10 @@ +# TRAVIS Configuration file +# +# XXX MIND SYNTAX TRICKS: +# - Keep column consistency +# - TAB characters NOT ALLOWED anywhere +# - COLON characters (in freestanding strings) not allowed in the script + language: cpp dist: xenial @@ -16,27 +23,27 @@ addons: packages: - openssl -matrix: +jobs: include: - os: linux env: - - BUILD_TYPE=Debug - - BUILD_OPTS='-DENABLE_BONDING=ON -DCMAKE_CXX_FLAGS="-Werror"' + - BUILD_TYPE=Debug CFG="monotonic openssl werror" + - CMAKE_OPTS='-DENABLE_BONDING=ON -DCMAKE_CXX_FLAGS="-Werror"' - env: - - BUILD_TYPE=Debug - - BUILD_OPTS='-DENABLE_LOGGING=OFF -DUSE_ENCLIB=mbedtls -DENABLE_MONOTONIC_CLOCK=ON -DENABLE_BONDING=ON -DCMAKE_CXX_FLAGS="-Werror"' + - BUILD_TYPE=Debug CFG="nologging mbedtls monotonic werror" + - CMAKE_OPTS='-DENABLE_LOGGING=OFF -DUSE_ENCLIB=mbedtls -DENABLE_MONOTONIC_CLOCK=ON -DENABLE_BONDING=ON -DCMAKE_CXX_FLAGS="-Werror"' - os: linux - env: BUILD_TYPE=Release - - os: osx - osx_image: xcode11.1 - env: - - BUILD_TYPE=Debug - - BUILD_OPTS='-DCMAKE_CXX_FLAGS="-Werror"' - - os: osx - osx_image: xcode11.1 - env: - - BUILD_TYPE=Release - - BUILD_OPTS='-DCMAKE_CXX_FLAGS="-Werror"' + env: BUILD_TYPE=Release CFG=default +# - os: osx +# osx_image: xcode11.1 +# env: +# - BUILD_TYPE=Debug CFG=werror +# - CMAKE_OPTS='-DCMAKE_CXX_FLAGS="-Werror"' +# - os: osx +# osx_image: xcode11.1 +# env: +# - BUILD_TYPE=Release CFG=werror +# - CMAKE_OPTS='-DCMAKE_CXX_FLAGS="-Werror"' - os: linux compiler: x86_64-w64-mingw32-g++ addons: @@ -53,37 +60,70 @@ matrix: - ./Configure --cross-compile-prefix=x86_64-w64-mingw32- mingw64 - make - cd .. - env: BUILD_TYPE=Release + env: BUILD_TYPE=Release CFG=no-UT # Power jobs + # Forcing Focal distro because Xenial + # has somehow outdated CMake - os: linux arch: ppc64le + dist: focal env: - - BUILD_TYPE=Debug + - ARCH=PowerPC BUILD_TYPE=Debug - arch: ppc64le + dist: focal env: - - BUILD_TYPE=Release - - BUILD_OPTS='-DENABLE_MONOTONIC_CLOCK=ON' + - ARCH=PowerPC BUILD_TYPE=Release CFG=monotonic + - CMAKE_OPTS='-DENABLE_MONOTONIC_CLOCK=ON' script: + - CMAKE_VERSION=$(cmake --version | head -1 | awk '{print $3}') + - echo CMAKE version $CMAKE_VERSION + - CMAKE_VERSION_OK=$(echo "puts [package vcompare $CMAKE_VERSION 3.10]" | tclsh); + - if [ "$CMAKE_VERSION_OK" == "-1" ]; then + echo "ERROR - cmake version too old, >=3.10 required"; + exit 1; + fi; + - export REQUIRE_UNITTESTS=1 + - if [ "$TRAVIS_COMPILER" == "x86_64-w64-mingw32-g++" ]; then + CMAKE_OPTS+=" -DCMAKE_C_COMPILER=x86_64-w64-mingw32-gcc"; + CMAKE_OPTS+=" -DCMAKE_CXX_COMPILER=x86_64-w64-mingw32-g++"; + CMAKE_OPTS+=" -DENABLE_STDCXX_SYNC=OFF -DENABLE_LOCALIF_WIN32=OFF -DENABLE_UNITTESTS=OFF -DUSE_OPENSSL_PC=OFF"; + CMAKE_OPTS+=" -DOPENSSL_ROOT_DIR=$PWD/openssl"; + CMAKE_OPTS+=" -DOPENSSL_CRYPTO_LIBRARY=$PWD/openssl/libcrypto-1_1-x64.dll"; + CMAKE_OPTS+=" -DCMAKE_SYSTEM_NAME=Windows"; + REQUIRE_UNITTESTS=0; + fi; + if [ "$TRAVIS_OS_NAME" == "osx" ]; then + export PKG_CONFIG_PATH="$(brew --prefix openssl)/lib/pkgconfig"; + fi; + if (( $REQUIRE_UNITTESTS )); then + CMAKE_OPTS+=" -DENABLE_UNITTESTS=ON"; + fi; - echo COMPILER $TRAVIS_COMPILER - echo SYSTEM $TRAVIS_OS_NAME - echo BUILD_TYPE $BUILD_TYPE - - echo BUILD_OPTS $BUILD_OPTS - - if [ "$TRAVIS_COMPILER" == "x86_64-w64-mingw32-g++" ]; then - export CC="x86_64-w64-mingw32-gcc"; - export CXX="x86_64-w64-mingw32-g++"; - cmake . -DCMAKE_BUILD_TYPE=$BUILD_TYPE $BUILD_OPTS -DENABLE_UNITTESTS="OFF" -DUSE_OPENSSL_PC="OFF" -DOPENSSL_ROOT_DIR="$PWD/openssl" -DCMAKE_SYSTEM_NAME="Windows"; - elif [ "$TRAVIS_OS_NAME" == "linux" ]; then - cmake . -DCMAKE_BUILD_TYPE=$BUILD_TYPE $BUILD_OPTS -DENABLE_UNITTESTS="ON"; - elif [ "$TRAVIS_OS_NAME" == "osx" ]; then - export PKG_CONFIG_PATH=$(brew --prefix openssl)"/lib/pkgconfig"; - cmake . -DCMAKE_BUILD_TYPE=$BUILD_TYPE $BUILD_OPTS -DENABLE_UNITTESTS="ON"; - fi - - make -j$(nproc); - - if [ "$TRAVIS_COMPILER" != "x86_64-w64-mingw32-g++" ]; then + - echo CMAKE_OPTS $CMAKE_OPTS + - export SUCCESS=0 + - cmake . --debug-output -DCMAKE_MESSAGE_LOG_LEVEL=VERBOSE -DCMAKE_BUILD_TYPE=$BUILD_TYPE $CMAKE_OPTS 2>&1 || SUCCESS=$?; + - if (($SUCCESS == 0)); then + echo "Configure OK"; + else + echo "-- OUTPUT --"; + cat CMakeFiles/CMakeOutput.log || echo "NO OUTPUT"; + echo "-- ERRORS --"; + cat CMakeFiles/CMakeError.log || echo "NO LOGS"; + exit 1; + fi; + - make VERBOSE=1 -j$(nproc); + - if (( $REQUIRE_UNITTESTS )); then ulimit -c unlimited; - ./test-srt -disable-ipv6; - SUCCESS=$?; - if [ -f core ]; then gdb -batch ./test-srt -c core -ex bt -ex "info thread" -ex quit; else echo "NO CORE - NO CRY!"; fi; - test $SUCCESS == 0; - fi + if [ ! -f ./test-srt ]; then + echo "ERROR - UT application not found"; + false; + else + ./test-srt -disable-ipv6; + SUCCESS=$?; + if [ -f core ]; then gdb -batch ./test-srt -c core -ex bt -ex "info thread" -ex quit; else echo "NO CORE - NO CRY!"; fi; + test $SUCCESS == 0; + fi; + fi; diff --git a/scripts/abi-check.tcl b/scripts/abi-check.tcl new file mode 100644 index 000000000..7f9c70e79 --- /dev/null +++ b/scripts/abi-check.tcl @@ -0,0 +1,76 @@ +#!/usr/bin/tclsh + +set here [file dirname $argv0] ;# points to [git top]/scripts +set top [file normalize $here/..] + +set abichecker [file normalize [file join $top submodules abi-compliance-checker abi-compliance-checker.pl]] + +if { ![file exists $abichecker] } { + puts stderr "Please update submodules first (compliance checker not found in the current view)" + exit 1 +} + +# Check if abi-dumper is installed + +set abidumper [auto_execok abi-dumper] +if {$abidumper == ""} { + set installer "" + foreach ii {zypper dnf apt} { + if {[auto_execok $ii] != ""} { + set installer $ii + break + } + } + if {$installer != ""} { + puts stderr "ABI dumper not installed. Use such commands to install\n" + puts stderr " $installer install abi-dumper" + } else { + puts stderr "ABI dumper not installed. Find out how to install abi-dumper in your system." + } + exit 1 +} + +# Arguments: +# [directory-with-base] [directory-with-pr] + +# NOTE: ABI dump will be done in every build directory as specified. + +proc generate-abi-dump {directory lver} { + set wd [pwd] + cd $directory + global top + + # You should have libsrt.so in this directory. + # Use [glob] to use exception if no file exists + glob libsrt.so + + exec >@stdout 2>@stderr abi-dumper libsrt.so -o libsrt-abi.dump -public-headers $top/srtcore -lver $lver + cd $wd +} + +set olddir [lindex $argv 0] +set newdir [lindex $argv 1] + +if {![file isdirectory $olddir] || ![file isdirectory $newdir]} { + puts stderr "Wrong arguments. Required build directory" + exit 1 +} + +set wd [pwd] +cd $olddir +set base_ver [exec $top/scripts/get-build-version.tcl] +cd $wd +cd $newdir +set new_ver [exec $top/scripts/get-build-version.tcl] +cd $wd + +generate-abi-dump $olddir $base_ver +generate-abi-dump $newdir $new_ver + +set res [catch {exec >@stdout 2>@stderr $abichecker -l libsrt -old $olddir/libsrt-abi.dump -new $newdir/libsrt-abi.dump} out] + +if {$res} { + puts stderr "ABI compat problems found!!!\nSee the HTML report" +} + + diff --git a/scripts/build-windows.ps1 b/scripts/build-windows.ps1 index c4c7b5740..a65247ccd 100644 --- a/scripts/build-windows.ps1 +++ b/scripts/build-windows.ps1 @@ -49,11 +49,25 @@ if ( $Env:APPVEYOR ) { $CONFIGURATION = $Env:CONFIGURATION - #appveyor has many openssl installations - place the latest one in the default location unless VS2013 - Remove-Item -Path "C:\OpenSSL-Win32" -Recurse -Force -ErrorAction SilentlyContinue | Out-Null - Remove-Item -Path "C:\OpenSSL-Win64" -Recurse -Force -ErrorAction SilentlyContinue | Out-Null - Copy-Item -Path "C:\OpenSSL-v111-Win32" "C:\OpenSSL-Win32" -Recurse | Out-Null - Copy-Item -Path "C:\OpenSSL-v111-Win64" "C:\OpenSSL-Win64" -Recurse | Out-Null + # See https://www.appveyor.com/docs/windows-images-software/#visual-studio-2022 + # According to AppVeyor environment definition: + # + # 2013 - 2017: C:\OpenSSL-Win*: v1.0.2p, C:\OpenSSL-v111-Win*: v1.1.1 (v1.1.1d for 2015+) + # 2019 - 2022: C:\OpenSSL-Win*: v1.1.1w and C:\OpenSSL-v3* with version 3+ + # SO: + # For 2013 - 2017: Delete C:\OpenSSL-Win* and replace it with contents of C:\OpenSSL-v111-Win* + # For 2019 - 2022: Do nothing. + # Note: No guarantee that C:\OpenSSL-Win* directories will always contain version v1.1.1+ in + # any future versions for AppVeyor Windows environment, but then probably the SRT project + # would have to be adjusted to this version anyway. + + if ($VS_VERSION -lt 2019) { + #appveyor has many openssl installations - place the latest one in the default location unless VS2013 + Remove-Item -Path "C:\OpenSSL-Win32" -Recurse -Force -ErrorAction SilentlyContinue | Out-Null + Remove-Item -Path "C:\OpenSSL-Win64" -Recurse -Force -ErrorAction SilentlyContinue | Out-Null + Copy-Item -Path "C:\OpenSSL-v111-Win32" "C:\OpenSSL-Win32" -Recurse | Out-Null + Copy-Item -Path "C:\OpenSSL-v111-Win64" "C:\OpenSSL-Win64" -Recurse | Out-Null + } } # persist VS_VERSION so it can be used in an artifact name later diff --git a/scripts/get-build-version.tcl b/scripts/get-build-version.tcl new file mode 100755 index 000000000..fafda061a --- /dev/null +++ b/scripts/get-build-version.tcl @@ -0,0 +1,47 @@ +#!/usr/bin/tclsh + +if {![file exists version.h]} { + puts stderr "No version.h file found - run this in the build directory!" + exit 1 +} + +set fd [open version.h r] + +set version "" +set line "" +while {[gets $fd line] != -1} { + # Use regexp because this is safer to avoid + # unexpected list interpretation mistakes + if {[regexp {#define SRT_VERSION_STRING} $line]} { + # Once matched this way, we can safely use tcl-list-like access + set version [lindex $line 2 0] + break + } +} +close $fd + +if {$version == ""} { + puts stderr "No version extracted (no SRT_VERSION_STRING found)" + exit 1 +} + +lassign $argv model part + +if {$model == ""} { + set model current +} + +lassign [split $version .] major minor patch + +if {$part == "minor"} { + set prefix "$minor" +} else { + set prefix "$major.$minor" +} + +if {$model == "base"} { + puts "$prefix.0" +} else { + puts "$prefix.$patch" +} + diff --git a/scripts/googletest-download.cmake b/scripts/googletest-download.cmake index 36554f316..f1f6d4902 100644 --- a/scripts/googletest-download.cmake +++ b/scripts/googletest-download.cmake @@ -1,7 +1,7 @@ # code copied from https://crascit.com/2015/07/25/cmake-gtest/ -cmake_minimum_required(VERSION 2.8.12 FATAL_ERROR) +cmake_minimum_required(VERSION 3.10 FATAL_ERROR) -project(googletest-download NONE) +project(GOOGLETEST_DOWNLOAD) include(ExternalProject) diff --git a/srtcore/utilities.h b/srtcore/utilities.h index ca8c365a3..8781b80e7 100644 --- a/srtcore/utilities.h +++ b/srtcore/utilities.h @@ -828,17 +828,14 @@ struct CallbackHolder // Test if the pointer is a pointer to function. Don't let // other type of pointers here. #if HAVE_CXX11 + // NOTE: No poor-man's replacement can be done for C++03 because it's + // not possible to fake calling a function without calling it and no + // other operation can be done without extensive transformations on + // the Signature type, still in C++03 possible only on functions up to + // 2 arguments (callbacks in SRT usually have more). static_assert(std::is_function::value, "CallbackHolder is for functions only!"); -#else - // This is a poor-man's replacement, which should in most compilers - // generate a warning, if `Signature` resolves to a value type. - // This would make an illegal pointer cast from a value to a function type. - // Casting function-to-function, however, should not. Unfortunately - // newer compilers disallow that, too (when a signature differs), but - // then they should better use the C++11 way, much more reliable and safer. - void* (*testfn)(void*) = (void*(*)(void*))f; - (void)(testfn); #endif + opaque = o; fn = f; } @@ -1111,7 +1108,7 @@ struct MapProxy { typename std::map::const_iterator p = find(); if (p == mp.end()) - return ""; + return ValueType(); return p->second; } diff --git a/submodules/abi-compliance-checker b/submodules/abi-compliance-checker new file mode 160000 index 000000000..7c175c45a --- /dev/null +++ b/submodules/abi-compliance-checker @@ -0,0 +1 @@ +Subproject commit 7c175c45a8ba9ac41b8e47d8ebbab557b623b18e From 84676b50be5e14b043b781257ebd9faa14e7a540 Mon Sep 17 00:00:00 2001 From: cl-ment Date: Tue, 21 Oct 2025 16:59:14 +0200 Subject: [PATCH 17/25] [build] Avoid using list(PREPEND). (#3230) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Clément Gérouville --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a7e979513..6e19caa2c 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -966,7 +966,7 @@ else () message(STATUS "Pthread library: ${PTHREAD_LIBRARY}") endif() -list(PREPEND CMAKE_REQUIRED_LIBRARIES "${PTHREAD_LIBRARY}") +set(CMAKE_REQUIRED_LIBRARIES "${PTHREAD_LIBRARY};${CMAKE_REQUIRED_LIBRARIES}") unset(CMAKE_REQUIRED_QUIET) check_symbol_exists(pthread_atfork "pthread.h" HAVE_PTHREAD_ATFORK) From 547688cce7db6949dd8236507331cbe2a14cd06b Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Wed, 22 Oct 2025 08:55:08 +0200 Subject: [PATCH 18/25] [core] Restored old cookie contest. Added development support for enforcing cookie values. (#3227) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Initial testing version of the cookie contest fix * Setting cookie contest 1.5.4 as default * [core] Restored old cookie contest. Added ignored wrong peer response on cookie collision * Fixed security report * Removed the (unnecessary now) fix for the future version compatibility * Update test/test_common.cpp - fixed a typo in a comment * Fixed TestFEC tests to be resistant of a listener-readiness bug (fixed in dev) * Try to fix the first issue * Removed remains after the screw-in cookie procedures --------- Co-authored-by: Mikolaj Malecki Co-authored-by: Clément Gérouville --- srtcore/core.cpp | 120 +++++++++++++++++------------------ srtcore/core.h | 16 +++++ test/test_common.cpp | 45 +++++++++++++ test/test_fec_rebuilding.cpp | 65 +++++++++---------- 4 files changed, 150 insertions(+), 96 deletions(-) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 0acef17bb..dd189f73c 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -4102,88 +4102,69 @@ void srt::CUDT::sendRendezvousRejection(const sockaddr_any& serv_addr, CPacket& m_pSndQueue->sendto(serv_addr, r_rsppkt, m_SourceAddr); } -void srt::CUDT::cookieContest() +enum HandshakeSide srt::CUDT::compareCookies(int32_t req, int32_t res) { - if (m_SrtHsSide != HSD_DRAW) - return; - - // Here m_ConnReq.m_iCookie is a local cookie value sent in connection request to the peer. - // m_ConnRes.m_iCookie is a cookie value sent by the peer in its connection request. - if (m_ConnReq.m_iCookie == 0 || m_ConnRes.m_iCookie == 0) + if (req < res) { - LOGC(cnlog.Debug, log << CONID() << "cookieContest: agent=" << m_ConnReq.m_iCookie << " peer=" << m_ConnRes.m_iCookie - << " - ERROR: zero not allowed!"); - - // Note that it's virtually impossible that Agent's cookie is not ready, this - // shall be considered IPE. - // Not all cookies are ready, don't start the contest. - return; + LOGC(gglog.Note, log << hex << setfill('0') << uppercase + << "REQ: " << setw(8) << req + << " RES: " << setw(8) << res << " - result: RESPONDER"); + return HSD_RESPONDER; } + else if (req > res) + { + LOGC(gglog.Note, log << hex << setfill('0') << uppercase + << "REQ: " << setw(8) << req + << " RES: " << setw(8) << res << " - result: INITIATOR"); + return HSD_INITIATOR; + } + return HSD_DRAW; +} - // INITIATOR/RESPONDER role is resolved by COOKIE CONTEST. - // - // The cookie contest must be repeated every time because it - // may change the state at some point. - // - // In SRT v1.4.3 and prior the below subtraction was performed in 32-bit arithmetic. - // The result of subtraction can overflow 32-bits. - // Example - // m_ConnReq.m_iCookie = -1480577720; - // m_ConnRes.m_iCookie = 811599203; - // int64_t llBetterCookie = -1480577720 - 811599203 = -2292176923 (FFFF FFFF 7760 27E5); - // int32_t iBetterCookie = 2002790373 (7760 27E5); - // - // Now 64-bit arithmetic is used to calculate the actual result of subtraction. - // - // In SRT v1.5.4 there was a version, that checked: - // - if LOWER 32-bits are 0, this is a draw - // - if bit 31 is set (AND with 0x80000000), the result is considered negative. - // This was erroneous because for 1 and 0x80000001 cookie values the - // result was always the same, regardless of the order: - // - // 0x0000000000000001 - 0xFFFFFFFF80000001 = 0x0000000080000000 - // 0xFFFFFFFF80000001 - 0x0000000080000000 = 0xFFFFFFFF80000000 - // - // >> if (contest & 0x80000000) -> results in true in both comparisons. - // - // This version takes the bare result of the 64-bit arithmetics. - const int64_t xreq = int64_t(m_ConnReq.m_iCookie); - const int64_t xres = int64_t(m_ConnRes.m_iCookie); +enum HandshakeSide srt::CUDT::backwardCompatibleCookieContest(int32_t req, int32_t res) +{ + const int64_t xreq = int64_t(req); + const int64_t xres = int64_t(res); const int64_t contest = xreq - xres; - // NOTE: for 1.6.0 use: - // fmtc hex64 = fmtc().uhex().fillzero().width(16); then - // << fmt(xreq, hex64) - LOGC(cnlog.Debug, log << CONID() << "cookieContest: agent=" << m_ConnReq.m_iCookie - << " peer=" << m_ConnRes.m_iCookie + LOGC(cnlog.Debug, log << "cookieContest: agent=" << req + << " peer=" << res << hex << uppercase << setfill('0') // PERSISTENT flags << " X64: "<< setw(16) << xreq << " vs. " << setw(16) << xres << " DIFF: " << setw(16) << contest); - if (contest == 0) + if ((contest & 0xFFFFFFFF) == 0) { - // DRAW! The only way to continue would be to force the - // cookies to be regenerated and to start over. But it's - // not worth a shot - this is an extremely rare case. - // This can simply do reject so that it can be started again. + return HSD_DRAW; + } + if (contest & 0x80000000) + { + const int64_t revert = xres - xreq; + if (revert & 0x80000000 && req > res) + return HSD_INITIATOR; + return HSD_RESPONDER; + } - // Pretend then that the cookie contest wasn't done so that - // it's done again. Cookies are baked every time anew, however - // the successful initial contest remains valid no matter how - // cookies will change. + return HSD_INITIATOR; +} - m_SrtHsSide = HSD_DRAW; +void srt::CUDT::cookieContest() +{ + if (m_SrtHsSide != HSD_DRAW) return; - } - if (contest < 0) + // Here m_ConnReq.m_iCookie is a local cookie value sent in connection request to the peer. + // m_ConnRes.m_iCookie is a cookie value sent by the peer in its connection request. + if (m_ConnReq.m_iCookie == 0 || m_ConnRes.m_iCookie == 0) { - m_SrtHsSide = HSD_RESPONDER; + LOGC(cnlog.Debug, log << CONID() << "cookieContest: agent=" << m_ConnReq.m_iCookie << " peer=" << m_ConnRes.m_iCookie + << " - ERROR: zero not allowed!"); + return; } + m_SrtHsSide = backwardCompatibleCookieContest(m_ConnReq.m_iCookie, m_ConnRes.m_iCookie); - m_SrtHsSide = HSD_INITIATOR; } // This function should complete the data for KMX needed for an out-of-band @@ -12153,3 +12134,18 @@ void srt::CUDT::processKeepalive(const CPacket& ctrlpkt, const time_point& tsArr if (m_config.bDriftTracer) m_pRcvBuffer->addRcvTsbPdDriftSample(ctrlpkt.getMsgTimeStamp(), tsArrival, -1); } + +namespace srt { +HandshakeSide getHandshakeSide(SRTSOCKET u) +{ + return CUDT::handshakeSide(u); +} + +HandshakeSide CUDT::handshakeSide(SRTSOCKET u) +{ + CUDTSocket *s = uglobal().locateSocket(u); + return s ? s->core().handshakeSide() : HSD_DRAW; +} +} + + diff --git a/srtcore/core.h b/srtcore/core.h index a07d17277..6bf75466e 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -294,6 +294,17 @@ class CUDT return m_ConnRes.m_iVersion; } + int32_t handshakeCookie() + { + return m_ConnReq.m_iCookie; + } + + static HandshakeSide handshakeSide(SRTSOCKET u); + HandshakeSide handshakeSide() + { + return m_SrtHsSide; + } + std::string CONID() const { #if ENABLE_LOGGING @@ -1010,6 +1021,8 @@ class CUDT public: static int installAcceptHook(SRTSOCKET lsn, srt_listen_callback_fn* hook, void* opaq); static int installConnectHook(SRTSOCKET lsn, srt_connect_callback_fn* hook, void* opaq); + static enum HandshakeSide compareCookies(int32_t req, int32_t res); + static enum HandshakeSide backwardCompatibleCookieContest(int32_t req, int32_t res); private: void installAcceptHook(srt_listen_callback_fn* hook, void* opaq) { @@ -1248,6 +1261,9 @@ class CUDT void removeEPollID(const int eid); }; +// DEBUG SUPPORT +HandshakeSide getHandshakeSide(SRTSOCKET s); + } // namespace srt #endif diff --git a/test/test_common.cpp b/test/test_common.cpp index 1a8cca061..b4e150f1b 100644 --- a/test/test_common.cpp +++ b/test/test_common.cpp @@ -5,6 +5,7 @@ #include "test_env.h" #include "utilities.h" #include "common.h" +#include "core.h" using namespace srt; @@ -71,3 +72,47 @@ TEST(CIPAddress, IPv4_in_IPv6_pton) test_cipaddress_pton(peer_ip, AF_INET6, ip); } + +void testCookieContest(int32_t agent_cookie, int32_t peer_cookie) +{ + using namespace srt; + using namespace std; + + cout << "TEST: Cookies: agent=" << hex << agent_cookie + << " peer=" << peer_cookie << endl << dec; + HandshakeSide agent_side = CUDT::backwardCompatibleCookieContest(agent_cookie, peer_cookie); + EXPECT_EQ(agent_side, HSD_INITIATOR); + HandshakeSide peer_side = CUDT::backwardCompatibleCookieContest(peer_cookie, agent_cookie); + EXPECT_EQ(peer_side, HSD_RESPONDER); +} + +TEST(Common, CookieContest) +{ + srt::TestInit srtinit; + using namespace std; + + srt_setloglevel(LOG_NOTICE); + + // In this function you should pass cookies always in the order: INITIATOR, RESPONDER. + cout << "TEST 1: two easy comparable values\n"; + testCookieContest(100, 50); + testCookieContest(-1, -1000); + testCookieContest(10055, -10000); + testCookieContest(-1480577720, 811599203); + testCookieContest( -2147483648, 2147483647); + testCookieContest(0x00000001, 0x80000001); + + /* XXX Blocked temporarily because in 1.5.5 they would fail, + and it's decided to not provide the fix, which is too + dangerous. The real fix will be provided in 1.6.0. + + // Values from PR 1517 + cout << "TEST 2: Values from PR 1517\n"; + testCookieContest(811599203, -1480577720); + testCookieContest(2147483647, -2147483648); + + cout << "TEST 3: wrong post-fix\n"; + // NOTE: 0x80000001 is a negative number in hex + testCookieContest(0x00000001, 0x80000001); + */ +} diff --git a/test/test_fec_rebuilding.cpp b/test/test_fec_rebuilding.cpp index 3a7cb0791..b39345c40 100644 --- a/test/test_fec_rebuilding.cpp +++ b/test/test_fec_rebuilding.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include "gtest/gtest.h" #include "test_env.h" @@ -92,6 +93,21 @@ class TestFECRebuilding: public srt::Test } }; +static std::future spawn_connect(SRTSOCKET s, sockaddr_in& sa, int timeout_ms = 1000) +{ + std::cout << "[M] SPAWNING srt_connect()\n"; + return std::async(std::launch::async, [s, &sa, timeout_ms]() + { + // Add a delay for starting connection to give a chance + // for the main thread to establish an EID with the listener + // BEFORE the handshake packet is received, otherwise the + // epoll will miss the signal (a bug fixed in 1.6.0). + std::this_thread::sleep_for(chrono::milliseconds(timeout_ms)); + std::cout << "[T] RUNNING srt_connect()\n"; + return srt_connect(s, (sockaddr*)& sa, sizeof(sa)); + }); +} + namespace srt { class TestMockCUDT { @@ -302,9 +318,7 @@ TEST(TestFEC, Connection) srt_listen(l, 1); - auto connect_res = std::async(std::launch::async, [&s, &sa]() { - return srt_connect(s, (sockaddr*)& sa, sizeof(sa)); - }); + auto connect_res = spawn_connect(s, sa, 1); // Make sure that the async call to srt_connect() is already kicked. std::this_thread::yield(); @@ -312,7 +326,7 @@ TEST(TestFEC, Connection) // Given 2s timeout for accepting as it has occasionally happened with Travis // that 1s might not be enough. SRTSOCKET la[] = { l }; - SRTSOCKET a = srt_accept_bond(la, 1, 2000); + SRTSOCKET a = srt_accept_bond(la, 1, 5000); ASSERT_NE(a, SRT_ERROR); EXPECT_EQ(connect_res.get(), SRT_SUCCESS); @@ -362,15 +376,13 @@ TEST(TestFEC, ConnectionReorder) srt_listen(l, 1); - auto connect_res = std::async(std::launch::async, [&s, &sa]() { - return srt_connect(s, (sockaddr*)& sa, sizeof(sa)); - }); + auto connect_res = spawn_connect(s, sa); // Make sure that the async call to srt_connect() is already kicked. std::this_thread::yield(); SRTSOCKET la[] = { l }; - SRTSOCKET a = srt_accept_bond(la, 1, 2000); + SRTSOCKET a = srt_accept_bond(la, 1, 5000); ASSERT_NE(a, SRT_ERROR); EXPECT_EQ(connect_res.get(), SRT_SUCCESS); @@ -420,15 +432,12 @@ TEST(TestFEC, ConnectionFull1) srt_listen(l, 1); - auto connect_res = std::async(std::launch::async, [&s, &sa]() { - return srt_connect(s, (sockaddr*)& sa, sizeof(sa)); - }); - + auto connect_res = spawn_connect(s, sa); // Make sure that the async call to srt_connect() is already kicked. std::this_thread::yield(); SRTSOCKET la[] = { l }; - SRTSOCKET a = srt_accept_bond(la, 1, 2000); + SRTSOCKET a = srt_accept_bond(la, 1, 5000); ASSERT_NE(a, SRT_ERROR); EXPECT_EQ(connect_res.get(), SRT_SUCCESS); @@ -478,15 +487,13 @@ TEST(TestFEC, ConnectionFull2) srt_listen(l, 1); - auto connect_res = std::async(std::launch::async, [&s, &sa]() { - return srt_connect(s, (sockaddr*)& sa, sizeof(sa)); - }); + auto connect_res = spawn_connect(s, sa); // Make sure that the async call to srt_connect() is already kicked. std::this_thread::yield(); SRTSOCKET la[] = { l }; - SRTSOCKET a = srt_accept_bond(la, 1, 2000); + SRTSOCKET a = srt_accept_bond(la, 1, 5000); ASSERT_NE(a, SRT_ERROR); EXPECT_EQ(connect_res.get(), SRT_SUCCESS); @@ -536,15 +543,13 @@ TEST(TestFEC, ConnectionMess) srt_listen(l, 1); - auto connect_res = std::async(std::launch::async, [&s, &sa]() { - return srt_connect(s, (sockaddr*)& sa, sizeof(sa)); - }); + auto connect_res = spawn_connect(s, sa); // Make sure that the async call to srt_connect() is already kicked. std::this_thread::yield(); SRTSOCKET la[] = { l }; - SRTSOCKET a = srt_accept_bond(la, 1, 2000); + SRTSOCKET a = srt_accept_bond(la, 1, 5000); ASSERT_NE(a, SRT_ERROR) << srt_getlasterror_str(); EXPECT_EQ(connect_res.get(), SRT_SUCCESS); @@ -592,15 +597,13 @@ TEST(TestFEC, ConnectionForced) srt_listen(l, 1); - auto connect_res = std::async(std::launch::async, [&s, &sa]() { - return srt_connect(s, (sockaddr*)& sa, sizeof(sa)); - }); + auto connect_res = spawn_connect(s, sa); // Make sure that the async call to srt_connect() is already kicked. std::this_thread::yield(); SRTSOCKET la[] = { l }; - SRTSOCKET a = srt_accept_bond(la, 1, 2000); + SRTSOCKET a = srt_accept_bond(la, 1, 5000); ASSERT_NE(a, SRT_ERROR); EXPECT_EQ(connect_res.get(), SRT_SUCCESS); @@ -645,9 +648,7 @@ TEST(TestFEC, RejectionConflict) srt_listen(l, 1); - auto connect_res = std::async(std::launch::async, [&s, &sa]() { - return srt_connect(s, (sockaddr*)& sa, sizeof(sa)); - }); + auto connect_res = spawn_connect(s, sa); // Make sure that the async call to srt_connect() is already kicked. std::this_thread::yield(); @@ -689,9 +690,7 @@ TEST(TestFEC, RejectionIncompleteEmpty) srt_listen(l, 1); - auto connect_res = std::async(std::launch::async, [&s, &sa]() { - return srt_connect(s, (sockaddr*)& sa, sizeof(sa)); - }); + auto connect_res = spawn_connect(s, sa); // Make sure that the async call to srt_connect() is already kicked. std::this_thread::yield(); @@ -737,9 +736,7 @@ TEST(TestFEC, RejectionIncomplete) srt_listen(l, 1); - auto connect_res = std::async(std::launch::async, [&s, &sa]() { - return srt_connect(s, (sockaddr*)& sa, sizeof(sa)); - }); + auto connect_res = spawn_connect(s, sa); // Make sure that the async call to srt_connect() is already kicked. std::this_thread::yield(); From a26a7021a501e8c4bc17c75f3a109cca529cf262 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Wed, 22 Oct 2025 10:47:34 +0200 Subject: [PATCH 19/25] [core] Reimplemented MAXREXMITBW controller basing on Token Bucket algorithm (#3220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Added more logs around rexmit bandwidth * [API] raise libSRT version number to 1.5.5 * Maintenance changes * Rewrite the Bandwidth estimation system. * Merged changes from Clement's implementation * Introduce CShaper to control the retransmission BW. * Fixed with average packet size measurement * Added self-adjusting of burst period * Fixed wrong formulas and added some logs * Removed old changes for CSndRateEstimator. Blocked the use of CSndRateEstimator. * Applied most direct review fixes * Refactoring: split packLostData into smaller parts * Some cosmetic fixes * Cosmetics * Removed Tie function that was using non-C++03-compatible assignment * Stretched connection timeout for accept in FEC tests * Trying to track the problem in FEC tests on Mac * Adjusted TestFEC tests to a bug in listener missing signal --------- Co-authored-by: Mikolaj Malecki Co-authored-by: Clément Gérouville --- srtcore/buffer_tools.h | 127 ++++++++ srtcore/common.cpp | 16 +- srtcore/common.h | 1 + srtcore/core.cpp | 576 ++++++++++++++++++++++++++++++----- srtcore/core.h | 156 +++++++++- srtcore/group.cpp | 4 + srtcore/utilities.h | 16 +- test/test_fec_rebuilding.cpp | 4 + 8 files changed, 801 insertions(+), 99 deletions(-) diff --git a/srtcore/buffer_tools.h b/srtcore/buffer_tools.h index 88479827d..1c90a56a1 100644 --- a/srtcore/buffer_tools.h +++ b/srtcore/buffer_tools.h @@ -198,6 +198,133 @@ class CSndRateEstimator int m_iRateBps; //< Rate in Bytes/sec. }; +// Utility class for bandwidth limitation +class CShaper +{ +public: + static double SHAPER_RESOLUTION_US() { return 1000000.; } // micro seconds + static double BURSTPERIOD_DEFAULT() { return 100; } + + typedef sync::steady_clock::time_point time_point; + typedef sync::steady_clock::duration duration; + CShaper (double min_tokens_capacity): + m_BurstPeriod(sync::milliseconds_from(BURSTPERIOD_DEFAULT())), + m_rate_Bps(0), + m_tokens(0), + m_tokensCapacity(0), + m_minTokensCapacity(min_tokens_capacity) + { + + } + + // Token capacity should be settable later + void setMinimumTokenCapacity(double cap) + { + m_minTokensCapacity = cap; + updateLimits(); + } + +private: + duration m_BurstPeriod; + double m_rate_Bps; // current_bitrate in Bytes per second + double m_tokens; // in bytes + double m_tokensCapacity; // in bytes + double m_minTokensCapacity; + time_point m_UpdateTime; + void setTokenCapacity(double tokens) + { + m_tokensCapacity = std::max(0., tokens); + } + + void setTokens(double tokens) + { + // It is allowed to set the negative number of tokens, + // just not less than the negative maximum + m_tokens = Bounds(1 - m_tokensCapacity, tokens, m_tokensCapacity); + } + + static double periodToTokens(double rate_Bps, duration period) + { + double seconds = double(sync::count_microseconds(period)) / SHAPER_RESOLUTION_US(); + return rate_Bps * seconds; + } + + static duration tokensToPeriod(double rate_Bps, double tokens) + { + double seconds = tokens / rate_Bps; + return sync::microseconds_from(seconds * SHAPER_RESOLUTION_US()); + } + + void updateLimits() + { + double token_cap = periodToTokens(m_rate_Bps, m_BurstPeriod); + if (token_cap < m_minTokensCapacity) + { + // We have a too small value; recalculate the burst period to reach the minimum. + duration minperiod = tokensToPeriod(m_rate_Bps, m_minTokensCapacity); + token_cap = m_minTokensCapacity; + m_BurstPeriod = minperiod; + } + setTokenCapacity(token_cap); + } + +public: + void setBitrate(double bw_Bps) + { + // NOTE: comparison on double to check the PREVIOUSLY + // SET value (m_rate_Bps), not recalculated value. + if (bw_Bps != m_rate_Bps) + { + m_rate_Bps = bw_Bps; + updateLimits(); + } + } + + void setMinimumBurstPeriod(duration hp) + { + if (m_BurstPeriod < hp) + { + setBurstPeriod(hp); + } + } + + void setBurstPeriod(duration bp) + { + if (bp != m_BurstPeriod) + { + m_BurstPeriod = bp; + updateLimits(); + } + } + + // Note that tick() must be always called after setBitrate. + void tick(const time_point& now) + { + duration delta = now - m_UpdateTime; + m_UpdateTime = now; + + double update_tokens = periodToTokens(m_rate_Bps, delta); + setTokens(update_tokens + m_tokens); + } + + bool enoughTokens(double len) const { return len <= m_tokens; } + void consumeTokens(double len) { setTokens(m_tokens - len); } + + + // For debug purposes + int ntokens() const { return m_tokens; } + duration burstPeriod() const { return m_BurstPeriod; } + int maxTokens() const { return m_tokensCapacity; } + // TOKENS = BITRATE * BURST_PERIOD / (SHAPER_UNIT * SHAPER_RESOLUTION_US) + // TOKENS * SHAPER_UNIT * SHAPER_RESOLUTION_US = BITRATE * BURST_PERIOD + // BITRATE = (TOKENS * SHAPER_UNIT * SHAPER_RESOLUTION_US) / (BURST_PERIOD) + double tokenRate_Bps(double tokens) const + { + return (tokens * SHAPER_RESOLUTION_US()) / (sync::count_microseconds(m_BurstPeriod)); + } + double availRate_Bps() const { return tokenRate_Bps(m_tokens); } + double usedRate_Bps() const { return tokenRate_Bps(m_tokensCapacity - m_tokens); } +}; } // namespace srt #endif diff --git a/srtcore/common.cpp b/srtcore/common.cpp index 2e94668ae..9d0b8e338 100644 --- a/srtcore/common.cpp +++ b/srtcore/common.cpp @@ -464,12 +464,12 @@ bool SrtParseConfig(const string& s, SrtConfig& w_config) return true; } -std::string FormatLossArray(const std::vector< std::pair >& lra) +string FormatLossArray(const vector< pair >& lra) { - std::ostringstream os; + ostringstream os; os << "[ "; - for (std::vector< std::pair >::const_iterator i = lra.begin(); i != lra.end(); ++i) + for (vector< pair >::const_iterator i = lra.begin(); i != lra.end(); ++i) { int len = CSeqNo::seqoff(i->first, i->second); os << "%" << i->first; @@ -482,6 +482,16 @@ std::string FormatLossArray(const std::vector< std::pair >& lr return os.str(); } +string FormatValue(int value, int factor, const char* unit) +{ + ostringstream out; + double showval = value; + showval /= factor; + out << std::fixed << showval << unit; + return out.str(); +} + + ostream& PrintEpollEvent(ostream& os, int events, int et_events) { static pair const namemap [] = { diff --git a/srtcore/common.h b/srtcore/common.h index ff84c3faf..15fa39e37 100644 --- a/srtcore/common.h +++ b/srtcore/common.h @@ -1437,6 +1437,7 @@ inline bool checkMappedIPv4(const sockaddr_in6& sa) std::string FormatLossArray(const std::vector< std::pair >& lra); std::ostream& PrintEpollEvent(std::ostream& os, int events, int et_events = 0); +std::string FormatValue(int value, int factor, const char* unit); } // namespace srt diff --git a/srtcore/core.cpp b/srtcore/core.cpp index dd189f73c..9e8d6b11e 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -259,6 +259,134 @@ CUDTUnited& srt::CUDT::uglobal() #endif +#ifdef ENABLE_RATE_MEASUREMENT +void srt::RateMeasurement::pickup(const clock_time& time) +{ + // Check if the time has passed the period. + clock_interval diff; + if (!passedInterval(time, (diff))) + { + HLOGC(bslog.Debug, log << "RateMeasurement: too early, " << FormatDuration(diff) + << " with begin TS=" << FormatTime(m_beginTime) << " now=" << FormatTime(time)); + // Still collect data for the current period. + return; + } + + // Doesn't matter what `diff` really is. Enough that we have + // this time aligned with 10ms resolution calls 10 times. + + Slice slice; + { + // Lock for the moment of taking the data + sync::ScopedLock lk (m_lock); + + slice.packets = m_nInstaPackets; + slice.bytes = m_nInstaBytes; + slice.begin_time = m_beginTime; + + m_nInstaPackets = 0; + m_nInstaBytes = 0; + m_beginTime = time; + } + + HLOGC(bslog.Debug, log << "RateMeasurement: passed " << FormatDuration(diff) + << " extracted packets=" << slice.packets << " bytes=" << slice.bytes + << " now beginTS=" << FormatTime(m_beginTime)); + + // Ok, now stickpile the slice. + // If the container is empty, do not stockpile empty slices. + if (m_slices.empty() && slice.bytes == 0) + { + HLOGC(bslog.Debug, log << "... NOT COLLECING packet with " << slice.bytes << " bytes to " << m_slices.size() << " total"); + // Empty-to-empty: DO NOT stockpile. + // NOTE: the container is never cleared when it has + // any older data, so this situation is only possible: + // - in the beginning + // - in the long run if it happened that 10 slices in + // a row were empty. + // So in this case we simply wait for having the first + // packets with something notified. We also know that + // the rate was initially 0 and it remains so now. + return; + } + + // We either have something and have pushed an empty slice + // or we have pushed filled slice onto the empty container. + // Still, we need at least 5 samples to start measuring. + m_slices.push_back(slice); + + // NOTE: this is also a situation that might have happened + // in the beginning, or after a long period without sending. + // If that period happens, the sending rate is also cleared, + // in any case we need to wait for the minimum samples. + if (m_slices.size() < MIN_SLICES) + { + HLOGC(bslog.Debug, log << "... NOT MEASURING: still " << m_slices.size() << " < 5 slices"); + return; + } + + // Keep track of only 10 last slices + if (m_slices.size() > MAX_SLICES) + { + m_slices.pop_front(); + + // After a slice was removed, there's already a chance + // that all other slices are also 0. Check the minimum + // number of slices + 1 and drop all that are empty + size_t end_range = 0; // default: nothing to delete + for (size_t i = 0; i < m_slices.size(); ++i) + { + if (i > MIN_SLICES + 1) + break; + + if (m_slices[i].bytes > 0) + break; + + // This can be skipped and check the next one. + end_range = i; + } + + // PARANOID + if (end_range >= m_slices.size()) + end_range = m_slices.size() - 1; // at least 1 element must stay + + m_slices.erase(m_slices.begin(), m_slices.begin() + end_range); + + m_beginTime = m_slices[0].begin_time; + HLOGC(bslog.Debug, log << "... Dropping 1 oldest + " << end_range << " empty slices, TS=" << FormatTime(m_beginTime)); + } + + // Check also if the begin time is older than 2* period; if so, + // reset it to that value + if (m_slices.size() > 1) // with 1 always m_beginTime = m_slices[0].begin_time + { + clock_time earliest = m_slices[1].begin_time - milliseconds_from(SLICE_INTERVAL_MS*2); + if (m_beginTime < earliest) + { + HLOGC(bslog.Debug, log << "... BEGIN TIME too old TS=" << FormatTime(m_beginTime) + << " - fixed to " << FormatTime(earliest)); + m_beginTime = earliest; + m_slices[0].begin_time = earliest; + } + } + + // Ok, so we have enough samples to record the measurement. + // (so we have at least one) + Summary sum (m_slices[0].begin_time); + + for (size_t i = 0; i < m_slices.size(); ++i) + { + sum.consume(m_slices[i]); + } + + m_currentPktRate = sum.ratePackets(time); + m_currentByteRate = sum.rateBytes(time, m_SysHeaderSize + CPacket::HDR_SIZE); + + HLOGC(bslog.Debug, log << "... CALCULATED from " << m_slices.size() << " slices: total TD=" << FormatDuration(time - m_slices[0].begin_time) + << " packets=" << sum.packets << " bytes=" << sum.bytes << " pkt-rate=" << m_currentPktRate << " byte-rate=" << m_currentByteRate); +} +#endif + void srt::CUDT::construct() { m_pSndBuffer = NULL; @@ -303,6 +431,12 @@ void srt::CUDT::construct() m_bPeerTLPktDrop = false; m_bBufferWasFull = false; + // Will be updated on first send +#ifdef ENABLE_MAXREXMITBW + m_zSndAveragePacketSize = 0; + m_zSndMaxPacketSize = 0; +#endif + // Initilize mutex and condition variables. initSynch(); @@ -313,7 +447,8 @@ void srt::CUDT::construct() srt::CUDT::CUDT(CUDTSocket* parent) : m_parent(parent) #ifdef ENABLE_MAXREXMITBW - , m_SndRexmitRate(sync::steady_clock::now()) + // , m_SndRexmitRate(sync::steady_clock::now()) + , m_SndRexmitShaper(CSrtConfig::DEF_MSS) #endif , m_iISN(-1) , m_iPeerISN(-1) @@ -342,7 +477,8 @@ srt::CUDT::CUDT(CUDTSocket* parent) srt::CUDT::CUDT(CUDTSocket* parent, const CUDT& ancestor) : m_parent(parent) #ifdef ENABLE_MAXREXMITBW - , m_SndRexmitRate(sync::steady_clock::now()) + // , m_SndRexmitRate(sync::steady_clock::now()) + , m_SndRexmitShaper(CSrtConfig::DEF_MSS) #endif , m_iISN(-1) , m_iPeerISN(-1) @@ -6202,6 +6338,13 @@ SRT_REJECT_REASON srt::CUDT::setupCC() m_tsLastRspAckTime = currtime; m_tsLastSndTime.store(currtime); +#ifdef ENABLE_RATE_MEASUREMENT + // XXX NOTE: use IPv4 or IPv6 as applicable! + HLOGC(bslog.Debug, log << CONID() << "RATE-MEASUREMENT: initializing time TS=" << FormatTime(currtime)); + m_SndRegularMeasurement.init(currtime, CPacket::UDP_HDR_SIZE); + m_SndRexmitMeasurement.init(currtime, CPacket::UDP_HDR_SIZE); +#endif + HLOGC(rslog.Debug, log << CONID() << "setupCC: setting parameters: mss=" << m_config.iMSS << " maxCWNDSize/FlowWindowSize=" << m_iFlowWindowSize << " rcvrate=" << m_iDeliveryRate << "p/s (" << m_iByteDeliveryRate << "B/S)" @@ -9461,80 +9604,157 @@ void srt::CUDT::updateAfterSrtHandshake(int hsv) } } -int srt::CUDT::packLostData(CPacket& w_packet) +// XXX During sender buffer refax turn this into either returning +// a sequence number or move it to the sender buffer facility. +// [[using locked (m_RecvAckLock)]] +std::pair srt::CUDT::getCleanRexmitOffset() { - // protect m_iSndLastDataAck from updating by ACK processing - UniqueLock ackguard(m_RecvAckLock); - const steady_clock::time_point time_now = steady_clock::now(); - const steady_clock::time_point time_nak = time_now - microseconds_from(m_iSRTT - 4 * m_iRTTVar); + // This function is required to look into the loss list and + // drop all sequences that are alrady revoked from the sender + // buffer, send the drop request if needed, and return the sender + // buffer offset for the next packet to retransmit, or -1 if + // there is no retransmission candidate at the moment. for (;;) { - w_packet.set_seqno(m_pSndLossList->popLostSeq()); - if (w_packet.seqno() == SRT_SEQNO_NONE) - break; + // REPEATABLE because we might need to drop this scheduled seq. - // XXX See the note above the m_iSndLastDataAck declaration in core.h - // This is the place where the important sequence numbers for - // sender buffer are actually managed by this field here. - const int offset = CSeqNo::seqoff(m_iSndLastDataAck, w_packet.seqno()); + // Pick up the first sequence. + int32_t seq = m_pSndLossList->popLostSeq(); + if (seq == SRT_SEQNO_NONE) + { + // No loss reports, we are clear here. + return std::make_pair(SRT_SEQNO_NONE, -1); + } + + int offset = CSeqNo::seqoff(m_iSndLastDataAck, seq); if (offset < 0) { // XXX Likely that this will never be executed because if the upper // sequence is not in the sender buffer, then most likely the loss // was completely ignored. - LOGC(qrlog.Error, - log << CONID() << "IPE/EPE: packLostData: LOST packet negative offset: seqoff(seqno() " - << w_packet.seqno() << ", m_iSndLastDataAck " << m_iSndLastDataAck << ")=" << offset - << ". Continue, request DROP"); + LOGC(qslog.Error, log << CONID() + << "IPE/EPE: packLostData: LOST packet negative offset: seqoff(seqno() " + << seq << ", m_iSndLastDataAck " << m_iSndLastDataAck << ")=" << offset + << ". Continue, request DROP"); + + int32_t oldest_outdated_seq = seq; + // We have received the first outdated sequence. + // Loop on to find out more. + // Interrupt on the first non-outdated sequence + // or when the loss list gets empty. + + for (;;) + { + seq = m_pSndLossList->popLostSeq(); + if (seq == SRT_SEQNO_NONE) + { + offset = -1; // set it because this will be returned. + break; + } + + offset = CSeqNo::seqoff(m_iSndLastDataAck, seq); + if (offset >= 0) + break; + } + + // Now we have 'seq' that is either non-sequence + // or a valid sequence. We can safely exit with this + // value from this branch. We just need to manage + // the drop report. // No matter whether this is right or not (maybe the attack case should be // considered, and some LOSSREPORT flood prevention), send the drop request // to the peer. int32_t seqpair[2] = { - w_packet.seqno(), + oldest_outdated_seq, CSeqNo::decseq(m_iSndLastDataAck) }; - HLOGC(qrlog.Debug, - log << CONID() << "PEER reported LOSS not from the sending buffer - requesting DROP: #" - << MSGNO_SEQ::unwrap(w_packet.msgflags()) << " SEQ:" << seqpair[0] << " - " << seqpair[1] << "(" - << (-offset) << " packets)"); + HLOGC(qslog.Debug, + log << CONID() << "PEER reported LOSS not from the sending buffer - requesting DROP: %(" + << seqpair[0] << " - " << seqpair[1] << ") (" << (-offset) << " packets)"); // See interpretation in processCtrlDropReq(). We don't know the message number, // so we request that the drop be exclusively sequence number based. int32_t msgno = SRT_MSGNO_CONTROL; - sendCtrl(UMSG_DROPREQ, &msgno, seqpair, sizeof(seqpair)); + } + + // We have exit anyway with the right sequence or no sequence. + if (seq == SRT_SEQNO_NONE) + return std::make_pair(SRT_SEQNO_NONE, -1); + + // For the right sequence though check also the rigt time. + const steady_clock::time_point time_now = steady_clock::now(); + if (!checkRexmitRightTime(offset, time_now)) + { + // Ignore this, even though valid, sequence, and pick up + // the next from the list. continue; } - if (m_bPeerNakReport && m_config.iRetransmitAlgo != 0) + // Ok, we have what we needed one way or another. + return std::make_pair(seq, offset); + } +} + +// [[using locked (m_RecvAckLock)]] +bool srt::CUDT::checkRexmitRightTime(int offset, const srt::sync::steady_clock::time_point& current_time) +{ + // If not configured, the time is always right + if (!m_bPeerNakReport || m_config.iRetransmitAlgo == 0) + return true; + + const steady_clock::time_point time_nak = current_time - microseconds_from(m_iSRTT - 4 * m_iRTTVar); + const steady_clock::time_point tsLastRexmit = m_pSndBuffer->getPacketRexmitTime(offset); + + if (tsLastRexmit >= time_nak) + { +#if ENABLE_HEAVY_LOGGING + ostringstream last_rextime; + if (is_zero(tsLastRexmit)) { - const steady_clock::time_point tsLastRexmit = m_pSndBuffer->getPacketRexmitTime(offset); - if (tsLastRexmit >= time_nak) - { - HLOGC(qrlog.Debug, log << CONID() << "REXMIT: ignoring seqno " - << w_packet.seqno() << ", last rexmit " << (is_zero(tsLastRexmit) ? "never" : FormatTime(tsLastRexmit)) - << " RTT=" << m_iSRTT << " RTTVar=" << m_iRTTVar - << " now=" << FormatTime(time_now)); - continue; - } + last_rextime << "REXMIT FIRST TIME"; } + else + { + last_rextime << "REXMITTED " << FormatDuration(current_time - tsLastRexmit) + << " ago at " << FormatTime(tsLastRexmit); + } + HLOGC(qslog.Debug, log << CONID() << "REXMIT: ignoring %" << CSeqNo::incseq(m_iSndLastDataAck, offset) + << " " << last_rextime.str() + << ", RTT: ~" << FormatValue(m_iSRTT, 1000, "ms") + << " <>" << FormatValue(m_iRTTVar, 1000, "") + << " now=" << FormatTime(current_time)); +#endif + return false; + } + + return true; +} - typedef CSndBuffer::DropRange DropRange; +// [[using locked (m_RecvAckLock)]] +int srt::CUDT::extractCleanRexmitPacket(int32_t seqno, int offset, CPacket& w_packet, + srt::sync::steady_clock::time_point& w_tsOrigin) +{ + // REPEATABLE BLOCK (not a real loop) + for (;;) + { + typedef CSndBuffer::DropRange DropRange; DropRange buffer_drop; - steady_clock::time_point tsOrigin; - const int payload = m_pSndBuffer->readData(offset, (w_packet), (tsOrigin), (buffer_drop)); + w_packet.set_seqno(seqno); + + const int payload = m_pSndBuffer->readData(offset, (w_packet), (w_tsOrigin), (buffer_drop)); if (payload == CSndBuffer::READ_DROP) { SRT_ASSERT(CSeqNo::seqoff(buffer_drop.seqno[DropRange::BEGIN], buffer_drop.seqno[DropRange::END]) >= 0); - HLOGC(qrlog.Debug, - log << CONID() << "loss-reported packets expired in SndBuf - requesting DROP: #" - << buffer_drop.msgno << " %(" << buffer_drop.seqno[DropRange::BEGIN] << " - " - << buffer_drop.seqno[DropRange::END] << ")"); + HLOGC(qslog.Debug, + log << CONID() << "loss-reported packets expired in SndBuf - requesting DROP: #" + << buffer_drop.msgno << " %(" << buffer_drop.seqno[DropRange::BEGIN] << " - " + << buffer_drop.seqno[DropRange::END] << ")"); sendCtrl(UMSG_DROPREQ, &buffer_drop.msgno, buffer_drop.seqno, sizeof(buffer_drop.seqno)); // skip all dropped packets @@ -9542,43 +9762,133 @@ int srt::CUDT::packLostData(CPacket& w_packet) m_iSndCurrSeqNo = CSeqNo::maxseq(m_iSndCurrSeqNo, buffer_drop.seqno[DropRange::END]); continue; } - else if (payload == CSndBuffer::READ_NONE) - continue; - // The packet has been ecrypted, thus the authentication tag is expected to be stored - // in the SND buffer as well right after the payload. - if (m_pCryptoControl && m_pCryptoControl->getCryptoMode() == CSrtConfig::CIPHER_MODE_AES_GCM) + if (payload == CSndBuffer::READ_NONE) { - w_packet.setLength(w_packet.getLength() + HAICRYPT_AUTHTAG_MAX); + LOGC(qslog.Error, log << CONID() << "loss-reported packet %" << w_packet.seqno() << " NOT FOUND in the sender buffer"); + return 0; } - // At this point we no longer need the ACK lock, - // because we are going to return from the function. - // Therefore unlocking in order not to block other threads. - ackguard.unlock(); + return payload; + } + +} - enterCS(m_StatsLock); - m_stats.sndr.sentRetrans.count(payload); - leaveCS(m_StatsLock); +int srt::CUDT::packLostData(CPacket& w_packet) +{ +#if ENABLE_MAXREXMITBW + +#ifdef ENABLE_RATE_MEASUREMENT +#define IF_RATE_MEA(expr) expr +#else +#define IF_RATE_MEA(expr) +#endif - // Despite the contextual interpretation of packet.m_iMsgNo around - // CSndBuffer::readData version 2 (version 1 doesn't return -1), in this particular - // case we can be sure that this is exactly the value of PH_MSGNO as a bitset. - // So, set here the rexmit flag if the peer understands it. - if (m_bPeerRexmitFlag) + // XXX NOTE: If you refactor the sender buffer so that the random access + // is possible, you might be able to extract the exact packet size to + // check for enough tokens and consume them in one step. + + bool maxrexmitbw_enabled = m_config.llMaxRexmitBW >= 0; + if (maxrexmitbw_enabled) + { + IF_RATE_MEA( IF_HEAVY_LOGGING(const int64_t iRexmitRateMeasured = m_SndRexmitMeasurement.rateBytes()); ) + size_t granted_size = m_zSndAveragePacketSize; + size_t len = granted_size + CPacket::HDR_SIZE + CPacket::UDP_HDR_SIZE; + + if (!m_SndRexmitShaper.enoughTokens(len)) { - w_packet.set_msgflags(w_packet.msgflags() | PACKET_SND_REXMIT); + HLOGC(qslog.Debug, log << "REXMIT-SH: BLOCKED pkt est/len=" << len << " exceeds " << m_SndRexmitShaper.ntokens() + << " tokens, rate/Bps:used=" << m_SndRexmitShaper.usedRate_Bps() << ",avail=" << m_SndRexmitShaper.availRate_Bps() + IF_RATE_MEA( << " (measured: " << FormatValue(iRexmitRateMeasured, 1024, "kBps") << ")" ) + ); + return 0; } - setDataPacketTS(w_packet, tsOrigin); -#ifdef ENABLE_MAXREXMITBW - m_SndRexmitRate.addSample(time_now, 1, w_packet.getLength()); + HLOGC(qslog.Debug, log << "REXMIT-SH: ALLOWED pkt est/len=" << len << " allowed, budget " << m_SndRexmitShaper.ntokens() + << " tokens, rate/Bps:used=" << m_SndRexmitShaper.usedRate_Bps() << ",avail=" << m_SndRexmitShaper.availRate_Bps() + IF_RATE_MEA( << " (measured: " << FormatValue(iRexmitRateMeasured, 1024, "kBps") << ")" ) + ); + } + +#undef IF_RATE_MEA #endif - return payload; + // Original sending time of the packet, to be stamped after filling. + steady_clock::time_point tsOrigin; + + { + // protect m_iSndLastDataAck from updating by ACK processing + ScopedLock ackguard(m_RecvAckLock); + int32_t seqno; + int offset; + + // Get the first sequence for retransmission, bypassing and taking care of + // those that are in the forgotten region, as well as required to be rejected. + Tie2(seqno, offset) = getCleanRexmitOffset(); + + if (seqno == SRT_SEQNO_NONE) + return 0; + + // Extract the packet from the sender buffer that is mapped to the expected sequence + // number, bypassing and taking care of those that are decided to be dropped. + const int payload = extractCleanRexmitPacket(seqno, offset, (w_packet), (tsOrigin)); + if (payload <= 0) + return 0; } - return 0; + HLOGC(qslog.Debug, log << CONID() << "packed REXMIT packet %" << w_packet.seqno() << " size=" << w_packet.getLength() + << " - still " << m_pSndLossList->getLossLength() << " LOSS ENTRIES left"); + + // POST-EXTRACTION length fixes. + + // The packet has been ecrypted, thus the authentication tag is expected to be stored + // in the SND buffer as well right after the payload. + if (m_pCryptoControl && m_pCryptoControl->getCryptoMode() == CSrtConfig::CIPHER_MODE_AES_GCM) + { + w_packet.setLength(w_packet.getLength() + HAICRYPT_AUTHTAG_MAX); + } + +#if ENABLE_MAXREXMITBW + if (maxrexmitbw_enabled) + { + // Token consumption will only happen when the retransmission + // effectively happens. + // XXX NOTE: In version 1.6.0 use the IP-version dependent value for UDP_HDR_SIZE + size_t network_size = w_packet.getLength() + CPacket::HDR_SIZE + CPacket::UDP_HDR_SIZE; + m_SndRexmitShaper.consumeTokens(network_size); + HLOGC(qslog.Debug, log << "REXMIT-SH: consumed " << network_size << " tokens, remain " << m_SndRexmitShaper.ntokens()); + } +#endif + + enterCS(m_StatsLock); + m_stats.sndr.sentRetrans.count(w_packet.getLength()); + leaveCS(m_StatsLock); + + // Despite the contextual interpretation of packet.m_iMsgNo around + // CSndBuffer::readData version 2 (version 1 doesn't return -1), in this particular + // case we can be sure that this is exactly the value of PH_MSGNO as a bitset. + // So, set here the rexmit flag if the peer understands it. + if (m_bPeerRexmitFlag) + { + w_packet.set_msgflags(w_packet.msgflags() | PACKET_SND_REXMIT); + } + setDataPacketTS(w_packet, tsOrigin); + +#ifdef ENABLE_MAXREXMITBW + // XXX OLD rexmit measurement + // m_SndRexmitRate.addSample(time_now, 1, w_packet.getLength()); +#endif + + // XXX Consider calculating the total packet length once you have a possibility + // to get the in-connection used IP version. +#ifdef ENABLE_RATE_MEASUREMENT + m_SndRexmitMeasurement.dataUpdate(1, w_packet.getLength()); + HLOGC(bslog.Debug, log << "RateMeasurement: REXMIT, pkt-size=" << w_packet.getLength() + << " - COLLECTED pkts=" << m_SndRexmitMeasurement.m_nInstaPackets + << " bytes=" << m_SndRexmitMeasurement.m_nInstaBytes); +#endif + + return w_packet.getLength(); } #if SRT_DEBUG_TRACE_SND @@ -9703,15 +10013,69 @@ void srt::CUDT::setDataPacketTS(CPacket& p, const time_point& ts) p.set_timestamp(makeTS(ts, tsStart)); } -bool srt::CUDT::isRetransmissionAllowed(const time_point& tnow SRT_ATR_UNUSED) +bool srt::CUDT::isRegularSendingPriority() { - // Prioritization of original packets only applies to Live CC. + // In order to have regular packets take precedense over retransmitted: + // - SRTO_TLPKTDROP = true + // - SRTO_MESSAGEAPI = true + // NOTE: + // - tlpktdrop is ignored in stream mode + // - messageapi without tlpktdrop is possible in non-live message mode + // - Live mode without tlpktdrop is possible and in this mode also the + // retransmitted packets should have priority. if (!m_bPeerTLPktDrop || !m_config.bMessageAPI) - return true; + return false; - // TODO: lock sender buffer? + // XXX NOTE: the current solution is simple - the regular packet takes precedence + // over a retransmitted packet, if there is at least one such packet already + // scheduled. + // + // This probably isn't the most wanted solution, some more elaborate condition + // might be better in some situations. For example, it should be acceptable + // that a packet that has very little time to be recovered is sent before a + // regular packet that has still STT + Latency time to deliver. The regular + // packets should still be favorized, but not necessarily at the expence of + // dismissing a recovery chance that wouldn't endanger the delivery of a regular + // packet. Criteria might be various, for example, the number of scheduled + // packets and their late delivery time might be taken into account. const time_point tsNextPacket = m_pSndBuffer->peekNextOriginal(); + if (tsNextPacket != time_point()) + { + // Have regular packet and we decided they have a priority. + HLOGC(qslog.Debug, log << "REXMIT-SH: BLOCKED because regular packets have priority"); + return true; + } + return false; +} + +namespace srt { +#ifdef ENABLE_MAXREXMITBW +static sync::steady_clock::duration optimisticSingleTripDuration(int rttval, int rttvar) +{ + int lowrtt = rttval - rttvar; + if (lowrtt < 0) // bullshit? + lowrtt = rttval; + + int stt = lowrtt/2; + + // Make sure that burst period has at least this value + sync::steady_clock::duration stt_td = sync::microseconds_from(stt); + + // Still, prevent from setting enormous STT values + if (stt_td < sync::seconds_from(4)) + { + return stt_td; + } + return sync::steady_clock::duration(); +} +#endif +} + +void srt::CUDT::updateSenderMeasurements(bool can_rexmit SRT_ATR_UNUSED) +{ + const time_point tnow SRT_ATR_UNUSED = steady_clock::now(); + #if SRT_DEBUG_TRACE_SND const int buffdelay_ms = count_milliseconds(m_pSndBuffer->getBufferingDelay(tnow)); // If there is a small loss, still better to retransmit. If timespan is already big, @@ -9727,31 +10091,37 @@ bool srt::CUDT::isRetransmissionAllowed(const time_point& tnow SRT_ATR_UNUSED) g_snd_logger.state.msTimespanTh = threshold_ms_min; g_snd_logger.state.msNextUniqueToSend = msNextUniqueToSend; g_snd_logger.state.usElapsedLastDrop = count_microseconds(tnow - m_tsLastTLDrop); - g_snd_logger.state.canRexmit = false; + g_snd_logger.state.canRexmit = can_rexmit; #endif - if (tsNextPacket != time_point()) - { - // Can send original packet, so just send it - return false; - } - #ifdef ENABLE_MAXREXMITBW + /* OLD rate estimator; CODE for packLostData()! m_SndRexmitRate.addSample(tnow, 0, 0); // Update the estimation. const int64_t iRexmitRateBps = m_SndRexmitRate.getRate(); - const int64_t iRexmitRateLimitBps = m_config.llMaxRexmitBW; if (iRexmitRateLimitBps >= 0 && iRexmitRateBps > iRexmitRateLimitBps) { // Too many retransmissions, so don't send anything. // TODO: When to wake up next time? return false; } -#endif + */ + const int64_t iRexmitRateLimitBps = m_config.llMaxRexmitBW; + if (iRexmitRateLimitBps >= 0) + { + int b4_tokens SRT_ATR_UNUSED = m_SndRexmitShaper.ntokens(); + m_SndRexmitShaper.setBitrate(iRexmitRateLimitBps); + duration stt_td = optimisticSingleTripDuration(m_iSRTT, m_iRTTVar); + if (stt_td != duration()) + m_SndRexmitShaper.setMinimumBurstPeriod(stt_td); + m_SndRexmitShaper.tick(tnow); + int a4_tokens SRT_ATR_UNUSED = m_SndRexmitShaper.ntokens(); -#if SRT_DEBUG_TRACE_SND - g_snd_logger.state.canRexmit = true; + HLOGC(qslog.Debug, log << "REXMIT-SW: tick tokens updated: " << b4_tokens << " -> " << a4_tokens + << "/" << m_SndRexmitShaper.maxTokens() << " period=" + << FormatDuration(m_SndRexmitShaper.burstPeriod())); + } #endif - return true; + } bool srt::CUDT::packData(CPacket& w_packet, steady_clock::time_point& w_nexttime, sockaddr_any& w_src_addr) @@ -9780,9 +10150,25 @@ bool srt::CUDT::packData(CPacket& w_packet, steady_clock::time_point& w_nexttime if (!m_bOpened) return false; - payload = isRetransmissionAllowed(enter_time) - ? packLostData((w_packet)) - : 0; + // This function returns true if there is a regular packet candidate waiting + // for being sent and sending this packet has a prioriry over retransmission candidate. + if (!isRegularSendingPriority()) + { + payload = packLostData((w_packet)); +#if ENABLE_MAXREXMITBW + if (payload == 0) + { + HLOGC(qslog.Debug, log << "REXMIT-SH: no rexmit required, remain " << m_SndRexmitShaper.ntokens() << " tokens"); + } +#endif + } + else + { + HLOGC(qslog.Debug, log << "REXMIT: retransmission SUPERSEDED, proceed with regular candidate"); + } + + // Updates the data that will be next used in packLostData() in next calls. + updateSenderMeasurements(payload != 0); IF_HEAVY_LOGGING(const char* reason); // The source of the data packet (normal/rexmit/filter) if (payload > 0) @@ -9808,6 +10194,18 @@ bool srt::CUDT::packData(CPacket& w_packet, steady_clock::time_point& w_nexttime m_tdSendTimeDiff = steady_clock::duration(); return false; } + +#if ENABLE_MAXREXMITBW + if (m_zSndAveragePacketSize > 0) + { + m_zSndAveragePacketSize = avg_iir<16>(m_zSndAveragePacketSize, w_packet.getLength()); + } + else + { + m_zSndAveragePacketSize = w_packet.getLength(); + } + m_zSndMaxPacketSize = std::max(m_zSndMaxPacketSize, w_packet.getLength()); +#endif new_packet_packed = true; // every 16 (0xF) packets, a packet pair is sent @@ -10044,6 +10442,11 @@ bool srt::CUDT::packUniqueData(CPacket& w_packet) g_snd_logger.trace(); #endif +#ifdef ENABLE_RATE_MEASUREMENT + HLOGC(bslog.Debug, log << "RATE-MEASUREMENT: REGULAR, pkt-size=" << w_packet.getLength()); + m_SndRegularMeasurement.dataUpdate(1, w_packet.getLength()); +#endif + return true; } @@ -11773,6 +12176,11 @@ void srt::CUDT::checkTimers() << " pkt-count=" << m_iPktCount << " liteack-count=" << m_iLightACKCount); #endif +#ifdef ENABLE_RATE_MEASUREMENT + m_SndRegularMeasurement.pickup(currtime); + m_SndRexmitMeasurement.pickup(currtime); +#endif + // Check if it is time to send ACK int debug_decision = checkACKTimer(currtime); diff --git a/srtcore/core.h b/srtcore/core.h index 6bf75466e..0e4b4b5d1 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -159,6 +159,133 @@ class CUDTSocket; class CUDTGroup; #endif +#ifdef ENABLE_RATE_MEASUREMENT +struct RateMeasurement +{ + typedef sync::steady_clock clock_type; + typedef clock_type::time_point clock_time; + typedef clock_type::duration clock_interval; + + static const int SLICE_INTERVAL_MS = 20; + static const size_t MIN_SLICES = 5; // min + static const size_t MAX_SLICES = 10; + + sync::Mutex m_lock; + + size_t m_SysHeaderSize; + + // Instantaneous data updated by sending packets + // AFFINITY: Snd:worker thread + // MODE: write + sync::atomic m_nInstaPackets; + sync::atomic m_nInstaBytes; + + void dataUpdate(int packets, int bytes) + { + sync::ScopedLock lk (m_lock); + m_nInstaPackets = m_nInstaPackets + packets; + m_nInstaBytes = m_nInstaBytes + bytes; + } + + // Cached last measurement + // AFFINITY: Snd:worker thread MODE: read + // AFFINITY: Rcv:worker thread MODE: write + // (not locking because one data is updated at a time). + sync::atomic m_currentPktRate; + sync::atomic m_currentByteRate; + int64_t rateBytes() const + { + return m_currentByteRate; + } + int64_t ratePackets() const + { + return m_currentPktRate; + } + + // Time of the last checkpoint or initialization + clock_time m_beginTime; + + struct Slice + { + int packets; + int bytes; + clock_time begin_time; + + Slice(): packets(0), bytes(0) {} // clock_time is non-POD + Slice(const clock_time& t): packets(0), bytes(0), begin_time(t) {} + + }; + + struct Summary: Slice + { + Summary(const clock_time& earliest_time): Slice(earliest_time), nsamples(0) {} + + unsigned char nsamples; + + void consume(const Slice& another) + { + if (is_zero(begin_time)) + begin_time = another.begin_time; + packets += another.packets; + bytes += another.bytes; + nsamples++; + } + + int64_t ratePackets(const clock_time& end_time) const + { + double packets_per_micro = double(packets) * 1000 * 1000; + double rate = packets_per_micro / count_microseconds(end_time - begin_time); + return rate; + } + + int64_t rateBytes(const clock_time& end_time, size_t hdr_size) const + { + double data_bytes = bytes + (packets * hdr_size); + double bytes_per_micro = data_bytes * 1000 * 1000; + double rate = bytes_per_micro / count_microseconds(end_time - begin_time); + return rate; + } + }; + + std::deque m_slices; + + RateMeasurement(): + m_SysHeaderSize(CPacket::UDP_HDR_SIZE), // XXX NOTE: IPv4 ! + m_nInstaPackets(0), + m_nInstaBytes(0), + m_currentPktRate(0), + m_currentByteRate(0) + { + } + + bool passedInterval(const clock_time& this_time, clock_interval& w_interval) + { + if (is_zero(m_beginTime)) + { + sync::ScopedLock lk (m_lock); + m_beginTime = this_time; + w_interval = clock_interval(0); + return false; + } + w_interval = this_time - m_beginTime; + return (sync::count_milliseconds(w_interval) > SLICE_INTERVAL_MS); + } + + // This is to be called in constructor, only once. + void init(const clock_time& time, size_t sys_hdr_size) + { + // Just formally. + sync::ScopedLock lk (m_lock); + m_beginTime = time; + m_SysHeaderSize = sys_hdr_size; + } + + // AFFINITY: Rcv:worker thread (update thread) + // This function should be called in regular time periods. + void pickup(const clock_time& time); +}; +#endif + // XXX REFACTOR: The 'CUDT' class is to be merged with 'CUDTSocket'. // There's no reason for separating them, there's no case of having them // anyhow managed separately. After this is done, with a small help with @@ -593,11 +720,14 @@ class CUDT SRT_ATTR_REQUIRES2(m_RecvAckLock, m_StatsLock) int sndDropTooLate(); - /// @bried Allow packet retransmission. - /// Depending on the configuration mode (live / file), retransmission - /// can be blocked if e.g. there are original packets pending to be sent. - /// @return true if retransmission is allowed; false otherwise. - bool isRetransmissionAllowed(const time_point& tnow); + // Returns true if there is a regular packet waiting for sending + // and sending regular packets has priority over retransmitted ones. + bool isRegularSendingPriority(); + + // Performs updates in the measurement variables after possible + // extraction of a retransmission packet. Some are for debug purposes, + // others for retransmission rate measurement. + void updateSenderMeasurements(bool can_rexmit); /// Connect to a UDT entity as per hs request. This will update /// required data in the entity, then update them also in the hs structure, @@ -873,7 +1003,16 @@ class CUDT CSndLossList* m_pSndLossList; // Sender loss list CPktTimeWindow<16, 16> m_SndTimeWindow; // Packet sending time window #ifdef ENABLE_MAXREXMITBW - CSndRateEstimator m_SndRexmitRate; // Retransmission rate estimation. + size_t m_zSndAveragePacketSize; + size_t m_zSndMaxPacketSize; + // XXX Old rate estimator for rexmit + // CSndRateEstimator m_SndRexmitRate; // Retransmission rate estimation. + CShaper m_SndRexmitShaper; + +#ifdef ENABLE_RATE_MEASUREMENT + RateMeasurement m_SndRegularMeasurement; // Regular rate measurement + RateMeasurement m_SndRexmitMeasurement; // Retransmission rate measurement +#endif #endif atomic_duration m_tdSendInterval; // Inter-packet time, in CPU clock cycles @@ -1131,6 +1270,11 @@ class CUDT /// @return payload size on success, <=0 on failure int packLostData(CPacket &packet); + std::pair getCleanRexmitOffset(); + bool checkRexmitRightTime(int offset, const srt::sync::steady_clock::time_point& current_time); + int extractCleanRexmitPacket(int32_t seqno, int offset, CPacket& w_packet, + srt::sync::steady_clock::time_point& w_tsOrigin); + /// Pack a unique data packet (never sent so far) in CPacket for sending. /// @param packet [in, out] a CPacket structure to fill. /// diff --git a/srtcore/group.cpp b/srtcore/group.cpp index 339512573..ed18a72d6 100644 --- a/srtcore/group.cpp +++ b/srtcore/group.cpp @@ -730,6 +730,10 @@ static bool getOptDefault(SRT_SOCKOPT optname, void* pw_optval, int& w_optlen) RD(true); case SRTO_MAXBW: RD(int64_t(-1)); +#ifdef ENABLE_MAXREXMITBW + case SRTO_MAXREXMITBW: + RD(int64_t(-1)); +#endif case SRTO_INPUTBW: RD(int64_t(-1)); case SRTO_MININPUTBW: diff --git a/srtcore/utilities.h b/srtcore/utilities.h index 8781b80e7..cf3add810 100644 --- a/srtcore/utilities.h +++ b/srtcore/utilities.h @@ -789,12 +789,6 @@ inline void insert_uniq(std::vector& v, const ArgValue& val) v.push_back(val); } -template -inline std::pair Tie(Type1& var1, Type2& var2) -{ - return std::pair(var1, var2); -} - // This can be used in conjunction with Tie to simplify the code // in loops around a whole container: // list::const_iterator it, end; @@ -972,6 +966,16 @@ inline void AccumulatePassFilterParallel(const int* p, size_t size, PassFilter +inline Type Bounds(Type lower, Type value, Type upper) +{ + if (value < lower) + return lower; + if (value > upper) + return upper; + return value; +} + inline std::string FormatBinaryString(const uint8_t* bytes, size_t size) { diff --git a/test/test_fec_rebuilding.cpp b/test/test_fec_rebuilding.cpp index b39345c40..3910c959b 100644 --- a/test/test_fec_rebuilding.cpp +++ b/test/test_fec_rebuilding.cpp @@ -91,6 +91,7 @@ class TestFECRebuilding: public srt::Test { delete fec; } + }; static std::future spawn_connect(SRTSOCKET s, sockaddr_in& sa, int timeout_ms = 1000) @@ -374,6 +375,9 @@ TEST(TestFEC, ConnectionReorder) ASSERT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1); ASSERT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1); + int conntimeo = 10000; + ASSERT_NE(srt_setsockflag(s, SRTO_CONNTIMEO, &conntimeo, sizeof (conntimeo)), SRT_ERROR); + srt_listen(l, 1); auto connect_res = spawn_connect(s, sa); From c09532f8edf28de91854f975bb16e643e8085ed9 Mon Sep 17 00:00:00 2001 From: Maria Sharabayko Date: Mon, 27 Oct 2025 11:06:48 +0100 Subject: [PATCH 20/25] [docs] Added link to the blogpost with - Live Streaming using SRT with QUIC Datagrams - research --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 0e18ff4a5..e6e7f7905 100644 --- a/README.md +++ b/README.md @@ -159,6 +159,7 @@ In live streaming configurations, the SRT protocol maintains a constant end-to-e - [RTMP vs. SRT: Comparing Latency and Maximum Bandwidth](https://www.haivision.com/resources/white-paper/srt-versus-rtmp/) White Paper. - [Documentation on GitHub](./docs#documentation-overview) with SRT API documents, features decsriptions, etc. - The SRT Protocol Internet Draft: [Datatracker](https://datatracker.ietf.org/doc/draft-sharabayko-srt/) | [Latest Version](https://datatracker.ietf.org/doc/html/draft-sharabayko-srt-01) | [Latest Working Copy](https://haivision.github.io/srt-rfc/draft-sharabayko-srt.html) | [GitHub Repo](https://github.com/Haivision/srt-rfc) +- If you are curious about live streaming using SRT with QUIC datagrams as an alternative to UDP transport, take a look at the following [blog post](https://medium.com/innovation-labs-blog/live-streaming-using-srt-with-quic-datagrams-7896f7ce7bf3?source=friends_link&sk=d0a00e79861d89673e27a04260f279b5) on Medium. ## Build Instructions From f0368c6998fbf6f789f8e484bdc6ea6f4a48a750 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Tue, 25 Nov 2025 08:50:17 +0100 Subject: [PATCH 21/25] [build] Added handling of LIBSRT_ prefix when setting cmake options for SRT build (#3219) * [BUILD] Added handling of LIBSRT_ prefix when setting cmake options for SRT build * Added documentation * Apply suggestions from code review Co-authored-by: stevomatthews * Fixed text alignment after doc review --------- Co-authored-by: Mikolaj Malecki Co-authored-by: stevomatthews --- CMakeLists.txt | 10 ++++++++++ docs/build/build-options.md | 33 +++++++++++++++++++++++++++++++++ scripts/haiUtil.cmake | 13 +++++++++++++ 3 files changed, 56 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6e19caa2c..cf9b360ed 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -122,6 +122,16 @@ endforeach() # SRT_MAVG_SAMPLING_RATE 40 /* Max sampling rate */ # SRT_ENABLE_FREQUENT_LOG_TRACE 0 : set to 1 to enable printing reason for suppressed freq logs +# CMake offers sometimes OLD behavior, where names that are defined below with +# option() function, will be deleted if they are not in cache or do not have set +# type - so effectively options provided through LIBSRT_* prefix would be rejected. +# We do want these variables, obviously. +cmake_policy(SET CMP0077 NEW) + +# Import all options preceded with LIBSRT_ likely from a parent scope. +# Names are not being checked, at worst it will set an unused variable. +srt_import_parent_options() + # option defaults set(ENABLE_HEAVY_LOGGING_DEFAULT OFF) diff --git a/docs/build/build-options.md b/docs/build/build-options.md index e09ea41bb..05fcf8be4 100644 --- a/docs/build/build-options.md +++ b/docs/build/build-options.md @@ -17,6 +17,39 @@ Additional information on building for Windows is available in the document and in the [SRT CookBook](https://srtlab.github.io/srt-cookbook/getting-started/build-on-windows/). +## Building as a subproject + +The CMake tool offers the ability to add a complete project as a subdirectory. +Variables used by the SRT project in this case remain in their own scope, but +all variables from the parent scope are reflected. In order to prevent name +clashes for option-designating variables, SRT provides a namespace-like +prefixing for the optional variables it uses. If you want to configure optional +variables from the level of `CMakeLists.txt` of the parent project, use the +`LIBSRT_` prefix for the option names. + +This will not prevent the variables from being seen as derived in SRT project +scope, but if you explicitly set a variable this way, it will be set to the +desired value inside the SRT project. It will not set the same variable in the +parent project, and it will also override (locally in SRT project only) any +value of a variable with the same name in the parent project. + +For example, if you want to set `ENABLE_SHARED=OFF` in the parent project, +simply do: + +``` +set (LIBSRT_ENABLE_SHARED OFF) +``` + +If you already have a variable named `ENABLE_SHARED` in your project (existing +before the call to `add_subdirectory` with SRT), its value will be derived in +the SRT project, unless you override it by setting `LIBSRT_ENABLE_SHARED` to a +different value. + +Note that the trick works simply by getting the actual variable name through +cutting off the `LIBSRT_` prefix; the check whether this variable is of any use +will be done after that. + + ## List of Build Options The following table lists available build options in alphabetical order. diff --git a/scripts/haiUtil.cmake b/scripts/haiUtil.cmake index 222661989..312480e1e 100644 --- a/scripts/haiUtil.cmake +++ b/scripts/haiUtil.cmake @@ -288,3 +288,16 @@ function (parse_compiler_type wct _type _suffix) endif() endif() endfunction() + +macro (srt_import_parent_options) + getVarsWith(LIBSRT_ extoptions) + foreach(ef ${extoptions}) + set (val ${${ef}}) + string(LENGTH LIBSRT_ pflen) + string(LENGTH ${ef} eflen) + math(EXPR alen ${eflen}-${pflen}) + string(SUBSTRING ${ef} ${pflen} ${alen} ef) + message(STATUS "IMPORTED OPTION: ${ef}='${val}'") + set (${ef} "${val}") + endforeach() +endmacro() From f8a53dadfb4d023405c6b49699f2f8bdf0b7fdd6 Mon Sep 17 00:00:00 2001 From: Tomoya Fujita Date: Tue, 16 Dec 2025 17:37:02 +0900 Subject: [PATCH 22/25] [build] Ubuntu 23.04 reached end-of-life in January 2024. (#3261) Signed-off-by: Tomoya.Fujita --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index e6e7f7905..4255c0663 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ [![Build Status Linux and macOS][travis-badge]][travis] [![Build Status Windows][appveyor-badge]][appveyor] -[![Ubuntu 23.04][Ubuntu-badge]][Ubuntu-package] +[![Ubuntu 24.04][Ubuntu-badge]][Ubuntu-package] [![Fedora 37][fedora-badge]][fedora-package] [![Debian][debian-badge]][debian-package] [![Homebrew][Homebrew-badge]][Homebrew-package] @@ -242,4 +242,4 @@ By contributing code to the SRT project, you agree to license your contribution [homebrew-badge]: https://repology.org/badge/version-for-repo/homebrew/srt.svg [Ubuntu-package]: https://repology.org/project/srt/versions -[Ubuntu-badge]: https://repology.org/badge/version-for-repo/ubuntu_23_04/srt.svg +[Ubuntu-badge]: https://repology.org/badge/version-for-repo/ubuntu_24_04/srt.svg From 72f0c6e0e9a186adec428a145f4e085412d96796 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Tue, 16 Dec 2025 09:37:59 +0100 Subject: [PATCH 23/25] [BUG] [core] Prevent blocking srt_close call from interrupting connection (#3259) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [BUG] Prevent blocking srt_close call from interrupting connection * Test: added check that srt_close call is not blocked. Ensured that waiting is interrupt ed on close. Ensured that the socket is closed right after wait quit * Fixed incorrect C++17 literals * Increased wait time tolerance for srt_close (failing on some slower CI machines) --------- Co-authored-by: Mikołaj Małecki --- srtcore/api.cpp | 5 +- srtcore/api.h | 5 ++ srtcore/core.cpp | 3 +- srtcore/core.h | 1 + test/test_connection_timeout.cpp | 122 ++++++++++++++++++++++++++++++- test/test_file_transmission.cpp | 16 +++- 6 files changed, 146 insertions(+), 6 deletions(-) diff --git a/srtcore/api.cpp b/srtcore/api.cpp index 22b093dcc..0b96a33bc 100644 --- a/srtcore/api.cpp +++ b/srtcore/api.cpp @@ -2144,6 +2144,9 @@ void srt::CUDTUnited::deleteGroup_LOCKED(CUDTGroup* g) int srt::CUDTUnited::close(CUDTSocket* s) { + // Set the closing flag BEFORE you attempt to acquire + s->setBreaking(); + HLOGC(smlog.Debug, log << s->core().CONID() << "CLOSE. Acquiring control lock"); ScopedLock socket_cg(s->m_ControlLock); HLOGC(smlog.Debug, log << s->core().CONID() << "CLOSING (removing from listening, closing CUDT)"); @@ -2973,7 +2976,7 @@ void srt::CUDTUnited::removeSocket(const SRTSOCKET u) return; } - LOGC(smlog.Note, log << "@" << s->m_SocketID << " busy=" << s->isStillBusy()); + HLOGC(smlog.Note, log << "@" << s->m_SocketID << " busy=" << s->isStillBusy()); #if ENABLE_BONDING if (s->m_GroupOf) diff --git a/srtcore/api.h b/srtcore/api.h index 42b97ef32..789d68d0c 100644 --- a/srtcore/api.h +++ b/srtcore/api.h @@ -216,6 +216,11 @@ class CUDTSocket core().m_bClosing = true; } + void setBreaking() + { + core().m_bBreaking = true; + } + /// This does the same as setClosed, plus sets the m_bBroken to true. /// Such a socket can still be read from so that remaining data from /// the receiver buffer can be read, but no longer sends anything. diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 9e8d6b11e..d13221f1c 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -414,6 +414,7 @@ void srt::CUDT::construct() m_bConnected = false; m_bClosing = false; m_bShutdown = false; + m_bBreaking = false; m_bBroken = false; m_bBreakAsUnstable = false; // TODO: m_iBrokenCounter should be still set to some default. @@ -3868,7 +3869,7 @@ void srt::CUDT::startConnect(const sockaddr_any& serv_addr, int32_t forced_isn) // We can't record this address yet until the cookie-confirmation is done, for safety reasons. sockaddr_any use_source_adr(serv_addr.family()); - while (!m_bClosing) + while (!m_bClosing && !m_bBreaking) { const steady_clock::time_point local_tnow = steady_clock::now(); const steady_clock::duration tdiff = local_tnow - m_tsLastReqTime.load(); diff --git a/srtcore/core.h b/srtcore/core.h index 0e4b4b5d1..81a5142a5 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -972,6 +972,7 @@ class CUDT sync::atomic m_bConnected; // Whether the connection is on or off sync::atomic m_bClosing; // If the UDT entity is closing sync::atomic m_bShutdown; // If the peer side has shutdown the connection + sync::atomic m_bBreaking; // The flag that declares interrupt of the connecting process sync::atomic m_bBroken; // If the connection has been broken sync::atomic m_bBreakAsUnstable; // A flag indicating that the socket should become broken because it has been unstable for too long. sync::atomic m_bPeerHealth; // If the peer status is normal diff --git a/test/test_connection_timeout.cpp b/test/test_connection_timeout.cpp index c8cbc8722..281376ce1 100644 --- a/test/test_connection_timeout.cpp +++ b/test/test_connection_timeout.cpp @@ -14,9 +14,10 @@ typedef int SOCKET; #include"platform_sys.h" #include "srt.h" #include "netinet_any.h" +#include "common.h" using namespace std; - +using namespace srt_logging; class TestConnectionTimeout : public ::srt::Test @@ -172,12 +173,19 @@ TEST_F(TestConnectionTimeout, BlockingLoop) { const sockaddr* psa = reinterpret_cast(&m_sa); const int connection_timeout_ms = 999; + cout << "[ ]\r[" << flush; + for (int i = 0; i < 10; ++i) { const SRTSOCKET client_sock = srt_create_socket(); EXPECT_GT(client_sock, 0); // socket_id should be > 0 if (client_sock <= 0) + { + cout << "!\n"; break; + } + + cout << "." << flush; // Set connection timeout to 999 ms to reduce the test execution time. // Also need to hit a time point between two threads: @@ -198,15 +206,125 @@ TEST_F(TestConnectionTimeout, BlockingLoop) EXPECT_EQ(error_code, SRT_ENOSERVER); if (error_code != SRT_ENOSERVER) { - cerr << "Connection attempt no. " << i << " resulted with: " + cout << "!\nConnection attempt no. " << i << " resulted with: " << error_code << " " << srt_getlasterror_str() << "\n"; break; } EXPECT_EQ(srt_close(client_sock), SRT_SUCCESS); } + cout << endl; +} + +TEST_F(TestConnectionTimeout, BlockingInterrupted) +{ + const SRTSOCKET client_sock = srt_create_socket(); + EXPECT_GT(client_sock, 0); // socket_id should be > 0 + + const int connection_timeout_ms = 10000; + EXPECT_EQ(srt_setsockopt(client_sock, 0, SRTO_CONNTIMEO, &connection_timeout_ms, sizeof connection_timeout_ms), SRT_SUCCESS); + + using namespace std::chrono; + + steady_clock::time_point begin = steady_clock::now(); + + std::thread interrupter ( [client_sock] () { + cout << "[T] START: Waiting 1s\n"; + std::this_thread::sleep_for(seconds(1)); + steady_clock::time_point b = steady_clock::now(); + + cout << "[T] CLOSING @" << client_sock << "\n"; + srt_close(client_sock); + steady_clock::time_point e = steady_clock::now(); + auto passed = duration_cast(e - b); + int close_time_passed_ms = passed.count(); + EXPECT_LT(close_time_passed_ms, 2000); + + cout << "[T] Thread exit\n"; + }); + + const sockaddr* psa = reinterpret_cast(&m_sa); + cout << "START: Connect @" << client_sock << " blind\n"; + EXPECT_EQ(srt_connect(client_sock, psa, sizeof m_sa), SRT_ERROR); + cout << "STOP: Connect\n"; + + steady_clock::time_point end = steady_clock::now(); + auto passed = duration_cast(end - begin); + int time_passed_ms = passed.count(); + cout << "Interrupted after " << time_passed_ms << "ms\n"; + + EXPECT_LT(time_passed_ms, 8000); + + interrupter.join(); } +TEST_F(TestConnectionTimeout, NonblockingInterrupted) +{ + const SRTSOCKET client_sock = srt_create_socket(); + // const SRTSOCKET extra_sock = srt_create_socket(); + EXPECT_GT(client_sock, 0); // socket_id should be > 0 + + const int connection_timeout_ms = 10000; + EXPECT_EQ(srt_setsockopt(client_sock, 0, SRTO_CONNTIMEO, &connection_timeout_ms, sizeof connection_timeout_ms), SRT_SUCCESS); + + const bool non_blocking = false; + EXPECT_EQ(srt_setsockflag(client_sock, SRTO_RCVSYN, &non_blocking, sizeof non_blocking), SRT_SUCCESS); + EXPECT_EQ(srt_setsockflag(client_sock, SRTO_SNDSYN, &non_blocking, sizeof non_blocking), SRT_SUCCESS); + + using namespace std::chrono; + + int eid = srt_epoll_create(); + + int conn_err = SRT_EPOLL_OUT | SRT_EPOLL_ERR; + + srt_epoll_add_usock(eid, client_sock, &conn_err); + // srt_epoll_add_usock(eid, extra_sock, &conn_err); + + steady_clock::time_point begin = steady_clock::now(); + + std::thread interrupter ( [client_sock] () { + cout << "[T] START: Waiting 1s\n"; + std::this_thread::sleep_for(seconds(1)); + steady_clock::time_point b = steady_clock::now(); + + cout << "[T] CLOSING @" << client_sock << "\n"; + srt_close(client_sock); + steady_clock::time_point e = steady_clock::now(); + auto passed = duration_cast(e - b); + int close_time_passed_ms = passed.count(); + EXPECT_LT(close_time_passed_ms, 2000); + + cout << "[T] Thread exit\n"; + }); + + const sockaddr* psa = reinterpret_cast(&m_sa); + cout << "START: Connect @" << client_sock << " blind\n"; + + // Result should be CONNREQ because this is non-blocking connect + EXPECT_EQ(srt_connect(client_sock, psa, sizeof m_sa), 0); + + cout << "EPOLL - wait for connect\n"; + + // Expect uwait to return -1 because all sockets have been + // removed from EID, so the call would block forever. + SRT_EPOLL_EVENT fdset[2]; + EXPECT_EQ(srt_epoll_uwait(eid, fdset, 2, 3000), -1); + SRT_SOCKSTATUS socket_status = srt_getsockstate(client_sock); + + steady_clock::time_point end = steady_clock::now(); + auto passed = duration_cast(end - begin); + int time_passed_ms = passed.count(); + cout << "Interrupted after " << time_passed_ms << "ms\n"; + + EXPECT_LT(time_passed_ms, 8000); + + interrupter.join(); + + cout << "SOCKET STATUS: " << SockStatusStr(socket_status) << endl; + EXPECT_GE(socket_status, SRTS_CLOSED); + + // srt_close(extra_sock); +} TEST(TestConnectionAPI, Accept) { diff --git a/test/test_file_transmission.cpp b/test/test_file_transmission.cpp index bfd668ac7..e7140bf69 100644 --- a/test/test_file_transmission.cpp +++ b/test/test_file_transmission.cpp @@ -50,9 +50,13 @@ TEST(Transmission, FileUpload) sa_lsn.sin_addr.s_addr = INADDR_ANY; sa_lsn.sin_port = htons(5555); + MAKE_UNIQUE_SOCK(sock_lsn_u, "listener", sock_lsn); + MAKE_UNIQUE_SOCK(sock_clr_u, "listener", sock_clr); + // Find unused a port not used by any other service. // Otherwise srt_connect may actually connect. int bind_res = -1; + std::cout << "Looking for a free port... " << std::flush; for (int port = 5000; port <= 5555; ++port) { sa_lsn.sin_port = htons(port); @@ -97,10 +101,13 @@ TEST(Transmission, FileUpload) std::atomic thread_exit { false }; + std::cout << "Running accept [A] thread\n"; + auto client = std::thread([&] { sockaddr_in remote; int len = sizeof remote; + std::cout << "[A] waiting for connection\n"; const SRTSOCKET accepted_sock = srt_accept(sock_lsn, (sockaddr*)&remote, &len); ASSERT_GT(accepted_sock, 0); @@ -115,27 +122,30 @@ TEST(Transmission, FileUpload) std::vector buf(1456); + std::cout << "[A] Connected, reading data...\n"; for (;;) { int n = srt_recv(accepted_sock, buf.data(), 1456); EXPECT_NE(n, SRT_ERROR) << srt_getlasterror_str(); if (n == 0) { - std::cerr << "Received 0 bytes, breaking.\n"; + std::cout << "Received 0 bytes, breaking.\n"; break; } else if (n == -1) { - std::cerr << "READ FAILED, breaking anyway\n"; + std::cout << "READ FAILED, breaking anyway\n"; break; } // Write to file any amount of data received copyfile.write(buf.data(), n); } + std::cout << "[A] Closing socket\n"; EXPECT_NE(srt_close(accepted_sock), SRT_ERROR); + std::cout << "[A] Exit\n"; thread_exit = true; }); @@ -144,6 +154,7 @@ TEST(Transmission, FileUpload) sa.sin_port = sa_lsn.sin_port; ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1); + std::cout << "Connecting...\n"; srt_connect(sock_clr, (sockaddr*)&sa, sizeof(sa)); std::cout << "Connection initialized" << std::endl; @@ -151,6 +162,7 @@ TEST(Transmission, FileUpload) std::ifstream ifile("file.source", std::ios::in | std::ios::binary); std::vector buf(1456); + std::cout << "Reading file and sending...\n"; for (;;) { size_t n = ifile.read(buf.data(), 1456).gcount(); From f5e6bb98d5ff1532c270e8314ec20a2de13df620 Mon Sep 17 00:00:00 2001 From: Tomoya Fujita Date: Mon, 12 Jan 2026 19:07:32 +0900 Subject: [PATCH 24/25] Old URL (404 - no longer exists) (#3271) Signed-off-by: Tomoya.Fujita --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4255c0663..16a1d8e08 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@

- SRT + SRT

From 80c871e1fe6604ef0d95556b9e7ba03497daa606 Mon Sep 17 00:00:00 2001 From: Tomoya Fujita Date: Mon, 12 Jan 2026 22:41:37 +0900 Subject: [PATCH 25/25] Detec IPv6 with colon to harden the check if IPv6 is used in the test. (#3264) * Detec IPv6 with colon to harden the check if IPv6 is used in the test. Signed-off-by: Tomoya.Fujita * just relies on srt::CreateAddr to resolve IP family. Signed-off-by: Tomoya Fujita --------- Signed-off-by: Tomoya.Fujita Signed-off-by: Tomoya Fujita --- test/test_reuseaddr.cpp | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/test/test_reuseaddr.cpp b/test/test_reuseaddr.cpp index 14789f897..929f6b3db 100644 --- a/test/test_reuseaddr.cpp +++ b/test/test_reuseaddr.cpp @@ -148,15 +148,6 @@ class ReuseAddr : public srt::Test int yes = 1; int no = 0; - int family = AF_INET; - string famname = "IPv4"; - if (ip.substr(0, 2) == "6.") - { - family = AF_INET6; - ip = ip.substr(2); - famname = "IPv6"; - } - cout << "[T/C] Setting up client socket\n"; ASSERT_NE(client_sock, SRT_INVALID_SOCK); ASSERT_EQ(srt_getsockstate(client_sock), SRTS_INIT); @@ -171,7 +162,8 @@ class ReuseAddr : public srt::Test int epoll_out = SRT_EPOLL_OUT; srt_epoll_add_usock(client_pollid, client_sock, &epoll_out); - sockaddr_any sa = srt::CreateAddr(ip, port, family); + sockaddr_any sa = srt::CreateAddr(ip, port, AF_UNSPEC); + string famname = (sa.family() == AF_INET) ? "IPv4" : "IPv6"; cout << "[T/C] Connecting to: " << sa.str() << " (" << famname << ")" << endl;