GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-143756: Fix potential data race in SSLContext.load_cert_chain by colesbury · Pull Request #143818 · python/cpython

Conversation

colesbury commented Jan 13, 2026 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Concurrent calls to load_cert_chain caused data races in OpenSSL code.

Issue: Data races in OpenSSL bindings #143756

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          pythongh-143756: Fix potential data race in SSLContext.load_cert_chain

4109a33

Concurrent calls to `load_cert_chain` caused data races in OpenSSL code.

bedevere-app Bot mentioned this pull request

Jan 13, 2026

Data races in OpenSSL bindings #143756

Open


          Update clinic file

46d033a

colesbury marked this pull request as ready for review

January 13, 2026 22:06

colesbury requested review from gpshead and picnixz as code owners

January 13, 2026 22:06

bedevere-app Bot added the awaiting core review label

Jan 13, 2026

colesbury requested a review from ZeroIntensity

January 13, 2026 22:06

colesbury added the topic-SSL label

Jan 13, 2026

picnixz reviewed

Jan 19, 2026

View reviewed changes

picnixz left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Do we need tests for that actually?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Modules/_ssl.c

		do { (save) = PyEval_SaveThread(); } while(0)
		#define PySSL_END_ALLOW_THREADS_S(save) \
		do { PyEval_RestoreThread(save); _PySSL_FIX_ERRNO; } while(0)
		#define PySSL_BEGIN_ALLOW_THREADS(self) { \

picnixz Jan 19, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

While you are it, can you use a do-while construct? (here we would just have the do {) In addition can you make it multiline with line continuations aligned on a tab multiple? (like PEP-7) TiA.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

colesbury Jan 20, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

ditto

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Modules/_ssl.c

		do { (save) = PyEval_SaveThread(); PyMutex_Lock(mutex); } while(0)
		#define PySSL_END_ALLOW_THREADS_S(save, mutex) \
		do { PyMutex_Unlock(mutex); PyEval_RestoreThread(save); _PySSL_FIX_ERRNO; } while(0)
		#define PySSL_BEGIN_ALLOW_THREADS_S(save) \

picnixz Jan 19, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can you use uppercase letters for the macro parameters please?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

colesbury Jan 20, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Let's not do a bunch of extra refactoring in a bugfix PR

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

picnixz Jan 20, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Oh right you want to backport this. I think there will be anyway conflicts so yeah let's not make it more complicate

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Modules/_ssl.c

Comment on lines +4634 to +4635

		if (keyfile == Py_None)
		keyfile = NULL;

picnixz Jan 19, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think this can be added in the if (keyfile) check instead.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

colesbury Jan 20, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

ditto; this is existing code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Modules/_ssl.c

		if (password != Py_None) {
		if (PyCallable_Check(password)) {
		pw_info.callable = password;
		} else if (!_pwinfo_set(&pw_info, password,

picnixz Jan 19, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change

	} else if (!_pwinfo_set(&pw_info, password,
	}
	else if (!_pwinfo_set(&pw_info, password,

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

colesbury Jan 20, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

ditto

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Modules/_ssl.c

		if (PyCallable_Check(password)) {
		pw_info.callable = password;
		} else if (!_pwinfo_set(&pw_info, password,
		"password should be a string or callable")) {

picnixz Jan 19, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

And realign this adter putting the elseif on its own line

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

colesbury Jan 20, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

ditto

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Modules/_ssl.c

		if (!PyUnicode_FSConverter(certfile, &certfile_bytes)) {
		if (PyErr_ExceptionMatches(PyExc_TypeError)) {
		PyErr_SetString(PyExc_TypeError,
		"certfile should be a valid filesystem path");

picnixz Jan 19, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

For a follow-up: This is a bit misleading. For me such message usually implies a ValueError instead of TypeError. Do we use this formulation for other occurrences of FSConverter?

Maybe: "expecting a path-like object for 'certfile', got ..."

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

colesbury Jan 20, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

ditto

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

colesbury commented Jan 20, 2026

Copy link

Copy Markdown

Contributor Author

Tests already exist. #143752 runs it with TSan

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz approved these changes

Jan 20, 2026

View reviewed changes

picnixz left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry as I was on mobile I didn't see that it was just moving code around. We can refactor this later (I really want to split ssl.c because it is messy to maintain but it is also annoying to create more files...)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-app Bot added awaiting merge and removed awaiting core review labels

Jan 20, 2026

colesbury merged commit bcf9cb0 into python:main

Jan 22, 2026

55 checks passed

Uh oh!

There was an error while loading. Please reload this page.

colesbury deleted the gh-143756-load_cert_chain branch

January 22, 2026 19:02

bedevere-app Bot removed the awaiting merge label

Jan 22, 2026

thunder-coding pushed a commit to thunder-coding/cpython that referenced this pull request

Feb 15, 2026


          pythongh-143756: Fix potential data race in SSLContext.load_cert_chain (

d74e5cd

pythongh-143818)

Concurrent calls to `load_cert_chain` caused data races in OpenSSL code.

kumaraditya303 added a commit to kumaraditya303/cpython that referenced this pull request

Jul 10, 2026


          pythongh-143756: skip test_ssl_in_multiple_threads under TSAN

c173e0d

Concurrent calls to SSLContext.load_cert_chain on the same context
race on the SSL_CTX default password callback. This is fixed in 3.15
(pythonGH-143818), but under TSAN with instrumented OpenSSL the race is now
reported on 3.14, so skip the test that triggers it.

kumaraditya303 added a commit to kumaraditya303/cpython that referenced this pull request

Jul 10, 2026


          pythongh-143756: skip test_load_cert_chain_thread_safety under TSAN

76ddcae

This test spawns 8 threads calling load_cert_chain on the same
context, directly triggering the data race on the SSL_CTX default
password callback (fixed in 3.15 by pythonGH-143818). It was the test
that failed both TSan CI jobs.

kumaraditya303 added a commit that referenced this pull request

Jul 10, 2026


          [3.14] gh-143750: Compile OpenSSL with TSan for TSan CI (#153316) (#1…

b49ec81

…53355)

Compile OpenSSL with TSan in the TSan CI job so that data races between
Python code and OpenSSL internals are visible to the sanitizer.

Also skip test_ssl_in_multiple_threads under TSan: concurrent calls to
SSLContext.load_cert_chain on the same context race on the SSL_CTX
default password callback. The race is fixed in 3.15+ (GH-143818), but
with TSan-instrumented OpenSSL it is now reported on 3.14.

(cherry picked from commit fc19ad7)

Co-authored-by: Sam Gross <[email protected]>

nascheme mentioned this pull request

Jul 14, 2026

Backport free-threading memory-safety fixes to 3.14 and 3.15 #153714

Closed

Reviewers

picnixz

picnixz approved these changes

gpshead

Awaiting requested review from gpshead

gpshead is a code owner

ZeroIntensity

Awaiting requested review from ZeroIntensity

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

topic-SSL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

colesbury commented Jan 13, 2026 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

colesbury commented Jan 20, 2026

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

colesbury commented Jan 13, 2026 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.