From 351c69a9f7cf00e45bc66821d3d79c325bffaad2 Mon Sep 17 00:00:00 2001 From: Clinton Nkwocha Date: Tue, 9 Dec 2025 18:28:10 +0000 Subject: [PATCH 1/2] Serialize `curl_multi_cleanup` calls for OpenSSL <1.1.0 Motivation: `URLSession` elusively crashes due to race conditions in OpenSSL <1.1.0 when calling `curl_multi_cleanup` concurrently on different threads. Modifications: - Add `CFURLSessionSSLVersion CFURLSessionSSLVersionInfo(void)` for detecting the OpenSSL version in use. - Adjust `_MultiHandle.deinit` to use a process-wide lock to serialize `curl_multi_cleanup` calls for environments with OpenSSL <1.1.0. Result: Prevents the crash while avoiding unnecessary performance overhead for newer OpenSSL versions which are thread-safe. Testing: Passing existing tests is sufficient. However, without this patch, the code snippet below crashes about once in 20 runs on Amazon Linux 2 > $ curl --version > _curl 8.3.0 (aarch64-koji-linux-gnu) libcurl/8.3.0 OpenSSL/1.0.2k-fips zlib/1.2.7 libidn2/2.3.0 libpsl/0.21.5 (+libidn2/2.3.0) libssh2/1.4.3 nghttp2/1.41.0 OpenLDAP/2.4.44_ > _Release-Date: 2023-09-13_ ... ``` DispatchQueue.concurrentPerform(iterations: 100) { _ in let session: URLSession = URLSession(configuration: URLSessionConfiguration.default) DispatchQueue.concurrentPerform(iterations: 100) { _ in _ = session } } ``` --- .../URLSession/libcurl/MultiHandle.swift | 18 +++++++++++++++++- .../CFURLSessionInterface.c | 16 ++++++++++++++++ .../include/CFURLSessionInterface.h | 7 +++++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/Sources/FoundationNetworking/URLSession/libcurl/MultiHandle.swift b/Sources/FoundationNetworking/URLSession/libcurl/MultiHandle.swift index f1b3ad6a21..8f885fdf0f 100644 --- a/Sources/FoundationNetworking/URLSession/libcurl/MultiHandle.swift +++ b/Sources/FoundationNetworking/URLSession/libcurl/MultiHandle.swift @@ -48,6 +48,15 @@ extension URLSession { fileprivate var timeoutSource: _TimeoutSource? = nil private var reentrantInUpdateTimeoutTimer = false + // Only use serialization for OpenSSL < 1.1.0 which has race conditions during cleanup + private static let _needsCleanupSerialization: Bool = { + let version = CFURLSessionSSLVersionInfo() + return version.major < 1 || (version.major == 1 && version.minor < 1) + }() + + // Process-wide cleanup lock + private static let _cleanupLock = NSLock() + init(configuration: URLSession._Configuration, workQueue: DispatchQueue) { queue = DispatchQueue(label: "MultiHandle.isolation", target: workQueue) setupCallbacks() @@ -58,7 +67,14 @@ extension URLSession { easyHandles.forEach { try! CFURLSessionMultiHandleRemoveHandle(rawHandle, $0.rawHandle).asError() } - try! CFURLSessionMultiHandleDeinit(rawHandle).asError() + + if Self._needsCleanupSerialization { + Self._cleanupLock.lock() + try! CFURLSessionMultiHandleDeinit(rawHandle).asError() + Self._cleanupLock.unlock() + } else { + try! CFURLSessionMultiHandleDeinit(rawHandle).asError() + } } } } diff --git a/Sources/_CFURLSessionInterface/CFURLSessionInterface.c b/Sources/_CFURLSessionInterface/CFURLSessionInterface.c index 327397fe78..7c1fbfd6e5 100644 --- a/Sources/_CFURLSessionInterface/CFURLSessionInterface.c +++ b/Sources/_CFURLSessionInterface/CFURLSessionInterface.c @@ -676,6 +676,22 @@ CFURLSessionCurlVersion CFURLSessionCurlVersionInfo(void) { return v; } +CFURLSessionSSLVersion CFURLSessionSSLVersionInfo(void) { + curl_version_info_data *info = curl_version_info(CURLVERSION_NOW); + CFURLSessionSSLVersion version = {0, 0, 0}; + + if (info && info->ssl_version) { + // Parse OpenSSL version string like "OpenSSL/1.0.2k-fips" or "OpenSSL/1.1.1" + const char *ssl_str = info->ssl_version; + if (strncmp(ssl_str, "OpenSSL/", 8) == 0) { + ssl_str += 8; // Skip "OpenSSL/" + sscanf(ssl_str, "%d.%d.%d", &version.major, &version.minor, &version.patch); + } + } + + return version; +} + int const CFURLSessionWriteFuncPause = CURL_WRITEFUNC_PAUSE; int const CFURLSessionReadFuncPause = CURL_READFUNC_PAUSE; diff --git a/Sources/_CFURLSessionInterface/include/CFURLSessionInterface.h b/Sources/_CFURLSessionInterface/include/CFURLSessionInterface.h index c6ca3766be..201168ff2e 100644 --- a/Sources/_CFURLSessionInterface/include/CFURLSessionInterface.h +++ b/Sources/_CFURLSessionInterface/include/CFURLSessionInterface.h @@ -580,6 +580,13 @@ typedef struct CFURLSessionCurlVersion { } CFURLSessionCurlVersion; CF_EXPORT CFURLSessionCurlVersion CFURLSessionCurlVersionInfo(void); +typedef struct CFURLSessionSSLVersion { + int major; + int minor; + int patch; +} CFURLSessionSSLVersion; +CF_EXPORT CFURLSessionSSLVersion CFURLSessionSSLVersionInfo(void); + CF_EXPORT int const CFURLSessionWriteFuncPause; CF_EXPORT int const CFURLSessionReadFuncPause; From daf40f449a8da947a639098a75e74e7dea8fa834 Mon Sep 17 00:00:00 2001 From: Clinton Nkwocha Date: Thu, 18 Dec 2025 19:56:53 +0000 Subject: [PATCH 2/2] Implement feedback --- .../URLSession/libcurl/MultiHandle.swift | 5 ++++- .../_CFURLSessionInterface/CFURLSessionInterface.c | 11 ++++++----- .../include/CFURLSessionInterface.h | 6 +++--- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/Sources/FoundationNetworking/URLSession/libcurl/MultiHandle.swift b/Sources/FoundationNetworking/URLSession/libcurl/MultiHandle.swift index 8f885fdf0f..a70f0315b5 100644 --- a/Sources/FoundationNetworking/URLSession/libcurl/MultiHandle.swift +++ b/Sources/FoundationNetworking/URLSession/libcurl/MultiHandle.swift @@ -50,7 +50,10 @@ extension URLSession { // Only use serialization for OpenSSL < 1.1.0 which has race conditions during cleanup private static let _needsCleanupSerialization: Bool = { - let version = CFURLSessionSSLVersionInfo() + guard let version = CFURLSessionOpenSSLVersionInfo()?.pointee else { + // Not OpenSSL, assume thread-safe + return false + } return version.major < 1 || (version.major == 1 && version.minor < 1) }() diff --git a/Sources/_CFURLSessionInterface/CFURLSessionInterface.c b/Sources/_CFURLSessionInterface/CFURLSessionInterface.c index 7c1fbfd6e5..81ca4262c8 100644 --- a/Sources/_CFURLSessionInterface/CFURLSessionInterface.c +++ b/Sources/_CFURLSessionInterface/CFURLSessionInterface.c @@ -676,20 +676,21 @@ CFURLSessionCurlVersion CFURLSessionCurlVersionInfo(void) { return v; } -CFURLSessionSSLVersion CFURLSessionSSLVersionInfo(void) { +// Get version info for OpenSSL (not other SSL libraries.) +CFURLSessionOpenSSLVersion * _Nullable CFURLSessionOpenSSLVersionInfo(void) { curl_version_info_data *info = curl_version_info(CURLVERSION_NOW); - CFURLSessionSSLVersion version = {0, 0, 0}; - if (info && info->ssl_version) { - // Parse OpenSSL version string like "OpenSSL/1.0.2k-fips" or "OpenSSL/1.1.1" const char *ssl_str = info->ssl_version; if (strncmp(ssl_str, "OpenSSL/", 8) == 0) { + // Parse OpenSSL version string like "OpenSSL/1.0.2k-fips" or "OpenSSL/1.1.1" + static CFURLSessionOpenSSLVersion version = {0, 0, 0}; ssl_str += 8; // Skip "OpenSSL/" sscanf(ssl_str, "%d.%d.%d", &version.major, &version.minor, &version.patch); + return &version; } } - return version; + return NULL; } diff --git a/Sources/_CFURLSessionInterface/include/CFURLSessionInterface.h b/Sources/_CFURLSessionInterface/include/CFURLSessionInterface.h index 201168ff2e..ecdc658620 100644 --- a/Sources/_CFURLSessionInterface/include/CFURLSessionInterface.h +++ b/Sources/_CFURLSessionInterface/include/CFURLSessionInterface.h @@ -580,12 +580,12 @@ typedef struct CFURLSessionCurlVersion { } CFURLSessionCurlVersion; CF_EXPORT CFURLSessionCurlVersion CFURLSessionCurlVersionInfo(void); -typedef struct CFURLSessionSSLVersion { +typedef struct CFURLSessionOpenSSLVersion { int major; int minor; int patch; -} CFURLSessionSSLVersion; -CF_EXPORT CFURLSessionSSLVersion CFURLSessionSSLVersionInfo(void); +} CFURLSessionOpenSSLVersion; +CF_EXPORT CFURLSessionOpenSSLVersion * _Nullable CFURLSessionOpenSSLVersionInfo(void); CF_EXPORT int const CFURLSessionWriteFuncPause;