From 63bf9620964fab46a1f235937d8e92e5b0bc405b Mon Sep 17 00:00:00 2001 From: Thierry Lelegard Date: Fri, 21 Feb 2025 11:36:48 +0100 Subject: [PATCH 01/14] [build] Fix Windows installers (#3113). Use the OpenSSL Universal installer on Windows to build on x86/x64 host targeting Arm64 systems. --- scripts/win-installer/README.md | 79 +++++++++ scripts/win-installer/build-win-installer.ps1 | 155 ++++++++---------- scripts/win-installer/install-openssl.ps1 | 73 +++------ scripts/win-installer/libsrt.nsi | 24 +-- 4 files changed, 184 insertions(+), 147 deletions(-) diff --git a/scripts/win-installer/README.md b/scripts/win-installer/README.md index 006b8a6bc..cab9d7661 100644 --- a/scripts/win-installer/README.md +++ b/scripts/win-installer/README.md @@ -18,6 +18,8 @@ These initial steps need to be executed once only. - Prerequisite 2: Install OpenSSL for Windows, 64-bit Intel, 32-bit Intel, 64-bit Arm. This can be done automatically by running the PowerShell script `install-openssl.ps1`. + If you are interested in more information about OpenSSL for Windows, scroll to the + appendix at the end of this page. - Prerequisite 3: Install NSIS, the NullSoft Installation Scripting system. This can be done automatically by running the PowerShell script `install-nsis.ps1`. @@ -128,3 +130,80 @@ This directory contains the following files: | libsrt.nsi | NSIS installation script (used to build the installer). | libsrt.props | Visual Studio property files to use libsrt (embedded in the installer). | README.md | This text file. + +## Appendix: Using OpenSSL on Windows + +The SRT protocol uses encryption. Internally, `libsrt` can use multiple cryptographic +libraries but OpenSSL is the default choice on all platforms, including Windows. + +OpenSSL is not part of the base installation of a Windows platform and should be +separately installed. OpenSSL is neither "Windows-friendly" and the OpenSSL team +does not provide Windows binaries. + +The production of OpenSSL binaries for Windows is delegated to Shining Light +Productions (http://slproweb.com/). Binaries for all architectures are available +here: http://slproweb.com/products/Win32OpenSSL.html + +### How to grab OpenSSL installers for Windows + +This is automatically done by the PowerShell script `install-openssl.ps1` in +this directory. Let's see what's behind the scene. + +First, the script determines which installers should be downloaded and where +to get them from. + +Initially, the HTML for the [slproweb](http://slproweb.com/products/Win32OpenSSL.html) +page was built on server-side. Our script grabbed the URL content, parsed the HTML +and extracted the URL of the binaries for the latest OpenSSL packages from the +"href" of the links. + +At some point in 2024, the site changed policy. The Web page was no longer +built on server-side, but on client-side. The downloaded HTML contains some +JavaScript which dynamically builds the URL of the binaries for the latest +OpenSSL binaries. The previous method now finds no valid package URL from +the downloaded page (a full browser is needed to execute the JavaScript). +The JavaScript downloads a JSON file which contains all references to +the OpenSSL binaries. + +Therefore, the script `install-openssl.ps1` now uses the same method: +download that JSON file and parse it. + +### Multi-architecture builds + +The `libsrt` installer contains static libraries for all three architectures +which are supported by Windows: x86, x64, Arm64. The corresponding static +libraries for OpenSSL are also needed and included in the installer. + +Because Visual Studio can build libraries and executables for all three +architectures, on any build system, we need to install the OpenSSL headers +and libraries for all three architectures, on the build system. + +This is what the script `install-openssl.ps1` does. However, the way to +do this has changed over time. + +As a starting point, there are three installers, one per architecture. +All installers are x86 executables which can be run on any Windows system +(Arm64 Windows emulates x86 and x64 when necessary). Additionally, the +three sets of files are installed side by side, without overlap. + +- x86: `C:\Program Files (x86)\OpenSSL-Win32` +- x64: `C:\Program Files\OpenSSL-Win64` +- Arm64: `C:\Program Files\OpenSSL-Win64-ARM` + +So, the script `install-openssl.ps1` installed them all and `libsrt` could +be built for all architecture. + +However, it works only on Arm64 systems. On x86 and x64 system, the OpenSSL +installer for Arm64 fails. The installer executable can run but there is an +explicit check in the installation process to abort when running on Intel +systems. + +Since most Windows build servers are Intel systems, this approach was no +longer appropriate. + +At some point, the producers of the OpenSSL binaries acknowledged the problem +and they created an additional form of installer: the "universal" one. This +package can be installed on any system and the binaries and headers are installed +for all architectures, in one single location. + +- Universal: `C:\Program Files (x86)\OpenSSL-WinUniversal` diff --git a/scripts/win-installer/build-win-installer.ps1 b/scripts/win-installer/build-win-installer.ps1 index 9e9ae7940..b6621550c 100644 --- a/scripts/win-installer/build-win-installer.ps1 +++ b/scripts/win-installer/build-win-installer.ps1 @@ -21,6 +21,11 @@ the defaut version is a detailed version number (most recent version, number of commits since then, short commit SHA). + .PARAMETER NoBuild + + Do not rebuild the SRT libraries. Assume that they are already built. + Only build the installer. + .PARAMETER NoPause Do not wait for the user to press at end of execution. By default, @@ -31,6 +36,7 @@ [CmdletBinding()] param( [string]$Version = "", + [switch]$NoBuild = $false, [switch]$NoPause = $false ) Write-Output "Building the SRT static libraries installer for Windows" @@ -89,54 +95,33 @@ Write-Output "Windows version info: $VersionInfo" #----------------------------------------------------------------------------- # Locate OpenSSL root from local installation. -# After version 3.something, OpenSSL has changed its installation layout. -# We have to look for OpenSSL libraries in various locations. -$SSL = @{ - "x64" = @{ - "alt" = "Win64"; - "bits" = 64; - "root" = "C:\Program Files\OpenSSL-Win64" - }; - "Win32" = @{ - "alt" = "x86"; - "bits" = 32; - "root" = "C:\Program Files (x86)\OpenSSL-Win32" - } - "ARM64" = @{ - "alt" = "arm64"; - "bits" = 64; - "root" = "C:\Program Files\OpenSSL-Win64-ARM" - }; -} +# We now requires the "universal" OpenSSL installer (see README.md). +# The universal OpenSSL installer is installed in $SSL. +# $ARCH translates between VC "platform" names to subdirectory names inside $SSL. +# $LIBDIR translates between VC "configuration" names to library subdirectory names. + +$SSL = "C:\Program Files (x86)\OpenSSL-WinUniversal" +$ARCH = @{x64 = "x64"; Win32 = "x86"; ARM64 = "arm64"} +$LIBDIR = @{Release = "MD"; Debug = "MDd"} +$LIBFILE = @{crypto = "libcrypto_static.lib"; ssl = "libssl_static.lib"} + +function ssl-include($pf) { return "$SSL\include\$($ARCH.$pf)" } +function ssl-libdir($pf, $conf) { return "$SSL\lib\VC\$($ARCH.$pf)\$($LIBDIR.$conf)" } +function ssl-lib($pf, $conf, $lib) { return "$(ssl-libdir $pf $conf)\$($LIBFILE.$lib)" } # Verify OpenSSL directories and static libraries. Write-Output "Searching OpenSSL libraries ..." $Missing = 0 -foreach ($arch in $SSL.Keys) { - $root = $SSL[$arch]["root"] - $bits = $SSL[$arch]["bits"] - $alt = $SSL[$arch]["alt"] - if (-not (Test-Path $root)) { - Write-Output "**** Missing $root" +foreach ($Platform in $SSL.Keys) { + if (-not (Test-Path -PathType Container $(ssl-include $Platform))) { + Write-Output "**** Missing $(ssl-include $Platform)" $Missing = $Missing + 1 } - else { - foreach ($lib in @("ssl", "crypto")) { - foreach ($conf in @("MD", "MDd")) { - $name = "lib${lib}${conf}" - $SSL[$arch][$name] = "" - foreach ($try in @("$root\lib\VC\static\lib${lib}${bits}${conf}.lib", - "$root\lib\VC\${arch}\${conf}\lib${lib}_static.lib", - "$root\lib\VC\${alt}\${conf}\lib${lib}_static.lib")) { - if (Test-Path $try) { - $SSL[$arch][$name] = $try - break - } - } - if (-not $SSL[$arch][$name]) { - Write-Output "**** OpenSSL static library for $name not found" - $Missing = $Missing + 1 - } + foreach ($lib in $LIBFILE.Keys) { + foreach ($conf in $LIBDIR.Keys) { + if (-not (Test-Path $(ssl-lib $Platform $conf $lib))) { + Write-Output "**** Missing $(ssl-lib $Platform $conf $lib)" + $Missing = $Missing + 1 } } } @@ -187,46 +172,40 @@ Write-Output "NSIS: $NSIS" # Configure and build SRT library using CMake on all architectures. #----------------------------------------------------------------------------- -foreach ($Platform in $SSL.Keys) { - - # Build directory. Cleanup to force a fresh cmake config. - $BuildDir = "$TmpDir\build.$Platform" - Remove-Item -Recurse -Force -ErrorAction SilentlyContinue $BuildDir - [void](New-Item -Path $BuildDir -ItemType Directory -Force) - - # Run CMake. - # Note: In previous versions of CMake, it was necessary to specify where - # OpenSSL was located using OPENSSL_ROOT_DIR, OPENSSL_LIBRARIES, and - # OPENSSL_INCLUDE_DIR. Starting with CMake 3.29.0, this is no longer - # necessary because CMake knows where to find the OpenSSL binaries from - # slproweb. Additionally, for some unkown reason, defining the OPENSSL_xxx - # variables no longer works with Arm64 libraries, even though the path to - # that version is correct. So, it's better to let CMake find OpenSSL by itself. - Write-Output "Configuring build for platform $Platform ..." - $SRoot = $SSL[$Platform]["root"] - $LibSSL = $SSL[$Platform]["libsslMD"] - $LibCrypto = $SSL[$Platform]["libcryptoMD"] - & $CMake -S $RepoDir -B $BuildDir -A $Platform -DENABLE_STDCXX_SYNC=ON - - # Patch version string in version.h - Get-Content "$BuildDir\version.h" | - ForEach-Object { - $_ -replace "#define *SRT_VERSION_STRING .*","#define SRT_VERSION_STRING `"$Version`"" - } | - Out-File "$BuildDir\version.new" -Encoding ascii - Move-Item "$BuildDir\version.new" "$BuildDir\version.h" -Force - - # Compile SRT. - Write-Output "Building for platform $Platform ..." - foreach ($Conf in @("Release", "Debug")) { - & $MSBuild "$BuildDir\SRT.sln" /nologo /maxcpucount /property:Configuration=$Conf /property:Platform=$Platform /target:srt_static +if (-not $NoBuild) { + foreach ($Platform in $ARCH.Keys) { + # Build directory. Cleanup to force a fresh cmake config. + $BuildDir = "$TmpDir\build.$Platform" + Remove-Item -Recurse -Force -ErrorAction SilentlyContinue $BuildDir + [void](New-Item -Path $BuildDir -ItemType Directory -Force) + + # Run CMake. + Write-Output "Configuring build for platform $Platform ..." + & $CMake -S $RepoDir -B $BuildDir -A $Platform ` + -DENABLE_STDCXX_SYNC=ON ` + -DOPENSSL_INCLUDE_DIR="$(ssl-include $Platform)" ` + -DOPENSSL_ROOT_DIR="$(ssl-libdir $Platform Release)" + + # Patch version string in version.h + Get-Content "$BuildDir\version.h" | + ForEach-Object { + $_ -replace "#define *SRT_VERSION_STRING .*","#define SRT_VERSION_STRING `"$Version`"" + } | + Out-File "$BuildDir\version.new" -Encoding ascii + Move-Item "$BuildDir\version.new" "$BuildDir\version.h" -Force + + # Compile SRT. + Write-Output "Building for platform $Platform ..." + foreach ($Conf in $LIBDIR.Keys) { + & $MSBuild "$BuildDir\SRT.sln" /nologo /maxcpucount /property:Configuration=$Conf /property:Platform=$Platform /target:srt_static + } } } # Verify the presence of compiled libraries. Write-Output "Checking compiled libraries ..." $Missing = 0 -foreach ($Conf in @("Release", "Debug")) { +foreach ($Conf in $LIBDIR.Keys) { foreach ($Platform in $SSL.Keys) { $Path = "$TmpDir\build.$Platform\$Conf\srt_static.lib" if (-not (Test-Path $Path)) { @@ -254,18 +233,18 @@ Write-Output "Building installer ..." /DOutDir="$OutDir" ` /DBuildRoot="$TmpDir" ` /DRepoDir="$RepoDir" ` - /DlibsslWin32MD="$($SSL.Win32.libsslMD)" ` - /DlibsslWin32MDd="$($SSL.Win32.libsslMDd)" ` - /DlibcryptoWin32MD="$($SSL.Win32.libcryptoMD)" ` - /DlibcryptoWin32MDd="$($SSL.Win32.libcryptoMDd)" ` - /DlibsslWin64MD="$($SSL.x64.libsslMD)" ` - /DlibsslWin64MDd="$($SSL.x64.libsslMDd)" ` - /DlibcryptoWin64MD="$($SSL.x64.libcryptoMD)" ` - /DlibcryptoWin64MDd="$($SSL.x64.libcryptoMDd)" ` - /DlibsslArm64MD="$($SSL.ARM64.libsslMD)" ` - /DlibsslArm64MDd="$($SSL.ARM64.libsslMDd)" ` - /DlibcryptoArm64MD="$($SSL.ARM64.libcryptoMD)" ` - /DlibcryptoArm64MDd="$($SSL.ARM64.libcryptoMDd)" ` + /DlibsslWin32Release="$(ssl-lib Win32 Release ssl)" ` + /DlibsslWin32Debug="$(ssl-lib Win32 Debug ssl)" ` + /DlibcryptoWin32Release="$(ssl-lib Win32 Release crypto)" ` + /DlibcryptoWin32Debug="$(ssl-lib Win32 Debug crypto)" ` + /DlibsslWin64Release="$(ssl-lib x64 Release ssl)" ` + /DlibsslWin64Debug="$(ssl-lib x64 Debug ssl)" ` + /DlibcryptoWin64Release="$(ssl-lib x64 Release crypto)" ` + /DlibcryptoWin64Debug="$(ssl-lib x64 Debug crypto)" ` + /DlibsslArm64Release="$(ssl-lib Arm64 Release ssl)" ` + /DlibsslArm64Debug="$(ssl-lib Arm64 Debug ssl)" ` + /DlibcryptoArm64Release="$(ssl-lib Arm64 Release crypto)" ` + /DlibcryptoArm64Debug="$(ssl-lib Arm64 Debug crypto)" ` "$ScriptDir\libsrt.nsi" if (-not (Test-Path $InstallExe)) { diff --git a/scripts/win-installer/install-openssl.ps1 b/scripts/win-installer/install-openssl.ps1 index 3352584b8..c9ec58068 100644 --- a/scripts/win-installer/install-openssl.ps1 +++ b/scripts/win-installer/install-openssl.ps1 @@ -37,24 +37,7 @@ param( Write-Output "OpenSSL download and installation procedure" -# A bit of history: Where to load OpenSSL binaries from? -# -# The packages are built by "slproweb". The binaries are downloadable from -# their Web page at: http://slproweb.com/products/Win32OpenSSL.html -# -# Initially, the HTML for this page was built on server-side. This script -# grabbed the URL content, parse the HTML and extracted the URL of the -# binaries for the latest OpenSSL packages from the "href" of the links. -# -# At some point in 2024, the site changed policy. The Web page is no longer -# built on server-side, but on client-side. The downloaded HTML contains some -# JavaScript which dynamically builds the URL of the binaries for the latest -# OpenSSL binaries. The previous method now finds no valid package URL from -# the downloaded page (a full browser is needed to execute the JavaScript). -# The JavaScript downloads a JSON file which contains all references to -# the OpenSSL binaries. This script now uses the same method: download that -# JSON file and parse it. - +# The list of OpenSSL packages is available in a JSON file. See more details in README.md. $PackageList = "https://github.com/slproweb/opensslhashes/raw/master/win32_openssl_hashes.json" # A function to exit this script. @@ -101,40 +84,36 @@ if ($status -ne 1 -and $status -ne 2) { } $config = ConvertFrom-Json $Response.Content -# Download and install MSI packages for 32 and 64-bit Intel, 64-bit Arm. -foreach ($conf in @(@{arch="intel"; bits=32}, @{arch="intel"; bits=64}, @{arch="arm"; bits=64})) { - - # Get the URL of the MSI installer from the JSON config. - $Url = $config.files | Get-Member | ForEach-Object { - $name = $_.name - $info = $config.files.$($_.name) - if (-not $info.light -and $info.installer -like "msi" -and $info.bits -eq $conf.bits -and $info.arch -like $conf.arch) { - $info.url - } - } | Select-Object -Last 1 - if (-not $Url) { - Exit-Script "#### No MSI installer found for $($conf.bits)-bit $($conf.arch)" +# Find the URL of the latest "universal" installer in the JSON config file. +$Url = $config.files | Get-Member | ForEach-Object { + $name = $_.name + $info = $config.files.$($_.name) + if (-not $info.light -and $info.installer -like "exe" -and $info.arch -like "universal") { + $info.url } +} | Select-Object -Last 1 +if (-not $Url) { + Exit-Script "#### No universal installer found" +} - $MsiName = (Split-Path -Leaf $Url) - $MsiPath = "$TmpDir\$MsiName" +$ExeName = (Split-Path -Leaf $Url) +$ExePath = "$TmpDir\$ExeName" - if (-not $ForceDownload -and (Test-Path $MsiPath)) { - Write-Output "$MsiName already downloaded, use -ForceDownload to download again" - } - else { - Write-Output "Downloading $Url ..." - Invoke-WebRequest -UseBasicParsing -UserAgent Download -Uri $Url -OutFile $MsiPath - } +if (-not $ForceDownload -and (Test-Path $ExePath)) { + Write-Output "$ExeName already downloaded, use -ForceDownload to download again" +} +else { + Write-Output "Downloading $Url ..." + Invoke-WebRequest -UseBasicParsing -UserAgent Download -Uri $Url -OutFile $ExePath +} - if (-not (Test-Path $MsiPath)) { - Exit-Script "$Url download failed" - } +if (-not (Test-Path $ExePath)) { + Exit-Script "$Url download failed" +} - if (-not $NoInstall) { - Write-Output "Installing $MsiName" - Start-Process msiexec.exe -ArgumentList @("/i", $MsiPath, "/qn", "/norestart") -Wait - } +if (-not $NoInstall) { + Write-Output "Installing $ExeName" + Start-Process -FilePath $ExePath -ArgumentList @("/VERYSILENT", "/SUPPRESSMSGBOXES", "/NORESTART", "/ALLUSERS") -Wait } Exit-Script diff --git a/scripts/win-installer/libsrt.nsi b/scripts/win-installer/libsrt.nsi index a3fff4451..66ff14745 100644 --- a/scripts/win-installer/libsrt.nsi +++ b/scripts/win-installer/libsrt.nsi @@ -142,38 +142,38 @@ Section "Install" CreateDirectory "$INSTDIR\lib\Release-x64" SetOutPath "$INSTDIR\lib\Release-x64" File /oname=srt.lib "${BuildWin64Dir}\Release\srt_static.lib" - File /oname=libcrypto.lib "${libcryptoWin64MD}" - File /oname=libssl.lib "${libsslWin64MD}" + File /oname=libcrypto.lib "${libcryptoWin64Release}" + File /oname=libssl.lib "${libsslWin64Release}" CreateDirectory "$INSTDIR\lib\Debug-x64" SetOutPath "$INSTDIR\lib\Debug-x64" File /oname=srt.lib "${BuildWin64Dir}\Debug\srt_static.lib" - File /oname=libcrypto.lib "${libcryptoWin64MDd}" - File /oname=libssl.lib "${libsslWin64MDd}" + File /oname=libcrypto.lib "${libcryptoWin64Debug}" + File /oname=libssl.lib "${libsslWin64Debug}" CreateDirectory "$INSTDIR\lib\Release-Win32" SetOutPath "$INSTDIR\lib\Release-Win32" File /oname=srt.lib "${BuildWin32Dir}\Release\srt_static.lib" - File /oname=libcrypto.lib "${libcryptoWin32MD}" - File /oname=libssl.lib "${libsslWin32MD}" + File /oname=libcrypto.lib "${libcryptoWin32Release}" + File /oname=libssl.lib "${libsslWin32Release}" CreateDirectory "$INSTDIR\lib\Debug-Win32" SetOutPath "$INSTDIR\lib\Debug-Win32" File /oname=srt.lib "${BuildWin32Dir}\Debug\srt_static.lib" - File /oname=libcrypto.lib "${libcryptoWin32MDd}" - File /oname=libssl.lib "${libsslWin32MDd}" + File /oname=libcrypto.lib "${libcryptoWin32Debug}" + File /oname=libssl.lib "${libsslWin32Debug}" CreateDirectory "$INSTDIR\lib\Release-Arm64" SetOutPath "$INSTDIR\lib\Release-Arm64" File /oname=srt.lib "${BuildArm64Dir}\Release\srt_static.lib" - File /oname=libcrypto.lib "${libcryptoArm64MD}" - File /oname=libssl.lib "${libsslArm64MD}" + File /oname=libcrypto.lib "${libcryptoArm64Release}" + File /oname=libssl.lib "${libsslArm64Release}" CreateDirectory "$INSTDIR\lib\Debug-Arm64" SetOutPath "$INSTDIR\lib\Debug-Arm64" File /oname=srt.lib "${BuildArm64Dir}\Debug\srt_static.lib" - File /oname=libcrypto.lib "${libcryptoArm64MDd}" - File /oname=libssl.lib "${libsslArm64MDd}" + File /oname=libcrypto.lib "${libcryptoArm64Debug}" + File /oname=libssl.lib "${libsslArm64Debug}" ; Add an environment variable to installation root. WriteRegStr HKLM ${EnvironmentKey} "LIBSRT" "$INSTDIR" From fd5638590939b0eb1e72836e8a2b2411555d6759 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Fri, 21 Feb 2025 12:35:47 +0100 Subject: [PATCH 02/14] [build] Added deprecation warning for Windows+PThreads configuration (#3112). --- CMakeLists.txt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 981440189..c65cbd592 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -43,11 +43,11 @@ set_if(DARWIN (${CMAKE_SYSTEM_NAME} MATCHES "Darwin") set_if(LINUX ${CMAKE_SYSTEM_NAME} MATCHES "Linux") set_if(BSD ${SYSNAME_LC} MATCHES "bsd$") set_if(MICROSOFT WIN32 AND (NOT MINGW AND NOT CYGWIN)) -set_if(GNU ${CMAKE_SYSTEM_NAME} MATCHES "GNU") +set_if(GNU_OS ${CMAKE_SYSTEM_NAME} MATCHES "GNU") # Use GNU_OS to not confuse with gcc set_if(ANDROID ${SYSNAME_LC} MATCHES "android") set_if(SUNOS "${SYSNAME_LC}" MATCHES "sunos") -set_if(POSIX LINUX OR DARWIN OR BSD OR SUNOS OR ANDROID OR (CYGWIN AND CYGWIN_USE_POSIX)) -set_if(SYMLINKABLE LINUX OR DARWIN OR BSD OR SUNOS OR CYGWIN OR GNU) +set_if(POSIX LINUX OR DARWIN OR BSD OR SUNOS OR ANDROID OR (CYGWIN AND CYGWIN_USE_POSIX) OR GNU_OS) +set_if(SYMLINKABLE LINUX OR DARWIN OR BSD OR SUNOS OR CYGWIN OR GNU_OS) set_if(NEED_DESTINATION ${CMAKE_VERSION} VERSION_LESS "3.14.0") include(GNUInstallDirs) @@ -881,6 +881,9 @@ elseif (PTHREAD_LIBRARY AND PTHREAD_INCLUDE_DIR) message(STATUS "Pthread library: ${PTHREAD_LIBRARY}") message(STATUS "Pthread include dir: ${PTHREAD_INCLUDE_DIR}") elseif (MICROSOFT) + + message(WARNING "DEPRECATION WARNING!\nUsing POSIX thread API with Windows is deprecated and will be removed") + find_package(pthreads QUIET) if (NOT PTHREAD_INCLUDE_DIR OR NOT PTHREAD_LIBRARY) From 7b77d4c93a7bc2e53e1cfd6dc83a1dcb0612d198 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Mon, 24 Feb 2025 10:34:31 +0100 Subject: [PATCH 03/14] [build] Fix use of the OPENSSL_USE_STATIC_LIBS CMake option (#3117). Now SRT_USE_OPENSSL_STATIC_LIBS is the CMake option of SRT. The OPENSSL_USE_STATIC_LIBS is CMake's option, still has to be used if SRT_USE_OPENSSL_STATIC_LIBS is set. --------- Co-authored-by: lilinxiong Co-authored-by: Mikolaj Malecki --- CMakeLists.txt | 32 +++++++++--- docs/build/build-options.md | 6 +-- scripts/ShowProjectConfig.cmake | 2 +- scripts/build-windows.ps1 | 2 +- srtcore/srt_shared.rc | 90 ++++++++++++++++----------------- 5 files changed, 76 insertions(+), 56 deletions(-) mode change 100644 => 100755 CMakeLists.txt diff --git a/CMakeLists.txt b/CMakeLists.txt old mode 100644 new mode 100755 index c65cbd592..d718792d1 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -172,7 +172,7 @@ option(ENABLE_CODE_COVERAGE "Enable code coverage reporting" OFF) option(ENABLE_MONOTONIC_CLOCK "Enforced clock_gettime with monotonic clock on GC CV" ${ENABLE_MONOTONIC_CLOCK_DEFAULT}) option(ENABLE_STDCXX_SYNC "Use C++11 chrono and threads for timing instead of pthreads" ${ENABLE_STDCXX_SYNC_DEFAULT}) option(USE_OPENSSL_PC "Use pkg-config to find OpenSSL libraries" ON) -option(OPENSSL_USE_STATIC_LIBS "Link OpenSSL libraries statically." OFF) +option(SRT_USE_OPENSSL_STATIC_LIBS "Link OpenSSL libraries statically." OFF) option(USE_BUSY_WAITING "Enable more accurate sending times at a cost of potentially higher CPU load" OFF) option(USE_GNUSTL "Get c++ library/headers from the gnustl.pc" OFF) option(ENABLE_SOCK_CLOEXEC "Enable setting SOCK_CLOEXEC on a socket" ON) @@ -180,6 +180,12 @@ option(ENABLE_SHOW_PROJECT_CONFIG "Enable show Project Configuration" OFF) option(ENABLE_CLANG_TSA "Enable Clang Thread Safety Analysis" OFF) +if (DEFINED OPENSSL_USE_STATIC_LIBS AND "${OPENSSL_USE_STATIC_LIBS}" STREQUAL "ON") + message(WARNING "Use of OPENSSL_USE_STATIC_LIBS as SRT build option here is deprecated. +Please use SRT_USE_OPENSSL_STATIC_LIBS instead.") + set(SRT_USE_OPENSSL_STATIC_LIBS ${OPENSSL_USE_STATIC_LIBS}) +endif() + # NOTE: Use ATOMIC_USE_SRT_SYNC_MUTEX and will override the auto-detection of the # Atomic implemetation in srtcore/atomic.h. option(ATOMIC_USE_SRT_SYNC_MUTEX "Use srt::sync::Mutex to Implement Atomics" OFF) @@ -389,7 +395,7 @@ if (ENABLE_ENCRYPTION) # fall back to find_package method otherwise if (USE_OPENSSL_PC) pkg_check_modules(SSL ${SSL_REQUIRED_MODULES}) - if (OPENSSL_USE_STATIC_LIBS) + if (SRT_USE_OPENSSL_STATIC_LIBS) # use `pkg-config --static xxx` found libs set(SSL_LIBRARIES ${SSL_STATIC_LIBRARIES}) endif() @@ -413,6 +419,11 @@ if (ENABLE_ENCRYPTION) ) message(STATUS "SSL via pkg-config: -L ${SSL_LIBRARY_DIRS} -I ${SSL_INCLUDE_DIRS} -l;${SSL_LIBRARIES}") else() + if (SRT_USE_OPENSSL_STATIC_LIBS) + # use `pkg-config --static xxx` found libs + set(OPENSSL_USE_STATIC_LIBS True) + set(OPENSSL_MSVC_STATIC_RT True) + endif() find_package(OpenSSL REQUIRED) set (SSL_INCLUDE_DIRS ${OPENSSL_INCLUDE_DIR}) set (SSL_LIBRARIES ${OPENSSL_LIBRARIES}) @@ -466,6 +477,10 @@ if (ENABLE_ENCRYPTION) # fall back to find_package method otherwise if (USE_OPENSSL_PC) pkg_check_modules(SSL ${SSL_REQUIRED_MODULES}) + if (SRT_USE_OPENSSL_STATIC_LIBS) + # use `pkg-config --static xxx` found libs + set(SSL_LIBRARIES ${SSL_STATIC_LIBRARIES}) + endif() endif() if (SSL_FOUND) # We have some cases when pkg-config is improperly configured @@ -486,6 +501,11 @@ if (ENABLE_ENCRYPTION) ) message(STATUS "SSL via pkg-config: -L ${SSL_LIBRARY_DIRS} -I ${SSL_INCLUDE_DIRS} -l;${SSL_LIBRARIES}") else() + if (SRT_USE_OPENSSL_STATIC_LIBS) + # use `pkg-config --static xxx` found libs + set(OPENSSL_USE_STATIC_LIBS True) + set(OPENSSL_MSVC_STATIC_RT True) + endif() find_package(OpenSSL REQUIRED) set (SSL_INCLUDE_DIRS ${OPENSSL_INCLUDE_DIR}) set (SSL_LIBRARIES ${OPENSSL_LIBRARIES}) @@ -1082,7 +1102,7 @@ if (srt_libspec_shared) if (MICROSOFT) target_link_libraries(${TARGET_srt}_shared PRIVATE ws2_32.lib) if (NOT (ENABLE_ENCRYPTION AND "${USE_ENCLIB}" STREQUAL "botan")) - if (OPENSSL_USE_STATIC_LIBS) + if (SRT_USE_OPENSSL_STATIC_LIBS) target_link_libraries(${TARGET_srt}_shared PRIVATE crypt32.lib) else() set_target_properties(${TARGET_srt}_shared PROPERTIES LINK_FLAGS "/DELAYLOAD:libeay32.dll") @@ -1121,7 +1141,7 @@ if (srt_libspec_static) endif() if (MICROSOFT) target_link_libraries(${TARGET_srt}_static PRIVATE ws2_32.lib) - if (OPENSSL_USE_STATIC_LIBS) + if (SRT_USE_OPENSSL_STATIC_LIBS) target_link_libraries(${TARGET_srt}_static PRIVATE crypt32.lib) endif() elseif (MINGW) @@ -1135,7 +1155,7 @@ endif() target_include_directories(srt_virtual PRIVATE ${SSL_INCLUDE_DIRS}) if (MICROSOFT) - if (OPENSSL_USE_STATIC_LIBS) + if (SRT_USE_OPENSSL_STATIC_LIBS) set (SRT_LIBS_PRIVATE ${SRT_LIBS_PRIVATE} ws2_32.lib crypt32.lib) else() set (SRT_LIBS_PRIVATE ${SRT_LIBS_PRIVATE} ws2_32.lib) @@ -1177,7 +1197,7 @@ endif() if (srt_libspec_shared) if (MICROSOFT) target_link_libraries(${TARGET_srt}_shared PUBLIC Ws2_32.lib) - if (OPENSSL_USE_STATIC_LIBS) + if (SRT_USE_OPENSSL_STATIC_LIBS) target_link_libraries(${TARGET_srt}_shared PUBLIC crypt32.lib) endif() endif() diff --git a/docs/build/build-options.md b/docs/build/build-options.md index 529bd5cd7..e09ea41bb 100644 --- a/docs/build/build-options.md +++ b/docs/build/build-options.md @@ -64,7 +64,7 @@ Option details are given further below. | [`USE_ENCLIB`](#use_enclib) | 1.3.3 | `STRING` | openssl | Encryption library to be used (`openssl`, `openssl-evp` (since 1.5.1), `gnutls`, `mbedtls`, `botan` (since 1.6.0)). | | [`USE_GNUSTL`](#use_gnustl) | 1.3.4 | `BOOL` | OFF | Use `pkg-config` with the `gnustl` package name to extract the header and library path for the C++ standard library. | | [`USE_OPENSSL_PC`](#use_openssl_pc) | 1.3.0 | `BOOL` | ON | Use `pkg-config` to find OpenSSL libraries. | -| [`OPENSSL_USE_STATIC_LIBS`](#openssl_use_static_libs) | 1.5.0 | `BOOL` | OFF | Link OpenSSL statically. | +| [`SRT_USE_OPENSSL_STATIC_LIBS`](#srt_use_openssl_static_libs)| 1.5.0 | `BOOL` | OFF | Link OpenSSL statically. | | [`USE_STATIC_LIBSTDCXX`](#use_static_libstdcxx) | 1.2.0 | `BOOL` | OFF | Enforces linking the SRT library against the static `libstdc++` library. | | [`WITH_COMPILER_PREFIX`](#with_compiler_prefix) | 1.3.0 | `STRING` | OFF | Sets C/C++ toolchains as `` and ``, overriding the default compiler. | | [`WITH_COMPILER_TYPE`](#with_compiler_type) | 1.3.0 | `STRING` | OFF | Sets the compiler type to be used (values: gcc, cc, clang, etc.). | @@ -618,8 +618,8 @@ built-in one). When ON, uses `pkg-config` to find OpenSSL libraries. You can turn this OFF to force `cmake` to find OpenSSL by its own preferred method. -### OPENSSL_USE_STATIC_LIBS -**`--openssl-use-static-libs`** (default: OFF) +### SRT_USE_OPENSSL_STATIC_LIBS +**`--srt-use-openssl-static-libs`** (default: OFF) When ON, OpenSSL libraries are linked statically. When `pkg-config`(`-DUSE_OPENSSL_PC=ON`) is used, static OpenSSL libraries are listed in `SSL_STATIC_LIBRARIES`. See `_STATIC` in [CMake's FindPkgConfig](https://cmake.org/cmake/help/latest/module/FindPkgConfig.html). diff --git a/scripts/ShowProjectConfig.cmake b/scripts/ShowProjectConfig.cmake index 108dde5db..9fc275bc8 100644 --- a/scripts/ShowProjectConfig.cmake +++ b/scripts/ShowProjectConfig.cmake @@ -166,7 +166,7 @@ function(ShowProjectConfig) " ENABLE_MONOTONIC_CLOCK: ${ENABLE_MONOTONIC_CLOCK}\n" " ENABLE_STDCXX_SYNC: ${ENABLE_STDCXX_SYNC}\n" " USE_OPENSSL_PC: ${USE_OPENSSL_PC}\n" - " OPENSSL_USE_STATIC_LIBS: ${OPENSSL_USE_STATIC_LIBS}\n" + " SRT_USE_OPENSSL_STATIC_LIBS: ${SRT_USE_OPENSSL_STATIC_LIBS}\n" " USE_BUSY_WAITING: ${USE_BUSY_WAITING}\n" " USE_GNUSTL: ${USE_GNUSTL}\n" " ENABLE_SOCK_CLOEXEC: ${ENABLE_SOCK_CLOEXEC}\n" diff --git a/scripts/build-windows.ps1 b/scripts/build-windows.ps1 index 66586158e..d2f07972d 100644 --- a/scripts/build-windows.ps1 +++ b/scripts/build-windows.ps1 @@ -175,7 +175,7 @@ if ( $VCPKG_OPENSSL -eq 'ON' ) { $cmakeFlags += " -DCMAKE_TOOLCHAIN_FILE=$projectRoot\vcpkg\scripts\buildsystems\vcpkg.cmake" } else { - $cmakeFlags += " -DOPENSSL_USE_STATIC_LIBS=$STATIC_LINK_SSL " + $cmakeFlags += " -DSRT_USE_OPENSSL_STATIC_LIBS=$STATIC_LINK_SSL " } # cmake uses a flag for architecture from vs2019, so add that as a suffix diff --git a/srtcore/srt_shared.rc b/srtcore/srt_shared.rc index 448192878..5639a0259 100644 --- a/srtcore/srt_shared.rc +++ b/srtcore/srt_shared.rc @@ -1,45 +1,45 @@ -// Microsoft Visual C++ generated resource script. -// -#include "version.h" -#include "winres.h" - -///////////////////////////////////////////////////////////////////////////// -// -// Version -// - -VS_VERSION_INFO VERSIONINFO -#ifdef SRT_VERSION_BUILD - FILEVERSION SRT_VERSION_MAJOR, SRT_VERSION_MINOR, SRT_VERSION_PATCH, SRT_VERSION_BUILD -#else - FILEVERSION SRT_VERSION_MAJOR, SRT_VERSION_MINOR, SRT_VERSION_PATCH -#endif - FILEFLAGSMASK 0x3fL -#ifdef _DEBUG - FILEFLAGS 0x1L -#else - FILEFLAGS 0x0L -#endif - FILEOS 0x40004L - FILETYPE 0x2L - FILESUBTYPE 0x0L -BEGIN - BLOCK "StringFileInfo" - BEGIN - BLOCK "080904b0" - BEGIN - VALUE "CompanyName", "SRT Alliance" - VALUE "FileDescription", "SRT Local Build" - VALUE "InternalName", "srt.dll" - VALUE "LegalCopyright", "" - VALUE "OriginalFilename", "srt.dll" - VALUE "ProductName", "SRT" - VALUE "ProductVersion", SRT_VERSION_STRING - END - END - BLOCK "VarFileInfo" - BEGIN - VALUE "Translation", 0x809, 1200 - END -END - +// Microsoft Visual C++ generated resource script. +// +#include "version.h" +#include "winres.h" + +///////////////////////////////////////////////////////////////////////////// +// +// Version +// + +VS_VERSION_INFO VERSIONINFO +#ifdef SRT_VERSION_BUILD + FILEVERSION SRT_VERSION_MAJOR, SRT_VERSION_MINOR, SRT_VERSION_PATCH, SRT_VERSION_BUILD +#else + FILEVERSION SRT_VERSION_MAJOR, SRT_VERSION_MINOR, SRT_VERSION_PATCH +#endif + FILEFLAGSMASK 0x3fL +#ifdef _DEBUG + FILEFLAGS 0x1L +#else + FILEFLAGS 0x0L +#endif + FILEOS 0x40004L + FILETYPE 0x2L + FILESUBTYPE 0x0L +BEGIN + BLOCK "StringFileInfo" + BEGIN + BLOCK "080904b0" + BEGIN + VALUE "CompanyName", "SRT Alliance" + VALUE "FileDescription", "SRT Local Build" + VALUE "InternalName", "srt.dll" + VALUE "LegalCopyright", "" + VALUE "OriginalFilename", "srt.dll" + VALUE "ProductName", "SRT" + VALUE "ProductVersion", SRT_VERSION_STRING + END + END + BLOCK "VarFileInfo" + BEGIN + VALUE "Translation", 0x809, 1200 + END +END + From 00e14f7bd9e15933a52005b33ad9c0f79c2d8d6d Mon Sep 17 00:00:00 2001 From: Maxim Sharabayko Date: Tue, 25 Feb 2025 10:22:12 +0100 Subject: [PATCH 04/14] [tests] Extending socket options test to group options (#3086). MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mikołaj Małecki Co-authored-by: Sektor van Skijlen --- .travis.yml | 4 + srtcore/group.cpp | 26 ++- srtcore/socketconfig.h | 4 +- test/test_bonding.cpp | 11 + test/test_env.h | 6 + test/test_socket_options.cpp | 425 ++++++++++++++++++++++++++++------- 6 files changed, 394 insertions(+), 82 deletions(-) diff --git a/.travis.yml b/.travis.yml index 94d5dac3a..0402fb51e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -65,6 +65,10 @@ matrix: - BUILD_TYPE=Release - BUILD_OPTS='-DENABLE_MONOTONIC_CLOCK=ON' script: + - 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++"; diff --git a/srtcore/group.cpp b/srtcore/group.cpp index 2c758396e..4c292cb89 100644 --- a/srtcore/group.cpp +++ b/srtcore/group.cpp @@ -461,6 +461,14 @@ void CUDTGroup::setOpt(SRT_SOCKOPT optName, const void* optval, int optlen) } } + // Before possibly storing the option, check if it is settable on a socket. + CSrtConfig testconfig; + + // Note: this call throws CUDTException by itself. + int result = testconfig.set(optName, optval, optlen); + if (result == -1) // returned in case of unknown option + throw CUDTException(MJ_NOTSUP, MN_INVAL, 0); + // Store the option regardless if pre or post. This will apply m_config.push_back(ConfigItem(optName, optval, optlen)); } @@ -724,8 +732,10 @@ static bool getOptDefault(SRT_SOCKOPT optname, void* pw_optval, int& w_optlen) RD(int64_t(-1)); case SRTO_INPUTBW: RD(int64_t(-1)); + case SRTO_MININPUTBW: + RD(int64_t(0)); case SRTO_OHEADBW: - RD(0); + RD(SRT_OHEAD_DEFAULT_P100); case SRTO_STATE: RD(SRTS_INIT); case SRTO_EVENT: @@ -746,8 +756,9 @@ static bool getOptDefault(SRT_SOCKOPT optname, void* pw_optval, int& w_optlen) RD(false); case SRTO_LATENCY: case SRTO_RCVLATENCY: - case SRTO_PEERLATENCY: RD(SRT_LIVE_DEF_LATENCY_MS); + case SRTO_PEERLATENCY: + RD(0); case SRTO_TLPKTDROP: RD(true); case SRTO_SNDDROPDELAY: @@ -758,14 +769,15 @@ static bool getOptDefault(SRT_SOCKOPT optname, void* pw_optval, int& w_optlen) RD(SRT_DEF_VERSION); case SRTO_PEERVERSION: RD(0); - + case SRTO_PEERIDLETIMEO: + RD(CSrtConfig::COMM_RESPONSE_TIMEOUT_MS); case SRTO_CONNTIMEO: - RD(-1); + RD(CSrtConfig::DEF_CONNTIMEO_S * 1000); // required milliseconds case SRTO_DRIFTTRACER: RD(true); case SRTO_MINVERSION: - RD(0); + RD(SRT_VERSION_MAJ1); case SRTO_STREAMID: RD(std::string()); case SRTO_CONGESTION: @@ -776,6 +788,10 @@ static bool getOptDefault(SRT_SOCKOPT optname, void* pw_optval, int& w_optlen) RD(0); case SRTO_GROUPMINSTABLETIMEO: RD(CSrtConfig::COMM_DEF_MIN_STABILITY_TIMEOUT_MS); + case SRTO_LOSSMAXTTL: + RD(0); + case SRTO_RETRANSMITALGO: + RD(1); } #undef RD diff --git a/srtcore/socketconfig.h b/srtcore/socketconfig.h index a6dd06caa..0f6c79887 100644 --- a/srtcore/socketconfig.h +++ b/srtcore/socketconfig.h @@ -70,6 +70,8 @@ written by #define SRT_VERSION_MIN(v) (0x00FF00 & (v)) #define SRT_VERSION_PCH(v) (0x0000FF & (v)) +static const int SRT_OHEAD_DEFAULT_P100 = 25; + // NOTE: SRT_VERSION is primarily defined in the build file. extern const int32_t SRT_DEF_VERSION; @@ -316,7 +318,7 @@ struct CSrtConfig: CSrtMuxerConfig , iCryptoMode(CIPHER_MODE_AUTO) , llInputBW(0) , llMinInputBW(0) - , iOverheadBW(25) + , iOverheadBW(SRT_OHEAD_DEFAULT_P100) , bRcvNakReport(true) , iMaxReorderTolerance(0) // Sensible optimal value is 10, 0 preserves old behavior , uKmRefreshRatePkt(0) diff --git a/test/test_bonding.cpp b/test/test_bonding.cpp index 4205b4665..e9efe939a 100644 --- a/test/test_bonding.cpp +++ b/test/test_bonding.cpp @@ -380,12 +380,18 @@ TEST(Bonding, Options) uint32_t val = 16; EXPECT_NE(srt_setsockflag(grp, SRTO_PBKEYLEN, &val, (int) sizeof val), SRT_ERROR); + const bool bfalse = true; + EXPECT_EQ(srt_setsockflag(grp, SRTO_RENDEZVOUS, &bfalse, (int)sizeof bfalse), SRT_ERROR); + #ifdef ENABLE_AEAD_API_PREVIEW val = 1; EXPECT_NE(srt_setsockflag(grp, SRTO_CRYPTOMODE, &val, sizeof val), SRT_ERROR); #endif #endif + const string packet_filter = "fec,cols:10,rows:5"; + EXPECT_NE(srt_setsockflag(grp, SRTO_PACKETFILTER, packet_filter.c_str(), (int)packet_filter.size()), SRT_ERROR); + // ================ // Linger is an option of a trivial type, but differes from other integer-typed options. // Therefore checking it specifically. @@ -451,6 +457,11 @@ TEST(Bonding, Options) check_streamid(gs); + std::array tmpbuf; + auto opt_len = (int)tmpbuf.size(); + EXPECT_EQ(srt_getsockflag(gs, SRTO_PACKETFILTER, tmpbuf.data(), &opt_len), SRT_SUCCESS); + std::cout << "Packet filter: " << std::string(tmpbuf.data(), opt_len) << '\n'; + // Connected, wait to close latch.wait(ux); diff --git a/test/test_env.h b/test/test_env.h index 5ca1a536e..edceee6e6 100644 --- a/test/test_env.h +++ b/test/test_env.h @@ -50,6 +50,12 @@ class TestEnv: public testing::Environment } static bool Allowed_Platform() { return false; } + +#ifdef ENABLE_BONDING + static bool Allowed_Bonding() { return true; } +#else + static bool Allowed_Bonding() { return false; } +#endif }; #define SRTST_REQUIRES(feature,...) if (!srt::TestEnv::Allowed_##feature(__VA_ARGS__)) { return; } diff --git a/test/test_socket_options.cpp b/test/test_socket_options.cpp index 58e60e761..fe27bb387 100644 --- a/test/test_socket_options.cpp +++ b/test/test_socket_options.cpp @@ -25,16 +25,27 @@ using namespace std; using namespace srt; +#define PLEASE_LOG 0 -class TestSocketOptions - : public ::srt::Test +#if PLEASE_LOG +#define LOGD(args) args +#else +#define LOGD(args) (void)0 +#endif + +class TestOptionsCommon: public srt::Test { protected: - TestSocketOptions() = default; + TestOptionsCommon() = default; + ~TestOptionsCommon() override = default; - ~TestSocketOptions() override = default; + sockaddr_any m_sa; + SRTSOCKET m_caller_sock = SRT_INVALID_SOCK; + SRTSOCKET m_listen_sock = SRT_INVALID_SOCK; + int m_pollid = 0; public: + void BindListener() const { // Specify address of the listener @@ -77,42 +88,104 @@ class TestSocketOptions return accepted_sock; } + void teardown() override + { + // Code here will be called just after the test completes. + // OK to throw exceptions from here if needed. + EXPECT_NE(srt_close(m_caller_sock), SRT_ERROR); + EXPECT_NE(srt_close(m_listen_sock), SRT_ERROR); + } +}; + + +class TestSocketOptions: public TestOptionsCommon +{ protected: + TestSocketOptions() = default; + ~TestSocketOptions() override = default; + // setup() is run immediately before a test starts. void setup() override { const int yes = 1; - - memset(&m_sa, 0, sizeof m_sa); - m_sa.sin_family = AF_INET; - m_sa.sin_port = htons(5200); - ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &m_sa.sin_addr), 1); + m_sa = srt::CreateAddr("127.0.0.1", 5200, AF_INET); + ASSERT_FALSE(m_sa.empty()); m_caller_sock = srt_create_socket(); - ASSERT_NE(m_caller_sock, SRT_INVALID_SOCK); + ASSERT_NE(m_caller_sock, SRT_INVALID_SOCK) << srt_getlasterror_str(); + m_listen_sock = srt_create_socket(); + ASSERT_NE(m_listen_sock, SRT_INVALID_SOCK) << srt_getlasterror_str(); + ASSERT_EQ(srt_setsockopt(m_caller_sock, 0, SRTO_RCVSYN, &yes, sizeof yes), SRT_SUCCESS); // for async connect ASSERT_EQ(srt_setsockopt(m_caller_sock, 0, SRTO_SNDSYN, &yes, sizeof yes), SRT_SUCCESS); // for async connect + ASSERT_EQ(srt_setsockopt(m_listen_sock, 0, SRTO_RCVSYN, &yes, sizeof yes), SRT_SUCCESS); // for async connect + ASSERT_EQ(srt_setsockopt(m_listen_sock, 0, SRTO_SNDSYN, &yes, sizeof yes), SRT_SUCCESS); // for async connect + } +}; + +#if ENABLE_BONDING +// Test group options +class TestGroupOptions: public TestOptionsCommon +{ +protected: + TestGroupOptions() = default; + ~TestGroupOptions() override = default; + + // Is run immediately before a test starts. + void setup() override + { + const int yes = 1; + + m_sa = srt::CreateAddr("127.0.0.1", 5200, AF_INET); + ASSERT_FALSE(m_sa.empty()); + + m_caller_sock = srt_create_group(SRT_GTYPE_BROADCAST); + ASSERT_NE(m_caller_sock, SRT_INVALID_SOCK) << srt_getlasterror_str(); m_listen_sock = srt_create_socket(); - ASSERT_NE(m_listen_sock, SRT_INVALID_SOCK); + ASSERT_NE(m_listen_sock, SRT_INVALID_SOCK) << srt_getlasterror_str(); + + ASSERT_EQ(srt_setsockopt(m_caller_sock, 0, SRTO_RCVSYN, &yes, sizeof yes), SRT_SUCCESS); // for async connect + ASSERT_EQ(srt_setsockopt(m_caller_sock, 0, SRTO_SNDSYN, &yes, sizeof yes), SRT_SUCCESS); // for async connect + ASSERT_EQ(srt_setsockopt(m_listen_sock, 0, SRTO_RCVSYN, &yes, sizeof yes), SRT_SUCCESS); // for async connect ASSERT_EQ(srt_setsockopt(m_listen_sock, 0, SRTO_SNDSYN, &yes, sizeof yes), SRT_SUCCESS); // for async connect + ASSERT_EQ(srt_setsockflag(m_listen_sock, SRTO_GROUPCONNECT, &yes, sizeof yes), SRT_SUCCESS); } +}; +#endif - void teardown() override +#if PLEASE_LOG +static std::string to_string(const linb::any& val) +{ + using namespace linb; + + if (val.type() == typeid(const char*)) { - // Code here will be called just after the test completes. - // OK to throw exceptions from here if needed. - EXPECT_NE(srt_close(m_caller_sock), SRT_ERROR); - EXPECT_NE(srt_close(m_listen_sock), SRT_ERROR); + return any_cast(val); } - sockaddr_in m_sa; - SRTSOCKET m_caller_sock = SRT_INVALID_SOCK; - SRTSOCKET m_listen_sock = SRT_INVALID_SOCK; + std::ostringstream out; + if (val.type() == typeid(bool)) + { + out << any_cast(val); + } + else if (val.type() == typeid(int)) + { + out << any_cast(val); + } + else if (val.type() == typeid(int64_t)) + { + out << any_cast(val); + } + else + { + return ""; + } - int m_pollid = 0; -}; + return out.str(); +} +#endif enum class RestrictionType { @@ -121,9 +194,70 @@ enum class RestrictionType POST = 2 }; +// FLAGS +// - READABLE (R) +// : You can call srt_getsockflag +// - WRITABLE (W) +// : You can call srt_setsockflag +// - SOCKETWISE (S) +// : Can be set on a socket +// - GROUPWISE (G) +// : Can be set on a group +// - DERIVED (D) +// : TRUE: If it's set on the group, it will be derived by members +// : FALSE: It cannot be set on the group to be derived by members +// - GROUPUNIQUE (I) +// : TRUE: If set on the group, it's assigned to a group (not members) +// : FALSE: If set on the group, it's derived by members +// - MODIFIABLE (M) +// : TRUE: Can be set on individual member socket differently. +// : FALSE: Cannot be altered on the individual member socket. + +namespace Flags +{ +enum type: char +{ + O = 0, // Marker for an unset flag + + R = 1 << 0, // readable + W = 1 << 1, // writable + S = 1 << 2, // can be set on single socket + G = 1 << 3, // can be set on group + D = 1 << 4, // when set on group, derived by the socket + I = 1 << 5, // when set on group, it concerns group only + M = 1 << 6 // can be modified on individual member +}; + +inline type operator|(type f1, type f2) +{ + char val = char(f1) | char(f2); + return type(val); +} + +inline bool operator&(type ff, type mask) +{ + char val = char(ff) & char(mask); + return val == mask; +} + +const std::string str(type t) +{ + static const char names [] = "RWSGDI+"; + std::string out; + + for (int i = 0; i < 7; ++i) + if (int(t) & (1 << i)) + out += names[i]; + + if (out.empty()) + return "O"; + return out; +} +} + const char* RestrictionTypeStr(RestrictionType val) { - const std::map type_to_str = { + static const std::map type_to_str = { { RestrictionType::PREBIND, "PREBIND" }, { RestrictionType::PRE, "PRE" }, { RestrictionType::POST, "POST" } @@ -143,26 +277,51 @@ struct OptionTestEntry linb::any dflt_val; linb::any ndflt_val; vector invalid_vals; + Flags::type flags; + + bool allof() const { return true; } + + template + bool allof(Flags::type flg, Args... args) const + { + return flags & flg && allof(args...); + } + + bool anyof() const { return false; } + + template + bool anyof(Flags::type flg, Args... args) const + { + return flags & flg || anyof(args...); + } }; static const size_t UDP_HDR_SIZE = 28; // 20 bytes IPv4 + 8 bytes of UDP { u16 sport, dport, len, csum }. static const size_t DFT_MTU_SIZE = 1500; // Default MTU size static const size_t SRT_PKT_SIZE = DFT_MTU_SIZE - UDP_HDR_SIZE; // MTU without UDP header +namespace Table +{ + // A trick to localize 1-letter flags without exposing + // them for the rest of the file. +using namespace Flags; + const OptionTestEntry g_test_matrix_options[] = { - // Option ID, Option Name | Restriction | optlen | min | max | default | nondefault | invalid vals | - //SRTO_BINDTODEVICE - //{ SRTO_CONGESTION, "SRTO_CONGESTION", RestrictionType::PRE, 4, "live", "file", "live", "file", {"liv", ""} }, - { SRTO_CONNTIMEO, "SRTO_CONNTIMEO", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 3000, 250, {-1} }, - { SRTO_DRIFTTRACER, "SRTO_DRIFTTRACER", RestrictionType::POST, sizeof(bool), false, true, true, false, {} }, - { SRTO_ENFORCEDENCRYPTION, "SRTO_ENFORCEDENCRYPTION", RestrictionType::PRE, sizeof(bool), false, true, true, false, {} }, - //SRTO_EVENT - { SRTO_FC, "SRTO_FC", RestrictionType::PRE, sizeof(int), 32, INT32_MAX, 25600, 10000, {-1, 31} }, - //SRTO_GROUPCONNECT + // Place 'O' if not set. + // Option ID, Option Name | Restriction | optlen | min | max | default | nondefault | invalid vals | flags: R | W | G | S | D | I | M + + //SRTO_BINDTODEVICE R | W | G | S | D | I | M + //{ SRTO_CONGESTION, "SRTO_CONGESTION", RestrictionType::PRE, 4, "live", "file", "live", "file", {"liv", ""}, O | W | O | S | O | O | O }, + { SRTO_CONNTIMEO, "SRTO_CONNTIMEO", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 3000, 250, {-1}, O | W | G | S | D | O | M }, + { SRTO_DRIFTTRACER, "SRTO_DRIFTTRACER", RestrictionType::POST, sizeof(bool), false, true, true, false, {}, R | W | G | S | D | O | O }, + { SRTO_ENFORCEDENCRYPTION, "SRTO_ENFORCEDENCRYPTION", RestrictionType::PRE, sizeof(bool), false, true, true, false, {}, O | W | G | S | D | O | O }, + //SRTO_EVENT R | O | O | S | O | O | O + { SRTO_FC, "SRTO_FC", RestrictionType::PRE, sizeof(int), 32, INT32_MAX, 25600, 10000, {-1, 31}, R | W | G | S | D | O | O }, + //SRTO_GROUPCONNECT O | W | O | S | O | O | O #if ENABLE_BONDING // Max value can't exceed SRTO_PEERIDLETIMEO - { SRTO_GROUPMINSTABLETIMEO, "SRTO_GROUPMINSTABLETIMEO", RestrictionType::PRE, sizeof(int), 60, 5000, 60, 70, {0, -1, 50, 5001} }, + { SRTO_GROUPMINSTABLETIMEO, "SRTO_GROUPMINSTABLETIMEO", RestrictionType::PRE, sizeof(int), 60, 5000, 60, 70, {0, -1, 50, 5001}, O | W | G | O | D | I | M }, #endif //SRTO_GROUPTYPE //SRTO_INPUTBW @@ -170,56 +329,66 @@ const OptionTestEntry g_test_matrix_options[] = //SRTO_IPTTL //SRTO_IPV6ONLY //SRTO_ISN - { SRTO_KMPREANNOUNCE, "SRTO_KMPREANNOUNCE", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 0, 1024, {-1} }, - { SRTO_KMREFRESHRATE, "SRTO_KMREFRESHRATE", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 0, 1024, {-1} }, + { SRTO_KMPREANNOUNCE, "SRTO_KMPREANNOUNCE", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 0, 1024, {-1}, O | W | G | S | D | O | O }, + { SRTO_KMREFRESHRATE, "SRTO_KMREFRESHRATE", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 0, 1024, {-1}, O | W | G | S | D | O | O }, //SRTO_KMSTATE - { SRTO_LATENCY, "SRTO_LATENCY", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 120, 200, {-1} }, + { SRTO_LATENCY, "SRTO_LATENCY", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 120, 200, {-1}, R | W | G | S | D | O | O }, //SRTO_LINGER - { SRTO_LOSSMAXTTL, "SRTO_LOSSMAXTTL", RestrictionType::POST, sizeof(int), 0, INT32_MAX, 0, 10, {} }, - { SRTO_MAXBW, "SRTO_MAXBW", RestrictionType::POST, sizeof(int64_t), int64_t(-1), INT64_MAX, int64_t(-1), int64_t(200000), {int64_t(-2)}}, + { SRTO_LOSSMAXTTL, "SRTO_LOSSMAXTTL", RestrictionType::POST, sizeof(int), 0, INT32_MAX, 0, 10, {}, R | W | G | S | D | O | M }, + { SRTO_MAXBW, "SRTO_MAXBW", RestrictionType::POST, sizeof(int64_t), int64_t(-1), INT64_MAX, int64_t(-1), int64_t(200000), {int64_t(-2)}, R | W | G | S | D | O | O }, #ifdef ENABLE_MAXREXMITBW - { SRTO_MAXREXMITBW, "SRTO_MAXREXMITBW", RestrictionType::POST, sizeof(int64_t), int64_t(-1), INT64_MAX, int64_t(-1), int64_t(200000), {int64_t(-2)}}, + { SRTO_MAXREXMITBW, "SRTO_MAXREXMITBW", RestrictionType::POST, sizeof(int64_t), int64_t(-1), INT64_MAX, int64_t(-1), int64_t(200000), {int64_t(-2)}, R | W | G | S | D | O | O }, #endif - { SRTO_MESSAGEAPI, "SRTO_MESSAGEAPI", RestrictionType::PRE, sizeof(bool), false, true, true, false, {} }, - { SRTO_MININPUTBW, "SRTO_MININPUTBW", RestrictionType::POST, sizeof(int64_t), int64_t(0), INT64_MAX, int64_t(0), int64_t(200000), {int64_t(-1)}}, - { SRTO_MINVERSION, "SRTO_MINVERSION", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 0x010000, 0x010300, {} }, - { SRTO_MSS, "SRTO_MSS", RestrictionType::PREBIND, sizeof(int), 76, 65536, 1500, 1400, {-1, 0, 75} }, - { SRTO_NAKREPORT, "SRTO_NAKREPORT", RestrictionType::PRE, sizeof(bool), false, true, true, false, {} }, - { SRTO_OHEADBW, "SRTO_OHEADBW", RestrictionType::POST, sizeof(int), 5, 100, 25, 20, {-1, 0, 4, 101} }, + { SRTO_MESSAGEAPI, "SRTO_MESSAGEAPI", RestrictionType::PRE, sizeof(bool), false, true, true, false, {}, O | W | G | S | D | O | O }, + { SRTO_MININPUTBW, "SRTO_MININPUTBW", RestrictionType::POST, sizeof(int64_t), int64_t(0), INT64_MAX, int64_t(0), int64_t(200000), {int64_t(-1)}, R | W | G | S | D | O | O }, + { SRTO_MINVERSION, "SRTO_MINVERSION", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 0x010000, 0x010300, {}, R | W | G | S | D | O | O }, + { SRTO_MSS, "SRTO_MSS", RestrictionType::PREBIND, sizeof(int), 76, 65536, 1500, 1400, {-1, 0, 75}, R | W | G | S | D | O | O }, + { SRTO_NAKREPORT, "SRTO_NAKREPORT", RestrictionType::PRE, sizeof(bool), false, true, true, false, {}, R | W | G | S | D | O | M }, + { SRTO_OHEADBW, "SRTO_OHEADBW", RestrictionType::POST, sizeof(int), 5, 100, 25, 20, {-1, 0, 4, 101}, R | W | G | S | D | O | O }, //SRTO_PACKETFILTER //SRTO_PASSPHRASE - { SRTO_PAYLOADSIZE, "SRTO_PAYLOADSIZE", RestrictionType::PRE, sizeof(int), 0, 1456, 1316, 1400, {-1, 1500} }, + { SRTO_PAYLOADSIZE, "SRTO_PAYLOADSIZE", RestrictionType::PRE, sizeof(int), 0, 1456, 1316, 1400, {-1, 1500}, O | W | G | S | D | O | O }, //SRTO_PBKEYLEN - //SRTO_PEERIDLETIMEO - { SRTO_PEERIDLETIMEO, "SRTO_PEERIDLETIMEO", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 5000, 4500, {-1} }, - { SRTO_PEERLATENCY, "SRTO_PEERLATENCY", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 0, 180, {-1} }, + { SRTO_PEERIDLETIMEO, "SRTO_PEERIDLETIMEO", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 5000, 4500, {-1}, R | W | G | S | D | O | M }, + { SRTO_PEERLATENCY, "SRTO_PEERLATENCY", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 0, 180, {-1}, R | W | G | S | D | O | O }, //SRTO_PEERVERSION - { SRTO_RCVBUF, "SRTO_RCVBUF", RestrictionType::PREBIND, sizeof(int), (int)(32 * SRT_PKT_SIZE), 2147483256, (int)(8192 * SRT_PKT_SIZE), 1000000, {-1} }, + { SRTO_RCVBUF, "SRTO_RCVBUF", RestrictionType::PREBIND, sizeof(int), (int)(32 * SRT_PKT_SIZE), 2147483256, (int)(8192 * SRT_PKT_SIZE), 1000000, {-1},R | W | G | S | D | O | M }, //SRTO_RCVDATA //SRTO_RCVKMSTATE - { SRTO_RCVLATENCY, "SRTO_RCVLATENCY", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 120, 1100, {-1} }, + { SRTO_RCVLATENCY, "SRTO_RCVLATENCY", RestrictionType::PRE, sizeof(int), 0, INT32_MAX, 120, 1100, {-1}, R | W | G | S | D | O | O }, //SRTO_RCVSYN - { SRTO_RCVTIMEO, "SRTO_RCVTIMEO", RestrictionType::POST, sizeof(int), -1, INT32_MAX, -1, 2000, {-2} }, + { SRTO_RCVTIMEO, "SRTO_RCVTIMEO", RestrictionType::POST, sizeof(int), -1, INT32_MAX, -1, 2000, {-2}, R | W | G | S | O | I | O }, //SRTO_RENDEZVOUS - { SRTO_RETRANSMITALGO, "SRTO_RETRANSMITALGO", RestrictionType::PRE, sizeof(int), 0, 1, 1, 0, {-1, 2} }, + { SRTO_RETRANSMITALGO, "SRTO_RETRANSMITALGO", RestrictionType::PRE, sizeof(int), 0, 1, 1, 0, {-1, 2}, R | W | G | S | D | O | O }, //SRTO_REUSEADDR //SRTO_SENDER - { SRTO_SNDBUF, "SRTO_SNDBUF", RestrictionType::PREBIND, sizeof(int), (int)(32 * SRT_PKT_SIZE), 2147483256, (int)(8192 * SRT_PKT_SIZE), 1000000, {-1} }, + { SRTO_SNDBUF, "SRTO_SNDBUF", RestrictionType::PREBIND, sizeof(int), (int)(32 * SRT_PKT_SIZE), 2147483256, (int)(8192 * SRT_PKT_SIZE), 1000000, {-1},R | W | G | S | D | O | M }, //SRTO_SNDDATA - { SRTO_SNDDROPDELAY, "SRTO_SNDDROPDELAY", RestrictionType::POST, sizeof(int), -1, INT32_MAX, 0, 1500, {-2} }, + { SRTO_SNDDROPDELAY, "SRTO_SNDDROPDELAY", RestrictionType::POST, sizeof(int), -1, INT32_MAX, 0, 1500, {-2}, O | W | G | S | D | O | M }, //SRTO_SNDKMSTATE //SRTO_SNDSYN - { SRTO_SNDTIMEO, "SRTO_SNDTIMEO", RestrictionType::POST, sizeof(int), -1, INT32_MAX, -1, 1400, {-2} }, + { SRTO_SNDTIMEO, "SRTO_SNDTIMEO", RestrictionType::POST, sizeof(int), -1, INT32_MAX, -1, 1400, {-2}, R | W | G | S | O | I | O }, //SRTO_STATE //SRTO_STREAMID - { SRTO_TLPKTDROP, "SRTO_TLPKTDROP", RestrictionType::PRE, sizeof(bool), false, true, true, false, {} }, + { SRTO_TLPKTDROP, "SRTO_TLPKTDROP", RestrictionType::PRE, sizeof(bool), false, true, true, false, {}, R | W | G | S | D | O | O }, //SRTO_TRANSTYPE //SRTO_TSBPDMODE //SRTO_UDP_RCVBUF //SRTO_UDP_SNDBUF //SRTO_VERSION }; +} // end namespace Table +using Table::g_test_matrix_options; + +template +void CheckGetSockOptMustFail(const OptionTestEntry& entry, SRTSOCKET sock, const char* desc) +{ + ValueType opt_val; + int opt_len = (int)entry.opt_len; + EXPECT_NE(srt_getsockopt(sock, 0, entry.optid, &opt_val, &opt_len), SRT_SUCCESS) + << desc << " Getting " << entry.optname << " must fail, but succeeded."; +} template void CheckGetSockOpt(const OptionTestEntry& entry, SRTSOCKET sock, const ValueType& value, const char* desc) @@ -264,6 +433,7 @@ void CheckSetSockOpt(const OptionTestEntry& entry, SRTSOCKET sock, const ValueTy template bool CheckDefaultValue(const OptionTestEntry& entry, SRTSOCKET sock, const char* desc) { + LOGD(cerr << "Will check default value: " << entry.optname << " = " << to_string(entry.dflt_val) << ": " << desc << endl); try { const ValueType dflt_val = linb::any_cast(entry.dflt_val); CheckGetSockOpt(entry, sock, dflt_val, desc); @@ -339,6 +509,7 @@ bool CheckInvalidValues(const OptionTestEntry& entry, SRTSOCKET sock, const char { for (const auto& inval : entry.invalid_vals) { + LOGD(cerr << "Will check INVALID value: " << entry.optname << " : " << to_string(inval) << ": " << sock_name << endl); try { const ValueType val = linb::any_cast(inval); CheckSetSockOpt(entry, sock, val, SRT_ERROR, sock_name); @@ -353,26 +524,76 @@ bool CheckInvalidValues(const OptionTestEntry& entry, SRTSOCKET sock, const char return true; } -TEST_F(TestSocketOptions, DefaultVals) +void TestDefaultValues(SRTSOCKET s) { + const char* test_desc = "[Caller, default]"; for (const auto& entry : g_test_matrix_options) { - const char* test_desc = "[Caller, default]"; + // Check flags. An option must be RW to test default value + const bool is_group = (s & SRTGROUP_MASK) != 0; + + if (!(entry.flags & Flags::R)) + { + // TODO: Check reading retuns an error. + LOGD(cerr << "Skipping " << entry.optname << ": not readable.\n"); + continue; // The flag must be READABLE and WRITABLE for this. + } + + // Check that retrieving a value must fail if the option is not a group option read on a group and not a socket + // option read on a socket. + bool readable = true; + if (is_group) + { + readable = entry.allof(Flags::G) && entry.anyof(Flags::I, Flags::D); + LOGD(cerr << "Group option " << entry.optname << ": expected " << (readable? "" : "NOT ") << "readable\n"); + } + else + { + readable = entry.allof(Flags::S); + LOGD(cerr << "Socket option " << entry.optname << ": expected " << (readable? "" : "NOT ") << "readable\n"); + } + + if (!readable) + { + if (entry.dflt_val.type() == typeid(bool)) + { + CheckGetSockOptMustFail(entry, s, test_desc); + } + else if (entry.dflt_val.type() == typeid(int)) + { + CheckGetSockOptMustFail(entry, s, test_desc); + } + else if (entry.dflt_val.type() == typeid(int64_t)) + { + CheckGetSockOptMustFail(entry, s, test_desc); + } + else if (entry.dflt_val.type() == typeid(const char*)) + { + CheckGetSockOptMustFail(entry, s, test_desc); + } + else + { + FAIL() << entry.optname << ": Unexpected type " << entry.dflt_val.type().name(); + } + + continue; // s is group && The option is not groupwise-individual option + } + if (entry.dflt_val.type() == typeid(bool)) { - EXPECT_TRUE(CheckDefaultValue(entry, m_caller_sock, test_desc)); + EXPECT_TRUE(CheckDefaultValue(entry, s, test_desc)); } else if (entry.dflt_val.type() == typeid(int)) { - EXPECT_TRUE(CheckDefaultValue(entry, m_caller_sock, test_desc)); + EXPECT_TRUE(CheckDefaultValue(entry, s, test_desc)); } else if (entry.dflt_val.type() == typeid(int64_t)) { - EXPECT_TRUE(CheckDefaultValue(entry, m_caller_sock, test_desc)); + EXPECT_TRUE(CheckDefaultValue(entry, s, test_desc)); } else if (entry.dflt_val.type() == typeid(const char*)) { - EXPECT_TRUE(CheckDefaultValue(entry, m_caller_sock, test_desc)); + EXPECT_TRUE(CheckDefaultValue(entry, s, test_desc)); } else { @@ -381,11 +602,34 @@ TEST_F(TestSocketOptions, DefaultVals) } } +TEST_F(TestSocketOptions, DefaultVals) +{ + TestDefaultValues(m_caller_sock); +} + +#if ENABLE_BONDING +TEST_F(TestGroupOptions, DefaultVals) +{ + SRTST_REQUIRES(Bonding); + TestDefaultValues(m_caller_sock); +} +#endif + TEST_F(TestSocketOptions, MaxVals) { // Note: Changing SRTO_FC changes SRTO_RCVBUF limitation for (const auto& entry : g_test_matrix_options) { + if (!(entry.flags & Flags::R)) + { + cerr << "Skipping " << entry.optname << ": option not readable\n"; + } + + if (!(entry.flags & Flags::W)) + { + cerr << "Skipping " << entry.optname << ": option not writable\n"; + } + if (entry.optid == SRTO_KMPREANNOUNCE || entry.optid == SRTO_KMREFRESHRATE) { cerr << "Skipping " << entry.optname << "\n"; @@ -419,6 +663,16 @@ TEST_F(TestSocketOptions, MinVals) // Note: Changing SRTO_FC changes SRTO_RCVBUF limitation for (const auto& entry : g_test_matrix_options) { + if (!(entry.flags & Flags::R)) + { + cerr << "Skipping " << entry.optname << ": option not readable\n"; + } + + if (!(entry.flags & Flags::W)) + { + cerr << "Skipping " << entry.optname << ": option not writable\n"; + } + const char* test_desc = "[Caller, min val]"; if (entry.min_val.type() == typeid(bool)) { @@ -441,23 +695,28 @@ TEST_F(TestSocketOptions, MinVals) } } -TEST_F(TestSocketOptions, InvalidVals) +void TestInvalidValues(SRTSOCKET s) { // Note: Changing SRTO_FC changes SRTO_RCVBUF limitation for (const auto& entry : g_test_matrix_options) { - const char* desc = "[Caller, invalid val]"; + if (!(entry.flags & Flags::W)) + { + cerr << "Skipping " << entry.optname << ": option not writable\n"; + } + + const char* desc = "[Group Caller, invalid val]"; if (entry.dflt_val.type() == typeid(bool)) { - EXPECT_TRUE(CheckInvalidValues(entry, m_caller_sock, desc)); + EXPECT_TRUE(CheckInvalidValues(entry, s, desc)); } else if (entry.dflt_val.type() == typeid(int)) { - EXPECT_TRUE(CheckInvalidValues(entry, m_caller_sock, desc)); + EXPECT_TRUE(CheckInvalidValues(entry, s, desc)); } else if (entry.dflt_val.type() == typeid(int64_t)) { - EXPECT_TRUE(CheckInvalidValues(entry, m_caller_sock, desc)); + EXPECT_TRUE(CheckInvalidValues(entry, s, desc)); } else { @@ -468,18 +727,32 @@ TEST_F(TestSocketOptions, InvalidVals) } } +TEST_F(TestSocketOptions, InvalidVals) +{ + TestInvalidValues(m_caller_sock); +} + + +#if ENABLE_BONDING +TEST_F(TestGroupOptions, InvalidVals) +{ + SRTST_REQUIRES(Bonding); + TestInvalidValues(m_caller_sock); +} +#endif + const char* StateToStr(SRT_SOCKSTATUS st) { std::map st_to_str = { - { SRTS_INIT, "SRTS_INIT" }, - { SRTS_OPENED, "SRTS_OPENED" }, - { SRTS_LISTENING, "SRTS_LISTENING" }, + { SRTS_INIT, "SRTS_INIT" }, + { SRTS_OPENED, "SRTS_OPENED" }, + { SRTS_LISTENING, "SRTS_LISTENING" }, { SRTS_CONNECTING, "SRTS_CONNECTING" }, - { SRTS_CONNECTED, "SRTS_CONNECTED" }, - { SRTS_BROKEN, "SRTS_BROKEN" }, - { SRTS_CLOSING, "SRTS_CLOSING" }, - { SRTS_CLOSED, "SRTS_CLOSED" }, - { SRTS_NONEXIST, "SRTS_NONEXIST" } + { SRTS_CONNECTED, "SRTS_CONNECTED" }, + { SRTS_BROKEN, "SRTS_BROKEN" }, + { SRTS_CLOSING, "SRTS_CLOSING" }, + { SRTS_CLOSED, "SRTS_CLOSED" }, + { SRTS_NONEXIST, "SRTS_NONEXIST" } }; return st_to_str.find(st) != st_to_str.end() ? st_to_str.at(st) : "INVALID"; @@ -871,7 +1144,7 @@ TEST_F(TestSocketOptions, StreamIDOdd) EXPECT_EQ(srt_setsockopt(m_caller_sock, 0, SRTO_STREAMID, sid_odd.c_str(), (int)sid_odd.size()), SRT_SUCCESS); - array buffer; + std::array buffer; auto buffer_len = (int) buffer.size(); EXPECT_EQ(srt_getsockopt(m_caller_sock, 0, SRTO_STREAMID, buffer.data(), &buffer_len), SRT_SUCCESS); EXPECT_EQ(std::string(buffer.data()), sid_odd); From dca5399b5257f9d5a5524e65031cc49f8d18c8b7 Mon Sep 17 00:00:00 2001 From: cl-ment Date: Wed, 26 Feb 2025 14:23:02 +0100 Subject: [PATCH 05/14] Automatic call of startup() and cleanup() #3098 (#3109) This pull request fixes issue #3098 by ensuring that startup() and cleanup() are called automatically. --------- Co-authored-by: Sektor van Skijlen Co-authored-by: Clement Gerouville Co-authored-by: Sektor van Skijlen --- .gitignore | 1 + srtcore/api.cpp | 224 +++++++++++++++++------------------ srtcore/api.h | 4 + srtcore/packetfilter.cpp | 97 +++++++++------ srtcore/packetfilter.h | 81 +++++++------ srtcore/socketconfig.cpp | 2 +- test/test_epoll.cpp | 5 +- test/test_fec_rebuilding.cpp | 1 - 8 files changed, 225 insertions(+), 190 deletions(-) diff --git a/.gitignore b/.gitignore index b4e22ff8d..5fc34a6a4 100644 --- a/.gitignore +++ b/.gitignore @@ -47,3 +47,4 @@ compile_commands.json # ignore files generated by clion and cmake .idea/ cmake-build-debug/ +tags diff --git a/srtcore/api.cpp b/srtcore/api.cpp index bb5dd64fe..1108c0a1d 100644 --- a/srtcore/api.cpp +++ b/srtcore/api.cpp @@ -189,11 +189,23 @@ srt::CUDTUnited::CUDTUnited() // XXX An unlikely exception thrown from the below calls // might destroy the application before `main`. This shouldn't // be a problem in general. + setupMutex(m_GCStartLock, "GCStart"); setupMutex(m_GCStopLock, "GCStop"); setupCond(m_GCStopCond, "GCStop"); setupMutex(m_GlobControlLock, "GlobControl"); setupMutex(m_IDLock, "ID"); setupMutex(m_InitLock, "Init"); + // Global initialization code +#ifdef _WIN32 + WORD wVersionRequested; + WSADATA wsaData; + wVersionRequested = MAKEWORD(2, 2); + + if (0 != WSAStartup(wVersionRequested, &wsaData)) + throw CUDTException(MJ_SETUP, MN_NONE, WSAGetLastError()); +#endif + CCryptoControl::globalInit(); + HLOGC(inlog.Debug, log << "SRT Clock Type: " << SRT_SYNC_CLOCK_STR); } srt::CUDTUnited::~CUDTUnited() @@ -201,11 +213,10 @@ srt::CUDTUnited::~CUDTUnited() // Call it if it wasn't called already. // This will happen at the end of main() of the application, // when the user didn't call srt_cleanup(). - if (m_bGCStatus) - { - cleanup(); - } - + enterCS(m_InitLock); + stopGarbageCollector(); + leaveCS(m_InitLock); + closeAllSockets(); releaseMutex(m_GlobControlLock); releaseMutex(m_IDLock); releaseMutex(m_InitLock); @@ -219,8 +230,11 @@ srt::CUDTUnited::~CUDTUnited() releaseCond(m_GCStopCond); #endif releaseMutex(m_GCStopLock); - + releaseMutex(m_GCStartLock); delete m_pCache; +#ifdef _WIN32 + WSACleanup(); +#endif } string srt::CUDTUnited::CONID(SRTSOCKET sock) @@ -233,39 +247,107 @@ string srt::CUDTUnited::CONID(SRTSOCKET sock) return os.str(); } -int srt::CUDTUnited::startup() +bool srt::CUDTUnited::startGarbageCollector() { - ScopedLock gcinit(m_InitLock); + + ScopedLock guard(m_GCStartLock); + if (!m_bGCStatus) + { + m_bClosing = false; + m_bGCStatus = StartThread(m_GCThread, garbageCollect, this, "SRT:GC"); + } + return m_bGCStatus; +} + +void srt::CUDTUnited::stopGarbageCollector() +{ + + ScopedLock guard(m_GCStartLock); if (m_bGCStatus) - return 1; + { + m_bGCStatus = false; + { + CUniqueSync gclock (m_GCStopLock, m_GCStopCond); + m_bClosing = true; + gclock.notify_all(); + } + m_GCThread.join(); + } +} - if (m_iInstanceCount++ > 0) - return 1; +void srt::CUDTUnited::closeAllSockets() +{ + // remove all sockets and multiplexers + HLOGC(inlog.Debug, log << "GC: GLOBAL EXIT - releasing all pending sockets. Acquring control lock..."); - // Global initialization code -#ifdef _WIN32 - WORD wVersionRequested; - WSADATA wsaData; - wVersionRequested = MAKEWORD(2, 2); + { + ScopedLock glock(m_GlobControlLock); - if (0 != WSAStartup(wVersionRequested, &wsaData)) - throw CUDTException(MJ_SETUP, MN_NONE, WSAGetLastError()); + for (sockets_t::iterator i = m_Sockets.begin(); i != m_Sockets.end(); ++i) + { + CUDTSocket* s = i->second; + s->breakSocket_LOCKED(); + +#if ENABLE_BONDING + if (s->m_GroupOf) + { + HLOGC(smlog.Debug, + log << "@" << s->m_SocketID << " IS MEMBER OF $" << s->m_GroupOf->id() + << " (IPE?) - REMOVING FROM GROUP"); + s->removeFromGroup(false); + } #endif + m_ClosedSockets[i->first] = s; - CCryptoControl::globalInit(); + // 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()) + continue; + } - PacketFilter::globalInit(); + enterCS(ls->second->m_AcceptLock); + ls->second->m_QueuedSockets.erase(s->m_SocketID); + leaveCS(ls->second->m_AcceptLock); + } + m_Sockets.clear(); - m_bClosing = false; + for (sockets_t::iterator j = m_ClosedSockets.begin(); j != m_ClosedSockets.end(); ++j) + { + j->second->m_tsClosureTimeStamp = steady_clock::time_point(); + } + } - if (!StartThread(m_GCThread, garbageCollect, this, "SRT:GC")) - return -1; + HLOGC(inlog.Debug, log << "GC: GLOBAL EXIT - releasing all CLOSED sockets."); + while (true) + { + checkBrokenSockets(); - m_bGCStatus = true; + enterCS(m_GlobControlLock); + bool empty = m_ClosedSockets.empty(); + leaveCS(m_GlobControlLock); + + if (empty) + break; + + HLOGC(inlog.Debug, log << "GC: checkBrokenSockets didn't wipe all sockets, repeating after 1s sleep"); + srt::sync::this_thread::sleep_for(milliseconds_from(1)); + } - HLOGC(inlog.Debug, log << "SRT Clock Type: " << SRT_SYNC_CLOCK_STR); - return 0; +} + + +int srt::CUDTUnited::startup() +{ + ScopedLock gcinit(m_InitLock); + m_iInstanceCount++; + if (m_bGCStatus) + return (m_iInstanceCount == 1) ? 1 : 0; + else + return startGarbageCollector() ? 0 : -1; } int srt::CUDTUnited::cleanup() @@ -283,28 +365,8 @@ int srt::CUDTUnited::cleanup() if (--m_iInstanceCount > 0) return 0; - if (!m_bGCStatus) - return 0; - - { - UniqueLock gclock(m_GCStopLock); - m_bClosing = true; - } - // NOTE: we can do relaxed signaling here because - // waiting on m_GCStopCond has a 1-second timeout, - // after which the m_bClosing flag is cheched, which - // is set here above. Worst case secenario, this - // pthread_join() call will block for 1 second. - CSync::notify_one_relaxed(m_GCStopCond); - m_GCThread.join(); - - m_bGCStatus = false; - - // Global destruction code -#ifdef _WIN32 - WSACleanup(); -#endif - + stopGarbageCollector(); + closeAllSockets(); return 0; } @@ -458,6 +520,7 @@ SRTSOCKET srt::CUDTUnited::newSocket(CUDTSocket** pps) throw CUDTException(MJ_SYSTEMRES, MN_MEMORY, 0); } + startGarbageCollector(); if (pps) *pps = ns; @@ -3391,66 +3454,6 @@ void* srt::CUDTUnited::garbageCollect(void* p) HLOGC(inlog.Debug, log << "GC: sleep 1 s"); self->m_GCStopCond.wait_for(gclock, seconds_from(1)); } - - // remove all sockets and multiplexers - HLOGC(inlog.Debug, log << "GC: GLOBAL EXIT - releasing all pending sockets. Acquring control lock..."); - - { - ScopedLock glock(self->m_GlobControlLock); - - for (sockets_t::iterator i = self->m_Sockets.begin(); i != self->m_Sockets.end(); ++i) - { - CUDTSocket* s = i->second; - s->breakSocket_LOCKED(); - -#if ENABLE_BONDING - if (s->m_GroupOf) - { - HLOGC(smlog.Debug, - log << "@" << s->m_SocketID << " IS MEMBER OF $" << s->m_GroupOf->id() - << " (IPE?) - REMOVING FROM GROUP"); - s->removeFromGroup(false); - } -#endif - self->m_ClosedSockets[i->first] = s; - - // remove from listener's queue - sockets_t::iterator ls = self->m_Sockets.find(s->m_ListenSocket); - if (ls == self->m_Sockets.end()) - { - ls = self->m_ClosedSockets.find(s->m_ListenSocket); - if (ls == self->m_ClosedSockets.end()) - continue; - } - - enterCS(ls->second->m_AcceptLock); - ls->second->m_QueuedSockets.erase(s->m_SocketID); - leaveCS(ls->second->m_AcceptLock); - } - self->m_Sockets.clear(); - - for (sockets_t::iterator j = self->m_ClosedSockets.begin(); j != self->m_ClosedSockets.end(); ++j) - { - j->second->m_tsClosureTimeStamp = steady_clock::time_point(); - } - } - - HLOGC(inlog.Debug, log << "GC: GLOBAL EXIT - releasing all CLOSED sockets."); - while (true) - { - self->checkBrokenSockets(); - - enterCS(self->m_GlobControlLock); - bool empty = self->m_ClosedSockets.empty(); - leaveCS(self->m_GlobControlLock); - - if (empty) - break; - - HLOGC(inlog.Debug, log << "GC: checkBrokenSockets didn't wipe all sockets, repeating after 1s sleep"); - srt::sync::this_thread::sleep_for(milliseconds_from(1)); - } - THREAD_EXIT(); return NULL; } @@ -3469,8 +3472,6 @@ int srt::CUDT::cleanup() SRTSOCKET srt::CUDT::socket() { - uglobal().startup(); - try { return uglobal().newSocket(); @@ -3518,9 +3519,6 @@ srt::CUDTGroup& srt::CUDT::newGroup(const int type) SRTSOCKET srt::CUDT::createGroup(SRT_GROUP_TYPE gt) { - // Doing the same lazy-startup as with srt_create_socket() - uglobal().startup(); - try { srt::sync::ScopedLock globlock(uglobal().m_GlobControlLock); diff --git a/srtcore/api.h b/srtcore/api.h index 48e7827f8..d0249ca10 100644 --- a/srtcore/api.h +++ b/srtcore/api.h @@ -462,6 +462,9 @@ class CUDTUnited CUDTSocket* locateAcquireSocket(SRTSOCKET u, ErrorHandling erh = ERH_RETURN); bool acquireSocket(CUDTSocket* s); + bool startGarbageCollector(); + void stopGarbageCollector(); + void closeAllSockets(); public: struct SocketKeeper @@ -534,6 +537,7 @@ class CUDTUnited private: srt::sync::atomic m_bClosing; + sync::Mutex m_GCStartLock; sync::Mutex m_GCStopLock; sync::Condition m_GCStopCond; diff --git a/srtcore/packetfilter.cpp b/srtcore/packetfilter.cpp index dc7e5b422..e8b2a6102 100644 --- a/srtcore/packetfilter.cpp +++ b/srtcore/packetfilter.cpp @@ -26,12 +26,13 @@ using namespace std; using namespace srt_logging; using namespace srt::sync; -bool srt::ParseFilterConfig(const string& s, SrtFilterConfig& w_config, PacketFilter::Factory** ppf) +namespace srt { +bool PacketFilter::Internal::ParseConfig(const string& s, SrtFilterConfig& w_config, PacketFilter::Factory** ppf) { if (!SrtParseConfig(s, (w_config))) return false; - PacketFilter::Factory* fac = PacketFilter::find(w_config.type); + PacketFilter::Factory* fac = find(w_config.type); if (!fac) return false; @@ -43,24 +44,26 @@ bool srt::ParseFilterConfig(const string& s, SrtFilterConfig& w_config, PacketFi return true; } -bool srt::ParseFilterConfig(const string& s, SrtFilterConfig& w_config) +bool ParseFilterConfig(const std::string& s, SrtFilterConfig& w_config) { - return ParseFilterConfig(s, (w_config), NULL); + return PacketFilter::internal().ParseConfig(s, (w_config), NULL); } // Parameters are passed by value because they need to be potentially modicied inside. -bool srt::CheckFilterCompat(SrtFilterConfig& w_agent, SrtFilterConfig peer) +bool PacketFilter::Internal::CheckFilterCompat(SrtFilterConfig& w_agent, const SrtFilterConfig& peer_in) { - PacketFilter::Factory* fac = PacketFilter::find(w_agent.type); + PacketFilter::Factory* fac = find(w_agent.type); if (!fac) return false; SrtFilterConfig defaults; - if (!ParseFilterConfig(fac->defaultConfig(), (defaults))) + if (!ParseConfig(fac->defaultConfig(), (defaults))) { return false; } + // Make a copy so that modifications can be done. This is only required for internal checks. + SrtFilterConfig peer = peer_in; set keys; // Extract all keys to identify also unspecified parameters on both sides // Note that theoretically for FEC it could simply check for the "cols" parameter @@ -109,20 +112,18 @@ bool srt::CheckFilterCompat(SrtFilterConfig& w_agent, SrtFilterConfig peer) return true; } -namespace srt { - struct SortBySequence +struct SortBySequence +{ + bool operator()(const CUnit* u1, const CUnit* u2) { - bool operator()(const CUnit* u1, const CUnit* u2) - { - int32_t s1 = u1->m_Packet.getSeqNo(); - int32_t s2 = u2->m_Packet.getSeqNo(); + int32_t s1 = u1->m_Packet.getSeqNo(); + int32_t s2 = u2->m_Packet.getSeqNo(); - return CSeqNo::seqcmp(s1, s2) < 0; - } - }; -} // namespace srt + return CSeqNo::seqcmp(s1, s2) < 0; + } +}; -void srt::PacketFilter::receive(CUnit* unit, std::vector& w_incoming, loss_seqs_t& w_loss_seqs) +void PacketFilter::receive(CUnit* unit, std::vector& w_incoming, loss_seqs_t& w_loss_seqs) { const CPacket& rpkt = unit->m_Packet; @@ -205,7 +206,7 @@ void srt::PacketFilter::receive(CUnit* unit, std::vector& w_incoming, lo } -bool srt::PacketFilter::packControlPacket(int32_t seq, int kflg, CPacket& w_packet) +bool PacketFilter::packControlPacket(int32_t seq, int kflg, CPacket& w_packet) { bool have = m_filter->packControlPacket(m_sndctlpkt, seq); if (!have) @@ -237,7 +238,7 @@ bool srt::PacketFilter::packControlPacket(int32_t seq, int kflg, CPacket& w_pack } -void srt::PacketFilter::InsertRebuilt(vector& incoming, CUnitQueue* uq) +void PacketFilter::InsertRebuilt(vector& incoming, CUnitQueue* uq) { if (m_provided.empty()) return; @@ -272,42 +273,61 @@ void srt::PacketFilter::InsertRebuilt(vector& incoming, CUnitQueue* uq) m_provided.clear(); } -bool srt::PacketFilter::IsBuiltin(const string& s) +// Placement here is necessary in order to mark the location to +// store the PacketFilter::Factory class characteristic object. +PacketFilter::Factory::~Factory() { - return builtin_filters.count(s); } -namespace srt { -std::set PacketFilter::builtin_filters; -PacketFilter::filters_map_t PacketFilter::filters; +#if HAVE_CXX11 + +PacketFilter::Internal& PacketFilter::internal() +{ + static PacketFilter::Internal instance; + return instance; +} + +#else // !HAVE_CXX11 + +static pthread_once_t s_PacketFactoryOnce = PTHREAD_ONCE_INIT; + +static PacketFilter::Internal *getInstance() +{ + static PacketFilter::Internal instance; + return &instance; } -srt::PacketFilter::Factory::~Factory() +PacketFilter::Internal& PacketFilter::internal() { + // We don't want lock each time, pthread_once can be faster than mutex. + pthread_once(&s_PacketFactoryOnce, reinterpret_cast(getInstance)); + return *getInstance(); } -void srt::PacketFilter::globalInit() +#endif + +PacketFilter::Internal::Internal() { // Add here builtin packet filters and mark them // as builtin. This will disallow users to register // external filters with the same name. - filters["fec"] = new Creator; - builtin_filters.insert("fec"); + m_filters["fec"] = new PacketFilter::Creator; + m_builtin_filters.insert("fec"); } -bool srt::PacketFilter::configure(CUDT* parent, CUnitQueue* uq, const std::string& confstr) +bool PacketFilter::configure(CUDT* parent, CUnitQueue* uq, const std::string& confstr) { m_parent = parent; SrtFilterConfig cfg; - if (!ParseFilterConfig(confstr, (cfg))) + if (!internal().ParseConfig(confstr, (cfg))) return false; // Extract the "type" key from parameters, or use // builtin if lacking. - filters_map_t::iterator selector = filters.find(cfg.type); - if (selector == filters.end()) + PacketFilter::Factory *factory = internal().find(cfg.type); + if (factory == NULL) return false; SrtFilterInitializer init; @@ -324,7 +344,7 @@ bool srt::PacketFilter::configure(CUDT* parent, CUnitQueue* uq, const std::strin << init.payload_size << " rcvbuf size=" << init.rcvbuf_size); // Found a filter, so call the creation function - m_filter = selector->second->Create(init, m_provided, confstr); + m_filter = factory->Create(init, m_provided, confstr); if (!m_filter) return false; @@ -336,7 +356,7 @@ bool srt::PacketFilter::configure(CUDT* parent, CUnitQueue* uq, const std::strin return true; } -bool srt::PacketFilter::correctConfig(const SrtFilterConfig& conf) +bool PacketFilter::correctConfig(const SrtFilterConfig& conf) { const string* pname = map_getp(conf.parameters, "type"); @@ -346,15 +366,16 @@ bool srt::PacketFilter::correctConfig(const SrtFilterConfig& conf) if (*pname == "adaptive") return true; - filters_map_t::iterator x = filters.find(*pname); - if (x == filters.end()) + PacketFilter::Factory *factory = internal().find(*pname); + if (factory == NULL) return false; return true; } -srt::PacketFilter::~PacketFilter() +PacketFilter::~PacketFilter() { delete m_filter; } +} // namespace srt diff --git a/srtcore/packetfilter.h b/srtcore/packetfilter.h index 429e81e79..2d1105042 100644 --- a/srtcore/packetfilter.h +++ b/srtcore/packetfilter.h @@ -51,7 +51,6 @@ class PacketFilter virtual ~Factory(); }; private: - friend bool ParseFilterConfig(const std::string& s, SrtFilterConfig& out, PacketFilter::Factory** ppf); template class Creator: public Factory @@ -74,7 +73,6 @@ class PacketFilter virtual ~Creator() {} }; - // We need a private wrapper for the auto-pointer, can't use // std::unique_ptr here due to no C++11. struct ManagedPtr @@ -114,14 +112,6 @@ class PacketFilter Factory* get() { return f; } }; - // The list of builtin names that are reserved. - static std::set builtin_filters; - - // Temporarily changed to linear searching, until this is exposed - // for a user-defined filter. - typedef std::map filters_map_t; - static filters_map_t filters; - // This is a filter container. SrtPacketFilterBase* m_filter; void Check() @@ -133,29 +123,52 @@ class PacketFilter // Don't do any check for now. } + friend class Internal; + public: + class Internal + { + public: - static void globalInit(); + // The list of builtin names that are reserved. + std::set m_builtin_filters; - static bool IsBuiltin(const std::string&); + // Temporarily changed to linear searching, until this is exposed + // for a user-defined filter. + typedef std::map filters_map_t; + filters_map_t m_filters; - template - static bool add(const std::string& name) - { - if (IsBuiltin(name)) - return false; - filters[name] = new Creator; - return true; - } + public: - static Factory* find(const std::string& type) - { - filters_map_t::iterator i = filters.find(type); - if (i == filters.end()) - return NULL; // No matter what to return - this is "undefined behavior" to be prevented - return i->second.get(); - } + bool IsBuiltin(const std::string& s) + { + return m_builtin_filters.count(s); + } + + template + bool add(const std::string& name) + { + if (IsBuiltin(name)) + return false; + + m_filters[name] = new Creator; + return true; + } + + Factory* find(const std::string& type) + { + filters_map_t::iterator i = m_filters.find(type); + if (i == m_filters.end()) + return NULL; // No matter what to return - this is "undefined behavior" to be prevented + return i->second.get(); + } + bool ParseConfig(const std::string& s, SrtFilterConfig& out, PacketFilter::Factory** ppf = NULL); + bool CheckFilterCompat(srt::SrtFilterConfig& w_agent, const srt::SrtFilterConfig& peer); + public: + Internal(); + }; + static Internal& internal(); // Singleton accessor // Filter is optional, so this check should be done always // manually. @@ -188,8 +201,8 @@ class PacketFilter ~PacketFilter(); // Simple wrappers - void feedSource(CPacket& w_packet); - SRT_ARQLevel arqLevel(); + void feedSource(CPacket& w_packet) { SRT_ASSERT(m_filter); return m_filter->feedSource((w_packet)); } + SRT_ARQLevel arqLevel() { SRT_ASSERT(m_filter); return m_filter->arqLevel(); } bool packControlPacket(int32_t seq, int kflg, CPacket& w_packet); void receive(CUnit* unit, std::vector& w_incoming, loss_seqs_t& w_loss_seqs); @@ -207,12 +220,10 @@ class PacketFilter std::vector m_provided; }; -bool CheckFilterCompat(SrtFilterConfig& w_agent, SrtFilterConfig peer); - -inline void PacketFilter::feedSource(CPacket& w_packet) { SRT_ASSERT(m_filter); return m_filter->feedSource((w_packet)); } -inline SRT_ARQLevel PacketFilter::arqLevel() { SRT_ASSERT(m_filter); return m_filter->arqLevel(); } - -bool ParseFilterConfig(const std::string& s, SrtFilterConfig& out, PacketFilter::Factory** ppf); +inline bool CheckFilterCompat(SrtFilterConfig& w_agent, const SrtFilterConfig& peer) +{ + return PacketFilter::internal().CheckFilterCompat((w_agent), peer); +} } // namespace srt diff --git a/srtcore/socketconfig.cpp b/srtcore/socketconfig.cpp index 1c067d059..a0c8a6a47 100644 --- a/srtcore/socketconfig.cpp +++ b/srtcore/socketconfig.cpp @@ -811,7 +811,7 @@ struct CSrtConfigSetter // Parse the configuration string prematurely SrtFilterConfig fc; PacketFilter::Factory* fax = 0; - if (!ParseFilterConfig(arg, (fc), (&fax))) + if (!PacketFilter::internal().ParseConfig(arg, (fc), (&fax))) { LOGC(aclog.Error, log << "SRTO_PACKETFILTER: Incorrect syntax. Use: FILTERTYPE[,KEY:VALUE...]. " diff --git a/test/test_epoll.cpp b/test/test_epoll.cpp index a8a88aa7a..f9d838030 100644 --- a/test/test_epoll.cpp +++ b/test/test_epoll.cpp @@ -12,6 +12,7 @@ using namespace std; using namespace srt; +#define TEST_UDP_PORT 9990 TEST(CEPoll, InfiniteWait) { @@ -571,7 +572,7 @@ class TestEPoll: public srt::Test sockaddr_in sa; memset(&sa, 0, sizeof sa); sa.sin_family = AF_INET; - sa.sin_port = htons(9999); + sa.sin_port = htons(TEST_UDP_PORT); ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1); @@ -645,7 +646,7 @@ class TestEPoll: public srt::Test sockaddr_in sa; memset(&sa, 0, sizeof sa); sa.sin_family = AF_INET; - sa.sin_port = htons(9999); + sa.sin_port = htons(TEST_UDP_PORT); sa.sin_addr.s_addr = INADDR_ANY; sockaddr* psa = (sockaddr*)&sa; diff --git a/test/test_fec_rebuilding.cpp b/test/test_fec_rebuilding.cpp index 7e1499212..3a7cb0791 100644 --- a/test/test_fec_rebuilding.cpp +++ b/test/test_fec_rebuilding.cpp @@ -29,7 +29,6 @@ class TestFECRebuilding: public srt::Test TestFECRebuilding() { // Required to make ParseCorrectorConfig work - PacketFilter::globalInit(); } void setup() override From 8bc76ac04e36e1e3dcf4c2f81b6e51092d5dd660 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Thu, 27 Feb 2025 13:20:49 +0100 Subject: [PATCH 06/14] [docs] Documentation fixes for socket options (#3125). --- docs/API/API-socket-options.md | 56 ++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/docs/API/API-socket-options.md b/docs/API/API-socket-options.md index d74907df1..c550845a7 100644 --- a/docs/API/API-socket-options.md +++ b/docs/API/API-socket-options.md @@ -201,7 +201,7 @@ The following table lists SRT API socket options in alphabetical order. Option d | Option Name | Since | Restrict | Type | Units | Default | Range | Dir |Entity | | :------------------------------------------------------ | :---: | :------: | :-------: | :-----: | :---------------: | :------: |:---:|:-----:| -| [`SRTO_BINDTODEVICE`](#SRTO_BINDTODEVICE) | 1.4.2 | pre-bind | `string` | | | | RW | S | +| [`SRTO_BINDTODEVICE`](#SRTO_BINDTODEVICE) | 1.4.2 | pre-bind | `string` | | "" | \* | RW | S | | [`SRTO_CONGESTION`](#SRTO_CONGESTION) | 1.3.0 | pre | `string` | | "live" | \* | W | S | | [`SRTO_CONNTIMEO`](#SRTO_CONNTIMEO) | 1.1.2 | pre | `int32_t` | ms | 3000 | 0.. | W | GSD+ | | [`SRTO_CRYPTOMODE`](#SRTO_CRYPTOMODE) | 1.5.2 | pre | `int32_t` | | 0 (Auto) | [0, 2] | W | GSD | @@ -210,7 +210,7 @@ The following table lists SRT API socket options in alphabetical order. Option d | [`SRTO_EVENT`](#SRTO_EVENT) | | | `int32_t` | flags | | | R | S | | [`SRTO_FC`](#SRTO_FC) | | pre | `int32_t` | pkts | 25600 | 32.. | RW | GSD | | [`SRTO_GROUPCONNECT`](#SRTO_GROUPCONNECT) | 1.5.0 | pre | `int32_t` | | 0 | 0...1 | W | S | -| [`SRTO_GROUPMINSTABLETIMEO`](#SRTO_GROUPMINSTABLETIMEO) | 1.5.0 | pre | `int32_t` | ms | 60 | 60-... | W | GDI+ | +| [`SRTO_GROUPMINSTABLETIMEO`](#SRTO_GROUPMINSTABLETIMEO) | 1.5.0 | pre | `int32_t` | ms | 60 | 60-... | W | GDI | | [`SRTO_GROUPTYPE`](#SRTO_GROUPTYPE) | 1.5.0 | | `int32_t` | enum | | | R | S | | [`SRTO_INPUTBW`](#SRTO_INPUTBW) | 1.0.5 | post | `int64_t` | B/s | 0 | 0.. | RW | GSD | | [`SRTO_IPTOS`](#SRTO_IPTOS) | 1.0.5 | pre-bind | `int32_t` | | (system) | 0..255 | RW | GSD | @@ -263,6 +263,48 @@ The following table lists SRT API socket options in alphabetical order. Option d | [`SRTO_UDP_SNDBUF`](#SRTO_UDP_SNDBUF) | | pre-bind | `int32_t` | bytes | 65536 | \* | RW | GSD+ | | [`SRTO_VERSION`](#SRTO_VERSION) | 1.1.0 | | `int32_t` | | | | R | S | +### Short summary for some general options' characteristics + +The following options cannot be set on a group: + +* [`SRTO_BINDTODEVICE`](#SRTO_BINDTODEVICE) - link-specific +* [`SRTO_CONGESTION`](#SRTO_CONGESTION) - "live" mode is the only supported for groups +* [`SRTO_GROUPCONNECT`](#SRTO_GROUPCONNECT) - to be set for a listener only +* [`SRTO_RENDEZVOUS`](#SRTO_RENDEZVOUS) - groups support only caller-listener mode +* [`SRTO_SENDER`](#SRTO_SENDER) - legacy option for <1.3.0, not available for bonding +* [`SRTO_TRANSTYPE`](#SRTO_TRANSTYPE) - live mode (default) is the only supported for groups +* [`SRTO_TSBPDMODE`](#SRTO_TSBPDMODE) + +The following options cannot be retrieved from a group: + +* [`SRTO_EVENT`](#SRTO_EVENT) - not supported +* [`SRTO_GROUPTYPE`](#SRTO_GROUPTYPE) - this is information for an incoming socket only +* [`SRTO_ISN`](#SRTO_ISN) - socket-specific +* [`SRTO_KMSTATE`](#SRTO_KMSTATE) - connection-specific +* [`SRTO_RCVDATA`](#SRTO_RCVDATA) - socket-specific +* [`SRTO_RCVKMSTATE`](#SRTO_RCVKMSTATE) - connection-specific +* [`SRTO_SNDDATA`](#SRTO_SNDDATA) - socket-specific (sender buffer is still one per socket, also in groups) +* [`SRTO_SNDKMSTATE`](#SRTO_SNDKMSTATE) - connection-specific +* [`SRTO_STATE`](#SRTO_STATE) - socket-specific state for the connection +* [`SRTO_VERSION`](#SRTO_VERSION) - not supported + +The following options can be set on a socket or on a group, but for +a group it has a different meaning than for a socket and they also +cannot be set on a group member socket. The reason for all of them +is such that the socket usually has this option set as required for +the group internals to work properly, while a member socket cannot +be used for reading and writing operations. + +* [`SRTO_RCVSYN`](#SRTO_RCVSYN) +* [`SRTO_RCVTIMEO`](#SRTO_RCVTIMEO) +* [`SRTO_SNDSYN`](#SRTO_SNDSYN) +* [`SRTO_SNDTIMEO`](#SRTO_SNDTIMEO) + +The following options can be set only on a group and not on a socket: + +* [`SRTO_GROUPMINSTABLETIMEO`](#SRTO_GROUPMINSTABLETIMEO) - specific for a group of type `SRT_GTYPE_BACKUP`. + + ### Option Descriptions #### SRTO_BINDTODEVICE @@ -304,7 +346,7 @@ if an appropriate instruction was given in the Stream ID. Currently supported congestion controllers are designated as "live" and "file", which correspond to the Live and File modes. -Note that it is not recommended to change this option manually, but you should +Note that it is not recommended to change this option directly, but you should rather change the whole set of options using the [`SRTO_TRANSTYPE`](#SRTO_TRANSTYPE) option. [Return to list](#list-of-options) @@ -317,9 +359,11 @@ rather change the whole set of options using the [`SRTO_TRANSTYPE`](#SRTO_TRANST | ------------------ | ----- | -------- | --------- | ------ | -------- | ------ | --- | ------ | | `SRTO_CONNTIMEO` | 1.1.2 | pre | `int32_t` | msec | 3000 | 0.. | W | GSD+ | -Connect timeout. This option applies to the caller and rendezvous connection -modes. For the rendezvous mode (see `SRTO_RENDEZVOUS`) the effective connection timeout -will be 10 times the value set with `SRTO_CONNTIMEO`. +Connection timeout value in milliseconds. This is the time up to which the connecting +facility will attempt to connect and wait for the response from the remote endpoint +before giving up with error status. The value applies for both caller and +rendezvous modes. For the rendezvous mode (see `SRTO_RENDEZVOUS`) the effective +connection timeout will be 10 times the value set with `SRTO_CONNTIMEO`. [Return to list](#list-of-options) From df81ecdff828cc8c5d10afd8a66b60fd799b1939 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Thu, 27 Feb 2025 13:26:04 +0100 Subject: [PATCH 07/14] [build] Fixed outdated SonarQube configuration (#3124). --- .github/workflows/cxx11-ubuntu.yaml | 4 ++-- sonar-project.properties | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/cxx11-ubuntu.yaml b/.github/workflows/cxx11-ubuntu.yaml index 918bbcd22..5566ed801 100644 --- a/.github/workflows/cxx11-ubuntu.yaml +++ b/.github/workflows/cxx11-ubuntu.yaml @@ -19,7 +19,7 @@ jobs: - name: configure run: | 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 + 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 . - name: test @@ -35,4 +35,4 @@ jobs: 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.build-wrapper-output=_build/"${{ env.BUILD_WRAPPER_OUT_DIR }}" + run: sonar-scanner --define sonar.cfamily.compile-commands="_build/compile_commands.json" diff --git a/sonar-project.properties b/sonar-project.properties index d11323df8..4bdaccdb2 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -32,5 +32,9 @@ 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 +#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 697dce0978c9e8c2f8fff4d1443f2cb69941bdc2 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Fri, 28 Feb 2025 10:02:28 +0100 Subject: [PATCH 08/14] [apps] Fixed correct bind address selection for windows multicast (#3126). Co-authored-by: BX --- apps/transmitmedia.cpp | 107 ++++++++++++++++++++------------------ scripts/build-windows.ps1 | 2 + testing/testmedia.cpp | 86 ++++++++++++++++-------------- testing/testmedia.hpp | 4 +- 4 files changed, 108 insertions(+), 91 deletions(-) diff --git a/apps/transmitmedia.cpp b/apps/transmitmedia.cpp index 8e535cf07..862875fa2 100644 --- a/apps/transmitmedia.cpp +++ b/apps/transmitmedia.cpp @@ -805,8 +805,10 @@ class UdpCommon { protected: int m_sock = -1; - sockaddr_any sadr; string adapter; + sockaddr_any interface_addr; + sockaddr_any target_addr; + bool is_multicast = false; map m_options; void Setup(string host, int port, map attr) @@ -821,33 +823,35 @@ class UdpCommon // set non-blocking mode #if defined(_WIN32) unsigned long ulyes = 1; - if (ioctlsocket(m_sock, FIONBIO, &ulyes) == SOCKET_ERROR) + const bool ioctl_error = (ioctlsocket(m_sock, FIONBIO, &ulyes) == SOCKET_ERROR); #else - if (ioctl(m_sock, FIONBIO, (const char *)&yes) < 0) + const bool ioctl_error = (ioctl(m_sock, FIONBIO, (const char *)&yes) == -1); #endif + if (ioctl_error) { Error(SysError(), "UdpCommon::Setup: ioctl FIONBIO"); } - sadr = CreateAddr(host, port); + interface_addr = CreateAddr("", port, AF_INET); + target_addr = CreateAddr(host, port); - bool is_multicast = false; + is_multicast = false; if (attr.count("multicast")) { // XXX: Here provide support for IPv6 multicast #1479 - if (sadr.family() != AF_INET) + if (target_addr.family() != AF_INET) { throw std::runtime_error("UdpCommon: Multicast on IPv6 is not yet supported"); } - if (!IsMulticast(sadr.sin.sin_addr)) + if (!IsMulticast(target_addr.sin.sin_addr)) { throw std::runtime_error("UdpCommon: requested multicast for a non-multicast-type IP address"); } is_multicast = true; } - else if (sadr.family() == AF_INET && IsMulticast(sadr.sin.sin_addr)) + else if (target_addr.family() == AF_INET && IsMulticast(target_addr.sin.sin_addr)) { is_multicast = true; } @@ -855,23 +859,19 @@ class UdpCommon if (is_multicast) { ip_mreq mreq; - sockaddr_any maddr (AF_INET); int opt_name; - void* mreq_arg_ptr; + const char* mreq_arg_ptr; socklen_t mreq_arg_size; adapter = attr.count("adapter") ? attr.at("adapter") : string(); - if ( adapter == "" ) + if (adapter == "") { Verb() << "Multicast: home address: INADDR_ANY:" << port; - maddr.sin.sin_family = AF_INET; - maddr.sin.sin_addr.s_addr = htonl(INADDR_ANY); - maddr.sin.sin_port = htons(port); // necessary for temporary use } else { Verb() << "Multicast: home address: " << adapter << ":" << port; - maddr = CreateAddr(adapter, port); + interface_addr = CreateAddr(adapter, port); } if (attr.count("source")) @@ -880,11 +880,11 @@ class UdpCommon ip_mreq_source mreq_ssm; /* this is an ssm. we need to use the right struct and opt */ opt_name = IP_ADD_SOURCE_MEMBERSHIP; - mreq_ssm.imr_multiaddr.s_addr = sadr.sin.sin_addr.s_addr; - mreq_ssm.imr_interface.s_addr = maddr.sin.sin_addr.s_addr; + mreq_ssm.imr_multiaddr.s_addr = target_addr.sin.sin_addr.s_addr; + mreq_ssm.imr_interface.s_addr = interface_addr.sin.sin_addr.s_addr; inet_pton(AF_INET, attr.at("source").c_str(), &mreq_ssm.imr_sourceaddr); mreq_arg_size = sizeof(mreq_ssm); - mreq_arg_ptr = &mreq_ssm; + mreq_arg_ptr = (const char*)&mreq_ssm; #else throw std::runtime_error("UdpCommon: source-filter multicast not supported by OS"); #endif @@ -892,39 +892,20 @@ class UdpCommon else { opt_name = IP_ADD_MEMBERSHIP; - mreq.imr_multiaddr.s_addr = sadr.sin.sin_addr.s_addr; - mreq.imr_interface.s_addr = maddr.sin.sin_addr.s_addr; + mreq.imr_multiaddr.s_addr = target_addr.sin.sin_addr.s_addr; + mreq.imr_interface.s_addr = interface_addr.sin.sin_addr.s_addr; mreq_arg_size = sizeof(mreq); - mreq_arg_ptr = &mreq; + mreq_arg_ptr = (const char*)&mreq; } #ifdef _WIN32 - const char* mreq_arg = (const char*)mreq_arg_ptr; const auto status_error = SOCKET_ERROR; #else - const void* mreq_arg = mreq_arg_ptr; const auto status_error = -1; #endif + auto res = setsockopt(m_sock, IPPROTO_IP, opt_name, mreq_arg_ptr, mreq_arg_size); -#if defined(_WIN32) || defined(__CYGWIN__) - // On Windows it somehow doesn't work when bind() - // is called with multicast address. Write the address - // that designates the network device here. - // Also, sets port sharing when working with multicast - sadr = maddr; - int reuse = 1; - int shareAddrRes = setsockopt(m_sock, SOL_SOCKET, SO_REUSEADDR, reinterpret_cast(&reuse), sizeof(reuse)); - if (shareAddrRes == status_error) - { - throw runtime_error("marking socket for shared use failed"); - } - Verb() << "Multicast(Windows): will bind to home address"; -#else - Verb() << "Multicast(POSIX): will bind to IGMP address: " << host; -#endif - int res = setsockopt(m_sock, IPPROTO_IP, opt_name, mreq_arg, mreq_arg_size); - - if ( res == status_error ) + if (res == status_error) { Error(errno, "adding to multicast membership failed"); } @@ -998,8 +979,35 @@ class UdpSource: public Source, public UdpCommon UdpSource(string host, int port, const map& attr) { Setup(host, port, attr); - int stat = ::bind(m_sock, sadr.get(), sadr.size()); - if ( stat == -1 ) + + // On Windows it somehow doesn't work when bind() + // is called with multicast address. Write the address + // that designates the network device here, always. + + // Hance we have two fields holding the address: + // * target_addr: address from which reading will be done + // * unicast: local address of the interface + // * multicast: the IGMP address + // * interface_addr: address of the local interface + // (same as target_addr for unicast) + + // In case of multicast, binding should be done: + // On most POSIX systems, to the multicast address (target_addr in this case) + // On Windows, to a local address (hence use interface_addr, as in this + // case it differs to target_addr). + +#if defined(_WIN32) || defined(__CYGWIN__) + sockaddr_any baddr = is_multicast ? interface_addr : target_addr; + static const char* const sysname = "Windows"; +#else + static const char* const sysname = "POSIX"; + sockaddr_any baddr = target_addr; +#endif + Verb("UDP:", is_multicast ? "Multicast" : "Unicast", "(", sysname, + "): will bind to: ", baddr.str()); + int stat = ::bind(m_sock, baddr.get(), baddr.size()); + + if (stat == -1) Error(SysError(), "Binding address for UDP"); eof = false; } @@ -1009,7 +1017,7 @@ class UdpSource: public Source, public UdpCommon if (pkt.payload.size() < chunk) pkt.payload.resize(chunk); - sockaddr_any sa(sadr.family()); + sockaddr_any sa(target_addr.family()); socklen_t si = sa.size(); int stat = recvfrom(m_sock, pkt.payload.data(), (int) chunk, 0, sa.get(), &si); if (stat < 1) @@ -1045,15 +1053,14 @@ class UdpTarget: public Target, public UdpCommon cerr << "\nWARN Host for UDP target is not provided. Will send to localhost:" << port << ".\n"; Setup(host, port, attr); - if (adapter != "") + if (is_multicast && interface_addr.isany() == false) { - sockaddr_any maddr = CreateAddr(adapter, 0); - if (maddr.family() != AF_INET) + if (interface_addr.family() != AF_INET) { Error(0, "UDP/target: IPv6 multicast not supported in the application"); } - in_addr addr = maddr.sin.sin_addr; + in_addr addr = interface_addr.sin.sin_addr; int res = setsockopt(m_sock, IPPROTO_IP, IP_MULTICAST_IF, reinterpret_cast(&addr), sizeof(addr)); if (res == -1) @@ -1066,7 +1073,7 @@ class UdpTarget: public Target, public UdpCommon int Write(const char* data, size_t len, int64_t src_time SRT_ATR_UNUSED, ostream & ignored SRT_ATR_UNUSED = cout) override { - int stat = sendto(m_sock, data, (int) len, 0, sadr.get(), sadr.size()); + int stat = sendto(m_sock, data, (int)len, 0, target_addr.get(), target_addr.size()); if ( stat == -1 ) { if ((false)) diff --git a/scripts/build-windows.ps1 b/scripts/build-windows.ps1 index d2f07972d..c4c7b5740 100644 --- a/scripts/build-windows.ps1 +++ b/scripts/build-windows.ps1 @@ -17,6 +17,7 @@ param ( [Parameter()][String]$STATIC_LINK_SSL = "OFF", [Parameter()][String]$CXX11 = "ON", [Parameter()][String]$BUILD_APPS = "ON", + [Parameter()][String]$BUILD_TESTAPPS = "OFF", [Parameter()][String]$UNIT_TESTS = "OFF", [Parameter()][String]$BUILD_DIR = "_build", [Parameter()][String]$VCPKG_OPENSSL = "OFF", @@ -135,6 +136,7 @@ $cmakeFlags = "-DCMAKE_BUILD_TYPE=$CONFIGURATION " + "-DENABLE_APPS=$BUILD_APPS " + "-DENABLE_ENCRYPTION=$ENABLE_ENCRYPTION " + "-DENABLE_BONDING=$BONDING " + + "-DENABLE_TESTING=$BUILD_TESTAPPS " + "-DENABLE_UNITTESTS=$UNIT_TESTS" # if VCPKG is flagged to provide OpenSSL, checkout VCPKG and install package diff --git a/testing/testmedia.cpp b/testing/testmedia.cpp index a7eb0541c..2c36a4d12 100755 --- a/testing/testmedia.cpp +++ b/testing/testmedia.cpp @@ -2753,20 +2753,22 @@ void UdpCommon::Setup(string host, int port, map attr) int yes = 1; ::setsockopt(m_sock, SOL_SOCKET, SO_REUSEADDR, (const char*)&yes, sizeof yes); - sadr = CreateAddr(host, port); + interface_addr = CreateAddr("", port, AF_INET); + target_addr = CreateAddr(host, port); - bool is_multicast = false; - if (sadr.family() == AF_INET) + is_multicast = false; + + if (target_addr.family() == AF_INET) { if (attr.count("multicast")) { - if (!IsMulticast(sadr.sin.sin_addr)) + if (!IsMulticast(target_addr.sin.sin_addr)) { throw std::runtime_error("UdpCommon: requested multicast for a non-multicast-type IP address"); } is_multicast = true; } - else if (IsMulticast(sadr.sin.sin_addr)) + else if (IsMulticast(target_addr.sin.sin_addr)) { is_multicast = true; } @@ -2774,23 +2776,19 @@ void UdpCommon::Setup(string host, int port, map attr) if (is_multicast) { ip_mreq mreq; - sockaddr_any maddr (AF_INET); int opt_name; - void* mreq_arg_ptr; + const char* mreq_arg_ptr; socklen_t mreq_arg_size; adapter = attr.count("adapter") ? attr.at("adapter") : string(); if (adapter == "") { Verb() << "Multicast: home address: INADDR_ANY:" << port; - maddr.sin.sin_family = AF_INET; - maddr.sin.sin_addr.s_addr = htonl(INADDR_ANY); - maddr.sin.sin_port = htons(port); // necessary for temporary use } else { Verb() << "Multicast: home address: " << adapter << ":" << port; - maddr = CreateAddr(adapter, port); + interface_addr = CreateAddr(adapter, port); } if (attr.count("source")) @@ -2799,11 +2797,11 @@ void UdpCommon::Setup(string host, int port, map attr) ip_mreq_source mreq_ssm; /* this is an ssm. we need to use the right struct and opt */ opt_name = IP_ADD_SOURCE_MEMBERSHIP; - mreq_ssm.imr_multiaddr.s_addr = sadr.sin.sin_addr.s_addr; - mreq_ssm.imr_interface.s_addr = maddr.sin.sin_addr.s_addr; + mreq_ssm.imr_multiaddr.s_addr = target_addr.sin.sin_addr.s_addr; + mreq_ssm.imr_interface.s_addr = interface_addr.sin.sin_addr.s_addr; inet_pton(AF_INET, attr.at("source").c_str(), &mreq_ssm.imr_sourceaddr); mreq_arg_size = sizeof(mreq_ssm); - mreq_arg_ptr = &mreq_ssm; + mreq_arg_ptr = (const char*)&mreq_ssm; #else throw std::runtime_error("UdpCommon: source-filter multicast not supported by OS"); #endif @@ -2811,37 +2809,18 @@ void UdpCommon::Setup(string host, int port, map attr) else { opt_name = IP_ADD_MEMBERSHIP; - mreq.imr_multiaddr.s_addr = sadr.sin.sin_addr.s_addr; - mreq.imr_interface.s_addr = maddr.sin.sin_addr.s_addr; + mreq.imr_multiaddr.s_addr = target_addr.sin.sin_addr.s_addr; + mreq.imr_interface.s_addr = interface_addr.sin.sin_addr.s_addr; mreq_arg_size = sizeof(mreq); - mreq_arg_ptr = &mreq; + mreq_arg_ptr = (const char*)&mreq; } #ifdef _WIN32 - const char* mreq_arg = (const char*)mreq_arg_ptr; const auto status_error = SOCKET_ERROR; #else - const void* mreq_arg = mreq_arg_ptr; const auto status_error = -1; #endif - -#if defined(_WIN32) || defined(__CYGWIN__) - // On Windows it somehow doesn't work when bind() - // is called with multicast address. Write the address - // that designates the network device here. - // Also, sets port sharing when working with multicast - sadr = maddr; - int reuse = 1; - int shareAddrRes = setsockopt(m_sock, SOL_SOCKET, SO_REUSEADDR, reinterpret_cast(&reuse), sizeof(reuse)); - if (shareAddrRes == status_error) - { - throw runtime_error("marking socket for shared use failed"); - } - Verb() << "Multicast(Windows): will bind to home address"; -#else - Verb() << "Multicast(POSIX): will bind to IGMP address: " << host; -#endif - int res = setsockopt(m_sock, IPPROTO_IP, opt_name, mreq_arg, mreq_arg_size); + auto res = setsockopt(m_sock, IPPROTO_IP, opt_name, mreq_arg_ptr, mreq_arg_size); if (res == status_error) { @@ -2913,7 +2892,34 @@ UdpCommon::~UdpCommon() UdpSource::UdpSource(string host, int port, const map& attr) { Setup(host, port, attr); - int stat = ::bind(m_sock, sadr.get(), sadr.size()); + + // On Windows it somehow doesn't work when bind() + // is called with multicast address. Write the address + // that designates the network device here, always. + + // Hance we have two fields holding the address: + // * target_addr: address from which reading will be done + // * unicast: local address of the interface + // * multicast: the IGMP address + // * interface_addr: address of the local interface + // (same as target_addr for unicast) + + // In case of multicast, binding should be done: + // On most POSIX systems, to the multicast address (target_addr in this case) + // On Windows, to a local address (hence use interface_addr, as in this + // case it differs to target_addr). + +#if defined(_WIN32) || defined(__CYGWIN__) + sockaddr_any baddr = is_multicast ? interface_addr : target_addr; + static const char* const sysname = "Windows"; +#else + static const char* const sysname = "POSIX"; + sockaddr_any baddr = target_addr; +#endif + Verb("UDP:", is_multicast ? "Multicast" : "Unicast", "(", sysname, + "): will bind to: ", baddr.str()); + int stat = ::bind(m_sock, baddr.get(), baddr.size()); + if (stat == -1) Error(SysError(), "Binding address for UDP"); eof = false; @@ -2927,7 +2933,7 @@ UdpSource::UdpSource(string host, int port, const map& attr) MediaPacket UdpSource::Read(size_t chunk) { bytevector data(chunk); - sockaddr_any sa(sadr.family()); + sockaddr_any sa(target_addr.family()); int64_t srctime = 0; AGAIN: int stat = recvfrom(m_sock, data.data(), (int) chunk, 0, sa.get(), &sa.syslen()); @@ -2975,7 +2981,7 @@ UdpTarget::UdpTarget(string host, int port, const map& attr) void UdpTarget::Write(const MediaPacket& data) { - int stat = sendto(m_sock, data.payload.data(), int(data.payload.size()), 0, (sockaddr*)&sadr, int(sizeof sadr)); + int stat = sendto(m_sock, data.payload.data(), int(data.payload.size()), 0, target_addr.get(), (int)target_addr.size()); if (stat == -1) Error(SysError(), "UDP Write/sendto"); } diff --git a/testing/testmedia.hpp b/testing/testmedia.hpp index 470e825ef..36a649130 100644 --- a/testing/testmedia.hpp +++ b/testing/testmedia.hpp @@ -304,8 +304,10 @@ class UdpCommon { protected: int m_sock = -1; - srt::sockaddr_any sadr; std::string adapter; + srt::sockaddr_any interface_addr; + srt::sockaddr_any target_addr; + bool is_multicast = false; std::map m_options; void Setup(std::string host, int port, std::map attr); void Error(int err, std::string src); From f3a8fe3f2883b0dacbc8400b97d474d3997e898b Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Wed, 12 Mar 2025 11:14:23 +0100 Subject: [PATCH 09/14] [core] Fixed bug: tsbpd might miss m_bClosing flag setting, flag not always properly set. (#1709) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [core] Fixed bug: tsbpd might miss m_bClosing flag setting, flag not always properly set. Added some sanity checks * Removed excessive variable * Added important comment from another PR * Localized the bWokeUpOnSignal variable better * Removed time-interrupted waiting on TsbPd thread as it caused more CPU usage --------- Co-authored-by: Mikołaj Małecki --- srtcore/core.cpp | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index dc1e2d118..0a8cc012b 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -4951,6 +4951,11 @@ EConnectStatus srt::CUDT::postConnect(const CPacket* pResponse, bool rendezvous, s->m_Status = SRTS_CONNECTED; // acknowledde any waiting epolls to write + // This must be done AFTER the group member status is upgraded to IDLE because + // this state change will trigger the waiting function in blocking-mode groupConnect + // and this may be immediately followed by exit from connect and start sending function, + // which must see this very link already as IDLE, not PENDING, which will make this + // link unable to be used and therefore the sending call would fail. uglobal().m_EPoll.update_events(m_SocketID, m_sPollID, SRT_EPOLL_CONNECT, true); CGlobEvent::triggerEvent(); @@ -5541,6 +5546,12 @@ void * srt::CUDT::tsbpd(void* param) gkeeper.group->updateLatestRcv(self->m_parent); } } + + // After re-acquisition of the m_RecvLock, re-check the closing flag + if (self->m_bClosing) + { + break; + } #endif CGlobEvent::triggerEvent(); tsNextDelivery = steady_clock::time_point(); // Ready to read, nothing to wait for. @@ -5566,6 +5577,7 @@ void * srt::CUDT::tsbpd(void* param) THREAD_PAUSED(); bWokeUpOnSignal = tsbpd_cc.wait_until(tsNextDelivery); THREAD_RESUMED(); + HLOGC(tslog.Debug, log << self->CONID() << "tsbpd: WAKE UP on " << (bWokeUpOnSignal? "SIGNAL" : "TIMEOUIT") << "!!!"); } else { @@ -6328,7 +6340,7 @@ bool srt::CUDT::closeInternal() ATR_NOEXCEPT // Inform the threads handler to stop. m_bClosing = true; - HLOGC(smlog.Debug, log << CONID() << "CLOSING STATE. Acquiring connection lock"); + HLOGC(smlog.Debug, log << CONID() << "CLOSING STATE (closing=true). Acquiring connection lock"); ScopedLock connectguard(m_ConnectionLock); @@ -7824,6 +7836,11 @@ void srt::CUDT::destroySynch() void srt::CUDT::releaseSynch() { SRT_ASSERT(m_bClosing); + if (!m_bClosing) + { + LOGC(smlog.Error, log << "releaseSynch: IPE: m_bClosing not set to false, TSBPD might hangup!"); + m_bClosing = true; + } // wake up user calls CSync::lock_notify_one(m_SendBlockCond, m_SendBlockLock); @@ -7831,8 +7848,8 @@ void srt::CUDT::releaseSynch() leaveCS(m_SendLock); // Awake tsbpd() and srt_recv*(..) threads for them to check m_bClosing. - CSync::lock_notify_one(m_RecvDataCond, m_RecvLock); - CSync::lock_notify_one(m_RcvTsbPdCond, m_RecvLock); + CSync::lock_notify_all(m_RecvDataCond, m_RecvLock); + CSync::lock_notify_all(m_RcvTsbPdCond, m_RecvLock); // Azquiring m_RcvTsbPdStartupLock protects race in starting // the tsbpd() thread in CUDT::processData(). @@ -9939,7 +9956,7 @@ void srt::CUDT::processClose() m_bBroken = true; m_iBrokenCounter = 60; - HLOGP(smlog.Debug, "processClose: sent message and set flags"); + HLOGP(smlog.Debug, "processClose: (closing=true) sent message and set flags"); if (m_bTsbPd) { @@ -11691,6 +11708,7 @@ void srt::CUDT::checkTimers() void srt::CUDT::updateBrokenConnection() { + HLOGC(smlog.Debug, log << "updateBrokenConnection: setting closing=true and taking out epoll events"); m_bClosing = true; releaseSynch(); // app can call any UDT API to learn the connection_broken error From f8e1bd6d3cc8f3abb1f4c60cb8b9a4359a2d7892 Mon Sep 17 00:00:00 2001 From: Mikolaj Malecki Date: Mon, 10 Mar 2025 12:41:49 +0100 Subject: [PATCH 10/14] [core] Late-rejection when listener responds with no packet filter --- srtcore/core.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 0a8cc012b..fef0d0c2a 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -2985,6 +2985,34 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, return false; } + // Check if the filter was configured on agent, + // but the peer responded with no filter. + if (!m_config.sPacketFilterConfig.empty() && !have_filter) + { + if (m_SrtHsSide == HSD_INITIATOR) + { + // IMPORTANT: + // If agent didn't set the packet filter in the configuration, the peer + // could have responded with its own configuration and enforce the packet filter + // setting here locally. In this case the sPacketFilterConfig will not be empty + // at the moment, even if it was empty initially. + + // The situation caught here is that the configuration for packet filter was set, + // but the peer responded as if it didn't understand it, or simply didn't accept it. + // In such a situation the configuration should be rejected because if the + // peer doesn't support packet filter, the caller side should not have set it. + m_RejectReason = SRT_REJ_FILTER; + LOGC(cnlog.Error, log << CONID() + << "HS EXT: Agent has configured packetfilter, but peer didn't respond."); + return false; + } + + // Responder: we are the listener that has configured packetfilter, + // but the caller didn't request packetfilter. + m_config.sPacketFilterConfig.set(""); + LOGC(cnlog.Warn, log << CONID() << "HS EXT: Agent has configured packetfilter, but peer didn't request it"); + } + #if ENABLE_BONDING // m_GroupOf and locking info: NULL check won't hurt here. If the group // was deleted in the meantime, it will be found out later anyway and result with error. From 0976f76a9db1c3144087a38c5c4df2200b600783 Mon Sep 17 00:00:00 2001 From: zhouliuya Date: Thu, 13 Mar 2025 10:22:27 +0000 Subject: [PATCH 11/14] [build] support OHOS(harmonyOS) system compile (#3139) * feat: support OHOS(harmonyOS) system compile * add support for OHOS platform detection. --- CMakeLists.txt | 6 +++++- srtcore/sync_posix.cpp | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d718792d1..847b6f23f 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -45,8 +45,9 @@ set_if(BSD ${SYSNAME_LC} MATCHES "bsd$") set_if(MICROSOFT WIN32 AND (NOT MINGW AND NOT CYGWIN)) set_if(GNU_OS ${CMAKE_SYSTEM_NAME} MATCHES "GNU") # Use GNU_OS to not confuse with gcc set_if(ANDROID ${SYSNAME_LC} MATCHES "android") +set_if(OHOS ${CMAKE_SYSTEM_NAME} MATCHES "OHOS") set_if(SUNOS "${SYSNAME_LC}" MATCHES "sunos") -set_if(POSIX LINUX OR DARWIN OR BSD OR SUNOS OR ANDROID OR (CYGWIN AND CYGWIN_USE_POSIX) OR GNU_OS) +set_if(POSIX LINUX OR DARWIN OR BSD OR SUNOS OR ANDROID OR OHOS OR (CYGWIN AND CYGWIN_USE_POSIX) OR GNU_OS) set_if(SYMLINKABLE LINUX OR DARWIN OR BSD OR SUNOS OR CYGWIN OR GNU_OS) set_if(NEED_DESTINATION ${CMAKE_VERSION} VERSION_LESS "3.14.0") @@ -791,6 +792,9 @@ elseif(LINUX) elseif(ANDROID) add_definitions(-DLINUX=1) message(STATUS "DETECTED SYSTEM: ANDROID; LINUX=1" ) +elseif(OHOS) + add_definitions(-DLINUX=1) + message(STATUS "DETECTED SYSTEM: OHOS; LINUX=1" ) elseif(CYGWIN) add_definitions(-DCYGWIN=1) message(STATUS "DETECTED SYSTEM: CYGWIN (posix mode); CYGWIN=1") diff --git a/srtcore/sync_posix.cpp b/srtcore/sync_posix.cpp index 8d7561a19..1668a9479 100644 --- a/srtcore/sync_posix.cpp +++ b/srtcore/sync_posix.cpp @@ -381,13 +381,13 @@ srt::sync::CThread& srt::sync::CThread::operator=(CThread& other) LOGC(inlog.Error, log << "IPE: Assigning to a thread that is not terminated!"); #ifndef DEBUG -#ifndef __ANDROID__ +#if !defined(__ANDROID__) && !defined(__OHOS__) // In case of production build the hanging thread should be terminated // to avoid hang ups and align with C++11 implementation. // There is no pthread_cancel on Android. See #1476. This error should not normally // happen, but if it happen, then detaching the thread. pthread_cancel(m_thread); -#endif // __ANDROID__ +#endif // __ANDROID__ __OHOS__ #else join(); #endif From 4d0e984088644224eb275f3f81ee66d09cd8f77c Mon Sep 17 00:00:00 2001 From: Mikolaj Malecki Date: Wed, 12 Mar 2025 11:17:23 +0100 Subject: [PATCH 12/14] [core] Fixed accidental blocking on sendmsg call --- srtcore/channel.cpp | 4 ++++ srtcore/core.cpp | 1 + 2 files changed, 5 insertions(+) diff --git a/srtcore/channel.cpp b/srtcore/channel.cpp index 0ec8f090f..81dbc4d35 100644 --- a/srtcore/channel.cpp +++ b/srtcore/channel.cpp @@ -552,6 +552,10 @@ void srt::CChannel::setUDPSockOpt() // Set receiving time-out value if (-1 == ::setsockopt(m_iSocket, SOL_SOCKET, SO_RCVTIMEO, (char*)&tv, sizeof(timeval))) throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR); + // Set sending time-out, too; O_NONBLOCK sets it in both directions, + // so both should be also set here for consistency. + if (-1 == ::setsockopt(m_iSocket, SOL_SOCKET, SO_SNDTIMEO, (char*)&tv, sizeof(timeval))) + throw CUDTException(MJ_SETUP, MN_NORES, NET_ERROR); #endif #ifdef SRT_ENABLE_PKTINFO diff --git a/srtcore/core.cpp b/srtcore/core.cpp index fef0d0c2a..f3812cc4a 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -8358,6 +8358,7 @@ int srt::CUDT::sendCtrlAck(CPacket& ctrlpkt, int size) { ctrlpkt.pack(UMSG_ACK, &m_iAckSeqNo, data, ACKD_FIELD_SIZE * ACKD_TOTAL_SIZE_SMALL); } + bufflock.unlock(); ctrlpkt.set_id(m_PeerID); setPacketTS(ctrlpkt, steady_clock::now()); From ee03ae90c4467d593067b5c6ee51ead2fcd1699d Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Fri, 14 Mar 2025 09:45:21 +0100 Subject: [PATCH 13/14] [tests] Fixed tests to avoid mistakes in CI (#3143) authored-by: Mikolaj Malecki --- test/test_bonding.cpp | 68 +++++++++++++++++++++++--------- test/test_connection_timeout.cpp | 28 +++++++------ 2 files changed, 64 insertions(+), 32 deletions(-) diff --git a/test/test_bonding.cpp b/test/test_bonding.cpp index e9efe939a..a1df9ec2d 100644 --- a/test/test_bonding.cpp +++ b/test/test_bonding.cpp @@ -726,13 +726,10 @@ TEST(Bonding, ConnectNonBlocking) for (const auto GTYPE: types) { g_listen_socket = srt_create_socket(); - sockaddr_in bind_sa; - memset(&bind_sa, 0, sizeof bind_sa); - bind_sa.sin_family = AF_INET; - EXPECT_EQ(inet_pton(AF_INET, ADDR.c_str(), &bind_sa.sin_addr), 1); - bind_sa.sin_port = htons(PORT); - EXPECT_NE(srt_bind(g_listen_socket, (sockaddr*)&bind_sa, sizeof bind_sa), -1); + sockaddr_any sa = srt::CreateAddr(ADDR, PORT, AF_INET); + + EXPECT_NE(srt_bind(g_listen_socket, sa.get(), sa.size()), -1); const int yes = 1; srt_setsockflag(g_listen_socket, SRTO_GROUPCONNECT, &yes, sizeof yes); EXPECT_NE(srt_listen(g_listen_socket, 5), -1); @@ -758,14 +755,17 @@ TEST(Bonding, ConnectNonBlocking) srt_connect_callback(ss, &ConnectCallback, this); - sockaddr_in sa; - sa.sin_family = AF_INET; - sa.sin_port = htons(PORT); - EXPECT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1); + cout << "TEST: Group type: " << GTYPE << endl; + + // synchronizers + std::promise + connect_passed, + accept_passed, + checks_done; //srt_setloglevel(LOG_DEBUG); - auto acthr = std::thread([&lsn_eid]() { + auto acthr = std::thread([&lsn_eid, &connect_passed, &accept_passed, &checks_done]() { SRT_EPOLL_EVENT ev[3]; ThreadName::set("TEST_A"); @@ -782,8 +782,15 @@ TEST(Bonding, ConnectNonBlocking) EXPECT_NE(ev[0].events & ev_in_bit, 0); bool have_also_update = ev[0].events & SRT_EPOLL_UPDATE; + cout << "[A] Accept delay until connect done...\n"; + // Delay with executing accept to keep the peer in "in progress" + // connection state. + connect_passed.get_future().get(); + + cout << "[A] Accept: go on\n"; + sockaddr_any adr; - int accept_id = srt_accept(g_listen_socket, adr.get(), &adr.len); + SRTSOCKET accept_id = srt_accept(g_listen_socket, adr.get(), &adr.len); // Expected: group reporting EXPECT_NE(accept_id & SRTGROUP_MASK, 0); @@ -803,19 +810,35 @@ TEST(Bonding, ConnectNonBlocking) EXPECT_EQ(ev[0].events, (int)SRT_EPOLL_UPDATE); } - cout << "[A] Waitig for close (up to 5s)\n"; + // As accept is expected to be finished and two connections were + // established, make sure that both connections are established. + // Wait until you have at least two members + SRT_SOCKGROUPDATA members[3]; + size_t grouplen = 3; + cout << "[A] Waiting until having 2 members in the group\n"; + while (srt_group_data(accept_id, members, &grouplen) != SRT_ERROR + && grouplen < 2) + { + grouplen = 3; + this_thread::sleep_for(milliseconds(250)); + } + accept_passed.set_value(); + + + cout << "[A] Waitig on epoll for close (up to 5s)\n"; // Wait up to 5s for an error srt_epoll_uwait(lsn_eid, ev, 3, 5000); - srt_close(accept_id); + checks_done.set_value(); + cout << "[A] thread finished\n"; }); cout << "Connecting two sockets\n"; SRT_SOCKGROUPCONFIG cc[2]; - cc[0] = srt_prepare_endpoint(NULL, (sockaddr*)&sa, sizeof sa); - cc[1] = srt_prepare_endpoint(NULL, (sockaddr*)&sa, sizeof sa); + cc[0] = srt_prepare_endpoint(NULL, sa.get(), sa.size()); + cc[1] = srt_prepare_endpoint(NULL, sa.get(), sa.size()); EXPECT_NE(srt_epoll_add_usock(poll_id, ss, &epoll_out), SRT_ERROR); @@ -824,21 +847,28 @@ TEST(Bonding, ConnectNonBlocking) char data[4] = { 1, 2, 3, 4}; cout << "Sending...\n"; int wrong_send = srt_send(ss, data, sizeof data); + + // Only now we allow the other thread to go on with accept + connect_passed.set_value(); cout << "Getting error...\n"; int errorcode = srt_getlasterror(NULL); EXPECT_EQ(wrong_send, -1); EXPECT_EQ(errorcode, SRT_EASYNCSND) << "REAL ERROR: " << srt_getlasterror_str(); + // Wait to make sure that both links are connected. + accept_passed.get_future().get(); + // Wait up to 2s SRT_EPOLL_EVENT ev[3]; const int uwait_result = srt_epoll_uwait(poll_id, ev, 3, 2000); - std::cout << "Returned from connecting two sockets " << std::endl; + std::cout << "Returned from connecting two sockets, waiting for all checks from [A]" << std::endl; + + checks_done.get_future().get(); EXPECT_EQ(uwait_result, 1); // Expect the group reported EXPECT_EQ(ev[0].fd, ss); - // One second to make sure that both links are connected. - this_thread::sleep_for(seconds(1)); + std::cout << "Closing group and releasing resources\n"; EXPECT_EQ(srt_close(ss), 0); acthr.join(); diff --git a/test/test_connection_timeout.cpp b/test/test_connection_timeout.cpp index ec52d9dd8..c8cbc8722 100644 --- a/test/test_connection_timeout.cpp +++ b/test/test_connection_timeout.cpp @@ -170,20 +170,22 @@ TEST_F(TestConnectionTimeout, Nonblocking) { */ TEST_F(TestConnectionTimeout, BlockingLoop) { - const SRTSOCKET client_sock = srt_create_socket(); - ASSERT_GT(client_sock, 0); // socket_id should be > 0 - - // Set connection timeout to 999 ms to reduce the test execution time. - // Also need to hit a time point between two threads: - // srt_connect will check TTL every second, - // CRcvQueue::worker will wait on a socket for 10 ms. - // Need to have a condition, when srt_connect will process the timeout. - const int connection_timeout_ms = 999; - EXPECT_EQ(srt_setsockopt(client_sock, 0, SRTO_CONNTIMEO, &connection_timeout_ms, sizeof connection_timeout_ms), SRT_SUCCESS); - const sockaddr* psa = reinterpret_cast(&m_sa); + const int connection_timeout_ms = 999; 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) + break; + + // Set connection timeout to 999 ms to reduce the test execution time. + // Also need to hit a time point between two threads: + // srt_connect will check TTL every second, + // CRcvQueue::worker will wait on a socket for 10 ms. + // Need to have a condition, when srt_connect will process the timeout. + EXPECT_EQ(srt_setsockopt(client_sock, 0, SRTO_CONNTIMEO, &connection_timeout_ms, sizeof connection_timeout_ms), SRT_SUCCESS); + const chrono::steady_clock::time_point chrono_ts_start = chrono::steady_clock::now(); EXPECT_EQ(srt_connect(client_sock, psa, sizeof m_sa), SRT_ERROR); @@ -200,9 +202,9 @@ TEST_F(TestConnectionTimeout, BlockingLoop) << error_code << " " << srt_getlasterror_str() << "\n"; break; } - } - EXPECT_EQ(srt_close(client_sock), SRT_SUCCESS); + EXPECT_EQ(srt_close(client_sock), SRT_SUCCESS); + } } From 952f9495246abc201bac55b8f9ad7409c0572423 Mon Sep 17 00:00:00 2001 From: Mikolaj Malecki Date: Fri, 21 Mar 2025 10:40:56 +0100 Subject: [PATCH 14/14] [test] Fixing ConnectNonBlocking: delay listen to prevent connection from being established too early --- test/test_bonding.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/test_bonding.cpp b/test/test_bonding.cpp index a1df9ec2d..e077ea1e0 100644 --- a/test/test_bonding.cpp +++ b/test/test_bonding.cpp @@ -732,7 +732,6 @@ TEST(Bonding, ConnectNonBlocking) EXPECT_NE(srt_bind(g_listen_socket, sa.get(), sa.size()), -1); const int yes = 1; srt_setsockflag(g_listen_socket, SRTO_GROUPCONNECT, &yes, sizeof yes); - EXPECT_NE(srt_listen(g_listen_socket, 5), -1); int lsn_eid = srt_epoll_create(); int lsn_events = SRT_EPOLL_IN | SRT_EPOLL_ERR | SRT_EPOLL_UPDATE; @@ -770,8 +769,14 @@ TEST(Bonding, ConnectNonBlocking) ThreadName::set("TEST_A"); - cout << "[A] Waiting for accept\n"; + cout << "[A] Waiting for main thread to pass connect()\n"; + + // Delay with executing accept to keep the peer in "in progress" + // connection state. + connect_passed.get_future().get(); + EXPECT_NE(srt_listen(g_listen_socket, 5), SRT_ERROR); + cout << "[A] Waiting for accept\n"; // This can wait in infinity; worst case it will be killed in process. int uwait_res = srt_epoll_uwait(lsn_eid, ev, 3, -1); EXPECT_EQ(uwait_res, 1); @@ -783,9 +788,6 @@ TEST(Bonding, ConnectNonBlocking) bool have_also_update = ev[0].events & SRT_EPOLL_UPDATE; cout << "[A] Accept delay until connect done...\n"; - // Delay with executing accept to keep the peer in "in progress" - // connection state. - connect_passed.get_future().get(); cout << "[A] Accept: go on\n";