Repository navigation
Take the context lock in wrap_bio() as wrap_socket() does - #219
Merged
Merged
Conversation
_configure_context() mutates the wrapped ssl.SSLContext, so wrap_socket() holds self._ctx_lock across its __enter__(). wrap_bio() did not, which means async stacks that use wrap_bio() (anyio -> httpcore -> httpx) can mutate one shared context from many threads at once. Adds a deterministic regression test that wrap_bio() configures the context under the lock, plus a threaded stress test over wrap_bio(). Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
bkrabach
marked this pull request as ready for review
September 6, 2026 04:16
sethmlarson
reviewed
Sep 10, 2026
sethmlarson
left a comment
Owner
There was a problem hiding this comment.
Thanks for this contribution and the explanation. Could you add an entry to the CHANGELOG?
sethmlarson
approved these changes
Sep 29, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
_configure_context()mutates the wrappedssl.SSLContext— on Linux it callsctx.set_default_verify_paths(), which writes the context'sX509_STORE.wrap_socket()holdsself._ctx_lockacross the__enter__()for that reason(#192 / #193, released in 0.10.4).
wrap_bio()still does a barewith _configure_context(self._ctx):.That gap matters because
wrap_bio()is the path async stacks take — anyio →httpcore → httpx never reach
wrap_socket()— so the unlocked path is the oneasync clients actually use.
What we saw
An application that mounts a session by resolving ~40 agents' model roles
concurrently (one
asyncio.gather, one HTTPS call each) dies partway throughwith no Python traceback and no result:
Exit 134, sometimes SIGSEGV/139 with the same stack. With
PYTHONFAULTHANDLER=1, every abort captured had the same shape — 20–37 threadsinside
_configure_context, all of them viawrap_bio, none viawrap_socket:truststore 0.10.4, Python 3.12.3, OpenSSL 3.0.13, Linux aarch64, in a container.
Verification
A/B/A in one disposable container, this branch built as a wheel and installed
over the stock 0.10.4 — same probe, same concurrency, only truststore changing:
Every abort carried the signature above: 25–31 threads in
_configure_context,100% via
wrap_bio, 0 viawrap_socket. An earlier independent run of the sameA/B/A got 7 aborts in 15 stock concurrent attempts (and 1 SIGSEGV in 10 stock
sequential), 0 in 20 patched, 2 in 5 after reverting.
The change
Deliberately the same shape
wrap_socket()uses, including holding the lock onlyacross
__enter__()and not for the duration of the wrap — so concurrent wrapsand handshakes are not serialized.
Tests
test_wrap_bio_configures_context_under_lock— deterministic. Patches_configure_contextto record whether_ctx_lockis held while the context isbeing configured, and asserts the lock is released again afterwards. It fails
on
main(recordsFalse) and passes with this change.test_threading_wrap_bio— a threaded hammer overwrap_bio(), in the styleof the existing
test_threading. Worth being honest that this one is onlyprobabilistic: without the lock the failure is a native abort that takes the
whole test process down rather than a failed assert, so the deterministic test
above is the real regression guard.
Full suite passes locally on Linux (
nox -s test, Python 3.12, with mkcert andinternet access): 61 passed, 2 skipped.
Notes
happens, but the lock looks worth having either way.
released, where
check_hostname/verify_modeare still relaxed. This PR doesnot address that one.
whatever fits best.
CI note
The three
pypy3.10jobs fail on this branch exactly as they do onmain(its last three runs, Sep 3–5):cryptographycannot build its wheel there because PyO3 0.29 requires PyPy ≥ 3.11 — the failure is inpip install -r dev-requirements.txt, before any test runs. Every CPython job (3.10–3.14 on all three OSes), Lint, and the pip integration job pass, including the new regression test.