Skip to content

[3.13] gh-134698: Hold a lock when the thread state is detached in ssl (GH-134724) #137126

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 27, 2025

Conversation

gpshead
Copy link
Member

@gpshead gpshead commented Jul 26, 2025

Lock when the thread state is detached.
(cherry picked from commit e047a35) ... or 3.14 backport fd565fd

backport via the pending 3.14 backport #137107

… in `ssl` (pythonGH-134724)

Lock when the thread state is detached.
(cherry picked from commit e047a35)

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Co-authored-by: Gregory P. Smith <greg@krypto.org>
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When backports are not automated I always struggle to see which was automated and which wasn't. I tried to compare it with the 3.14 backport and it seems that the changes are identical. There is one place when I'm unusure about the usage of _Py_SSL_FIX_ERRNO.

I'll ask Peter to have a look as he wrote the original PR.

@@ -3980,7 +3992,8 @@ _password_callback(char *buf, int size, int rwflag, void *userdata)
_PySSLPasswordInfo *pw_info = (_PySSLPasswordInfo*) userdata;
PyObject *fn_ret = NULL;

PySSL_END_ALLOW_THREADS_S(pw_info->thread_state);
pw_info->thread_state = PyThreadState_Swap(pw_info->thread_state);
_PySSL_FIX_ERRNO;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this one is needed because I don't know if the callback mechanism could set an errno that is not propagated so I'm ok to keep it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The former PySSL_END_ALLOW_THREADS_S macro that this replaces included the _PySSL_FIX_ERRNO macro, but I think you're right that it's not needed. I'm not worried about breakage because _PySSL_FIX_ERRNO only applies to Windows debug builds anyway.

Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as well.

@gpshead gpshead merged commit 4a37dd6 into python:3.13 Jul 27, 2025
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-SSL type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants