Skip to content

gh-150191: Avoid data race in test_sni_callback_race#150193

Open
colesbury wants to merge 1 commit into
python:mainfrom
colesbury:gh-150191-fix-test-sni-callback-race
Open

gh-150191: Avoid data race in test_sni_callback_race#150193
colesbury wants to merge 1 commit into
python:mainfrom
colesbury:gh-150191-fix-test-sni-callback-race

Conversation

@colesbury
Copy link
Copy Markdown
Contributor

@colesbury colesbury commented May 21, 2026

Use a single handshake thread to avoid OpenSSL-internal data races on shared SSL_CTX state, while keeping multiple togglers to exercise the sni_callback setter race from gh-149816.

Use a single handshake thread to avoid OpenSSL-internal data races on
shared SSL_CTX state, while keeping multiple togglers to exercise the
sni_callback setter race from pythongh-149816.
@colesbury colesbury requested review from encukou and mpage May 21, 2026 16:26
@bedevere-app bedevere-app Bot added the tests Tests in the Lib/test dir label May 21, 2026
@colesbury colesbury added skip news needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels May 21, 2026
@colesbury
Copy link
Copy Markdown
Contributor Author

A few other related changes:

  • Run the test in the GIL-enabled build as well. "test is only useful if the GIL is disabled" is wrong. The data race reported exists in the GIL-enabled build as well. In general, we should write these tests so that they run in both the GIL-enabled and free-threaded build.
  • Use threading_helper.run_concurrently, which sets up the needed exception tracking and uses a barrier, which helps catch data races by making it more likely that threads run at the same time.
  • Run the threads for 0.1 seconds. Previously "done.set()" was called immediately so that the threads barely had time to run, if at all. (Alternatively, we could run them for a fixed number of iterations)

@colesbury colesbury marked this pull request as ready for review May 21, 2026 16:30
@colesbury colesbury requested review from gpshead and picnixz as code owners May 21, 2026 16:30
@kiri11
Copy link
Copy Markdown
Contributor

kiri11 commented May 21, 2026

@colesbury Thanks for the test improvements!

  • Run the threads for 0.1 seconds. Previously "done.set()" was called immediately so that the threads barely had time to run, if at all. (Alternatively, we could run them for a fixed number of iterations)

Running it for at least 0.1s does dramatically increase the consistency, although I've been told we shouldn't add static delays to the tests, even as small as 0.1s.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants