gh-124984: Fix `ssl` thread safety by ZeroIntensity · Pull Request #124993 · python/cpython · GitHub

As it turns out, OpenSSL doesn't like being called in multiple threads. This adds a per-socket (and per-context and per-session) lock for all OpenSSL calls.

Issue: Segfault on Python 3.13.0rc3 (free-threaded) with requests #124984

Thanks, that's a lot of locks.

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Remaining question: should we backport this enhancement (bugfix?) to Python 3.13?

Remaining question: should we backport this enhancement (bugfix?) to Python 3.13?

Yeah, I had the same thought. I'll leave that decision to @Yhg1s

Backport this to 3.13? Uuhm...

No.

Without this change, the ssl module is not usable on a Free Threaded build (it does easily crash). The change only affects the Free Threaded build: adding @critical_section does nothing in the regular build, and the added test is ignored on regular Python (only run on Free Threading). I would be fine with no backporting it, since Free Threading remains experimental.

I think that fixing the SSL module crash in the 3.13 free threading build is important -- lots of basic tasks around HTTP requests are likely to crash without it.

I don't think "lines of code changed" is a good measure of the complexity here -- the Modules/_ssl.c changes are mostly mechanical, and the only behavioral changes is additional locking in the free threading build.

If backporting this PR is a nonstarter than we should consider a smaller, more targeted change that only adds @critical_section to SSLContext methods (without changing getters and setters). That seems less ideal to me, but should fix the common SSL crash and be a pretty small code change.

Also, thank you @ZeroIntensity for fixing this bug and @vstinner, @corona10, and everyone else that reviewed the PR.

Thanks @ZeroIntensity for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

Sorry, @ZeroIntensity and @vstinner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 4c53b2577531c77193430cdcd66ad6385fcda81f 3.13

@ZeroIntensity: Automated backport failed. Would you mind to backport the change manually?

With a backport, it might be easier to take a decision on fixing 3.13 or not.

Yeah, I can do it later today.

GH-125780 is a backport of this pull request to the 3.13 branch.

Make SSL objects thread safe in Free Theaded build by using critical sections. (cherry picked from commit 4c53b25) Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

@Yhg1s: Are you still against the backport to 3.13 after @colesbury's comment?

I'm okay with a backport of just the @critical_section changes (since those expand to nothing in the normal build, and the free-threaded build is experimental anyway). It's the larger refactorings that worry me.

It's the larger refactorings that worry me.

Other changes are tests and changes to use the code declared with @critical_section.

There isn't any other refactoring going on here, I just had to switch the getters and setters over to AC for the critical section. Another issue is that not backporting this to 3.13 will also hurt any automatic backports for ssl in the future.

Make SSL objects thread safe in Free Theaded build by using critical sections. (cherry picked from commit 4c53b25) Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Make SSL objects thread safe in Free Theaded build by using critical sections. Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

The symbol _ssl__SSLContext_security_level_set is used here, but nowhere added

This is generated clinic code. I'm pretty sure this route is never taken, and the macro right after this is used instead.

Oh, thank you. I didn't know that

ZeroIntensity added 15 commits October 4, 2024 22:21

Make SSL objects thread safe.

132a2dd

Fix silly syntax errors.

572e48f

Fix thread safety for set_alpn_protocols

b0cd005

Fix thread safety for configure_hostname

0629acd

Make some more operations atomic.

3712d05

Fix thread safety with hostnames.

eb06e05

Fix thread safety for BIO.

30fce36

Add blurb.

0d63bfb

Add test.

7268ebb

Fix deadlock.

28004f1

Remove useless locks in constructor.

caa205a

Zero-out locks upon initialization.

578d40a

Wrap lock initializations with #ifdef Py_GIL_DISABLED

c601906

Fix lock ordering deadlock.

014a95a

Remove lock for do_handshake()

ebb8b40

bedevere-app Bot added the awaiting review label Oct 5, 2024

bedevere-app Bot mentioned this pull request Oct 5, 2024

Segfault on Python 3.13.0rc3 (free-threaded) with requests #124984

Closed

ZeroIntensity requested review from AA-Turner and colesbury October 5, 2024 04:25

ZeroIntensity added topic-SSL topic-free-threading labels Oct 5, 2024

ZeroIntensity added 3 commits October 5, 2024 00:28

Fix typo.

a3ff154

Make tests better.

591535f

Fix tests.

53e6d59

JelleZijlstra reviewed Oct 5, 2024

View reviewed changes

Comment thread Lib/test/test_ssl.py

JelleZijlstra reviewed Oct 5, 2024

View reviewed changes

Comment thread Modules/_ssl.c Outdated

Comment thread Modules/_ssl.c Outdated

Comment thread Modules/_ssl.c Outdated

picnixz reviewed Oct 5, 2024

View reviewed changes

Comment thread Modules/_ssl.c Outdated

Comment thread Modules/_ssl.c Outdated

Comment thread Modules/_ssl.c Outdated

Comment thread Modules/_ssl.c Outdated

ZeroIntensity and others added 3 commits October 5, 2024 10:14

Fix missing global.

cd047ba

Update comment.

3f3715b

Update Modules/_ssl.c

d808932

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

vstinner added the needs backport to 3.13 bugs and security fixes label Oct 21, 2024

miss-islington-app Bot assigned vstinner Oct 21, 2024

ZeroIntensity assigned ZeroIntensity and unassigned vstinner Oct 21, 2024

bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Oct 21, 2024

ZeroIntensity mentioned this pull request Nov 5, 2024

Calling many methods on _ssl._SSLSocket() segfaults #126455

Closed

ZeroIntensity mentioned this pull request Nov 15, 2024

Restore docstrings in ssl #126890

Closed

ZeroIntensity mentioned this pull request Nov 26, 2024

gh-126890: Restore stripped ssl docstrings #127281

Merged

ZeroIntensity mentioned this pull request Jan 17, 2025

gh-128942: make arraymodule.c free-thread safe #128943

Merged

youknowone reviewed Oct 28, 2025

View reviewed changes

youknowone mentioned this pull request Oct 28, 2025

Unexisting function _ssl__SSLContext_security_level_set is referenced in source code #140692

Closed

kumaraditya303 mentioned this pull request Oct 30, 2025

performance regression in ssl module under free-threading #140795

Open

+              #endif
+              #if defined(_SSL__SSLCONTEXT_SECURITY_LEVEL_GETSETDEF)
+              #  undef _SSL__SSLCONTEXT_SECURITY_LEVEL_GETSETDEF
+              #  define _SSL__SSLCONTEXT_SECURITY_LEVEL_GETSETDEF {"security_level", (getter)_ssl__SSLContext_security_level_get, (setter)_ssl__SSLContext_security_level_set, _ssl__SSLContext_security_level_DOCSTR},

gh-124984: Fix `ssl` thread safety by ZeroIntensity · Pull Request #124993 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-124984: Fix ssl thread safety - #124993

gh-124984: Fix ssl thread safety#124993vstinner merged 60 commits intopython:mainpython/cpython:mainfrom ZeroIntensity:fix-ssl-threadsafetyZeroIntensity/cpython:fix-ssl-threadsafetyCopy head branch name to clipboard

Conversation

ZeroIntensity commented Oct 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner commented Oct 19, 2024

Uh oh!

ZeroIntensity commented Oct 19, 2024

Uh oh!

Yhg1s commented Oct 20, 2024

Uh oh!

vstinner commented Oct 20, 2024

Uh oh!

colesbury commented Oct 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

colesbury commented Oct 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miss-islington-app Bot commented Oct 21, 2024

Uh oh!

miss-islington-app Bot commented Oct 21, 2024

Uh oh!

vstinner commented Oct 21, 2024

Uh oh!

ZeroIntensity commented Oct 21, 2024

Uh oh!

bedevere-app Bot commented Oct 21, 2024

Uh oh!

vstinner commented Oct 29, 2024

Uh oh!

Yhg1s commented Nov 4, 2024

Uh oh!

vstinner commented Nov 4, 2024

Uh oh!

ZeroIntensity commented Nov 4, 2024

Uh oh!

youknowone Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

ZeroIntensity Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

youknowone Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

gh-124984: Fix ssl thread safety#124993
vstinner merged 60 commits into
python:mainfrom
ZeroIntensity:fix-ssl-threadsafety

ZeroIntensity commented Oct 5, 2024 •
edited
Loading

colesbury commented Oct 20, 2024 •
edited
Loading

colesbury commented Oct 20, 2024 •
edited
Loading