Skip to content

Use a shared queue for URLSessions to prevent crash during _MultiHandle.deinit #5016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 19, 2024

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
2 of 3 checks passed
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