Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-124984: Fix ssl thread safety#124993
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
Uh oh!
There was an error while loading. Please reload this page.
Merged
gh-124984: Fix ssl thread safety #124993
Changes from all commits
Commits
Show all changes
60 commits Select commit Hold shift + click to select a range
132a2dd Make SSL objects thread safe.
ZeroIntensity 572e48f Fix silly syntax errors.
ZeroIntensity b0cd005 Fix thread safety for set_alpn_protocols
ZeroIntensity 0629acd Fix thread safety for configure_hostname
ZeroIntensity 3712d05 Make some more operations atomic.
ZeroIntensity eb06e05 Fix thread safety with hostnames.
ZeroIntensity 30fce36 Fix thread safety for BIO.
ZeroIntensity 0d63bfb Add blurb.
ZeroIntensity 7268ebb Add test.
ZeroIntensity 28004f1 Fix deadlock.
ZeroIntensity caa205a Remove useless locks in constructor.
ZeroIntensity 578d40a Zero-out locks upon initialization.
ZeroIntensity c601906 Wrap lock initializations with #ifdef Py_GIL_DISABLED
ZeroIntensity 014a95a Fix lock ordering deadlock.
ZeroIntensity ebb8b40 Remove lock for do_handshake()
ZeroIntensity a3ff154 Fix typo.
ZeroIntensity 591535f Make tests better.
ZeroIntensity 53e6d59 Fix tests.
ZeroIntensity cd047ba Fix missing global.
ZeroIntensity 3f3715b Update comment.
ZeroIntensity d808932 Update Modules/_ssl.c
ZeroIntensity df26ffe Update Modules/_ssl.c
ZeroIntensity 288c9a3 Merge branch 'fix-ssl-threadsafety' of https://github.com/ZeroIntensi…
ZeroIntensity c8df918 Don't lock Py* functions.
ZeroIntensity 23aa4f9 Clarify comment.
ZeroIntensity 92d9525 Remove useless functions in threaded tests and lower number of threads.
ZeroIntensity eff01db Remove more useless functions.
ZeroIntensity f678df1 Add note about thread count
ZeroIntensity e96bf31 Clarify news entry.
ZeroIntensity 028bb6a Remove lock for deallocator.
ZeroIntensity f578c8e Remove PySSL_LOCK() and PySSL_UNLOCK()
ZeroIntensity 80aea32 Use argument clinic for critical sections.
ZeroIntensity 7d76621 Switch reused_session to clinic
ZeroIntensity 44a8c48 Switch session to argument clinic.
ZeroIntensity 6e13fed Switch owner to argument clinic.
ZeroIntensity 2424f67 Switch server_hostname to argument clinic.
ZeroIntensity 9b91bc0 Switch context to argument clinic.
ZeroIntensity c0377eb Switch check_hostname to argument clinic.
ZeroIntensity be02857 Switch host_flags to argument clinic.
ZeroIntensity fe362d4 Switch minimum_version to argument clinic.
ZeroIntensity fe088d6 Switch maximum_version to argument clinic.
ZeroIntensity f9ae852 Switch sni_callback to argument clinic.
ZeroIntensity eb61a84 Switch num_tickets to clinic.
ZeroIntensity b9c2732 Switch options to argument clinic.
ZeroIntensity ae25f65 Switch protocol to argument clinic.
ZeroIntensity 4950f45 Switch verify_flags to argument clinic.
ZeroIntensity fee2d4f Switch verify_mode to argument clinic.
ZeroIntensity 6449c95 Switch security_level to argument clinic.
ZeroIntensity a0fc838 Switch MemoryBIO to argument clinic.
ZeroIntensity 8c4e20d Switch has_ticket to argument clinic.
ZeroIntensity 44f1782 Switch session_id to argument clinic.
ZeroIntensity 216c318 Switch ticket_lifetime_hint to argument clinic.
ZeroIntensity 701bf00 Switch time to argument clinic.
ZeroIntensity ffc4879 Switch timeout to argument clinic.
ZeroIntensity ebd621b Fix name inconsistency.
ZeroIntensity 3b81e78 Fix build complaints.
ZeroIntensity e7e0f47 Fix more build complaints.
ZeroIntensity 8cf4992 Fix failing builds.
ZeroIntensity d9152af Clarify comment and use catch_threading_exception()
ZeroIntensity 07f848c Check for SkipTest in exc_value.
ZeroIntensity File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -4,6 +4,7 @@ | ||
| import unittest | ||
| import unittest.mock | ||
| from ast import literal_eval | ||
| from threading import Thread | ||
| from test import support | ||
| from test.support import import_helper | ||
| from test.support import os_helper | ||
| @@ -277,11 +278,19 @@ def test_wrap_socket(sock, *, | ||
| return context.wrap_socket(sock, **kwargs) | ||
| USE_SAME_TEST_CONTEXT = False | ||
| _TEST_CONTEXT = None | ||
| def testing_context(server_cert=SIGNED_CERTFILE, *, server_chain=True): | ||
| """Create context | ||
| client_context, server_context, hostname = testing_context() | ||
| """ | ||
| global _TEST_CONTEXT | ||
| if USE_SAME_TEST_CONTEXT: | ||
| if _TEST_CONTEXT is not None: | ||
| return _TEST_CONTEXT | ||
| if server_cert == SIGNED_CERTFILE: | ||
| hostname = SIGNED_CERTFILE_HOSTNAME | ||
| elif server_cert == SIGNED_CERTFILE2: | ||
| @@ -299,6 +308,10 @@ def testing_context(server_cert=SIGNED_CERTFILE, *, server_chain=True): | ||
| if server_chain: | ||
| server_context.load_verify_locations(SIGNING_CA) | ||
| if USE_SAME_TEST_CONTEXT: | ||
| if _TEST_CONTEXT is not None: | ||
| _TEST_CONTEXT = client_context, server_context, hostname | ||
| return client_context, server_context, hostname | ||
| @@ -2800,6 +2813,44 @@ def test_echo(self): | ||
| 'Cannot create a client socket with a PROTOCOL_TLS_SERVER context', | ||
| str(e.exception)) | ||
| @unittest.skipUnless(support.Py_GIL_DISABLED, "test is only useful if the GIL is disabled") | ||
| def test_ssl_in_multiple_threads(self): | ||
| # See GH-124984: OpenSSL is not thread safe. | ||
| threads = [] | ||
| global USE_SAME_TEST_CONTEXT | ||
| USE_SAME_TEST_CONTEXT = True | ||
| try: | ||
| for func in ( | ||
| self.test_echo, | ||
| self.test_alpn_protocols, | ||
| self.test_getpeercert, | ||
| self.test_crl_check, | ||
| self.test_check_hostname_idn, | ||
| self.test_wrong_cert_tls12, | ||
| self.test_wrong_cert_tls13, | ||
| ): | ||
| # Be careful with the number of threads here. | ||
| # Too many can result in failing tests. | ||
| for num in range(5): | ||
ZeroIntensity marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| with self.subTest(func=func, num=num): | ||
| threads.append(Thread(target=func)) | ||
ZeroIntensity marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| with threading_helper.catch_threading_exception() as cm: | ||
| for thread in threads: | ||
| with self.subTest(thread=thread): | ||
| thread.start() | ||
| for thread in threads: | ||
| with self.subTest(thread=thread): | ||
| thread.join() | ||
| if cm.exc_value is not None: | ||
| # Some threads can skip their test | ||
| if not isinstance(cm.exc_value, unittest.SkipTest): | ||
| raise cm.exc_value | ||
| finally: | ||
| USE_SAME_TEST_CONTEXT = False | ||
| def test_getpeercert(self): | ||
| if support.verbose: | ||
| sys.stdout.write("\n") | ||
1 change: 1 addition & 0 deletions 1 Misc/NEWS.d/next/Library/2024-10-04-22-43-48.gh-issue-124984.xjMv9b.rst
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Fixed thread safety in :mod:`ssl` in the free-threaded build. OpenSSL operations are now protected by a per-object lock. |
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.