Skip to content

Conversation

@jrflat
Copy link
Contributor

@jrflat jrflat commented Jul 18, 2024

We've seen various crashes in libcrypto that occur following URLSession._MultiHandle.deinit. The crash was particularly frequent on Amazon Linux 2 (see swiftlang/swift-package-manager#7624), and generally follows a similar pattern to this backtrace:

 0      0x0000ffffa4e4b948 getrn + 136 in libcrypto.so.1.0.2k
 1 [ra] 0x0000ffffa4e4bddc lh_delete + 55 in libcrypto.so.1.0.2k
 2 [ra] 0x0000ffffa4e4eaac int_thread_del_item + 123 in libcrypto.so.1.0.2k
 3 [ra] 0x0000ffffa4e4f564 ERR_error_string + 151 in libcrypto.so.1.0.2k
 4 [ra] 0x0000ffffa5316d68 Curl_close + 135 in libcurl.so.4.8.0
 5 [ra] 0x0000ffffa52c2374 Curl_conncache_close_all_connections + 371 in libcurl.so.4.8.0
 6 [ra] 0x0000ffffa52f8884 curl_multi_cleanup + 215 in libcurl.so.4.8.0
 7 [ra] 0x0000ffffa7b893b0 URLSession._MultiHandle.deinit + 255 in libFoundationNetworking.so

curl can be configured with various TLS libraries, and OpenSSL is a common choice. Previously, each URLSession had its own work queue, but given that "OpenSSL is not completely thread-safe, and unfortunately not all global resources have the necessary locks" (source), and URLSession does not account for this, this sometimes led to the above crash in multi-threaded contexts.

It appears that certain versions of OpenSSL might be more or less susceptible to this crash, but given that curl can be built with a wide array of versions, or even different TLS libraries, the only effective fix we found was to make URLSession single-threaded by sharing a target queue for all sessions. This is similar to the loader queuing behavior on Darwin.

Thanks to @guoye-zhang, @AnkshitJain, and @bnbarham for help debugging/discovering this fix!

@jrflat
Copy link
Contributor Author

jrflat commented Jul 18, 2024

@swift-ci please test

@parkera parkera merged commit 2e87e5e into swiftlang:main Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants