Skip to content

gh-158646: Fix race in test_shutdown_corrupted_ssl_sends_close_notify - #160848

Merged
asvetlov merged 3 commits into
python:mainfrom
asvetlov:claude/fix-gh-158646-98ccc0
Oct 29, 2026
Merged

asvetlov merged 3 commits into
python:mainfrom
asvetlov:claude/fix-gh-158646-98ccc0

Conversation

@asvetlov

@asvetlov asvetlov commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

The server side of test_shutdown_corrupted_ssl_sends_close_notify called unwrap() right after corrupting the client's TLS stream, which sent its own close_notify to a client that was about to close its socket. That turned the client's close into a TCP reset. Linux still delivers data queued before a reset, but Windows discards it, so the server's read failed with WinError 10054 (sometimes 10053) even though the client had sent close_notify correctly. The asyncio code is fine; the test was racy.

Fix: the server now waits for the client's close_notify with recv() (which returns b'' on a clean TLS EOF) before calling unwrap(). A ragged EOF without close_notify still makes unwrap() raise SSLError, so the test keeps catching the gh-98078 regression (verified by temporarily removing the gh-98078 send in sslproto.py).

Verified on a Windows 11 x64 debug build:

Run Result
Unmodified test, 100 iterations × Selector + Proactor 16 server-thread exceptions, 1 unittest failure
Fixed test, 300 iterations × Selector + Proactor 0 failures
Full test_asyncio.test_sslproto on Windows and Linux pass

🤖 Generated with Claude Code

…notify

The server called unwrap() right after corrupting the client's TLS
stream, which sent its own close_notify to a client that was about to
close its socket.  That turned the client's close into a TCP reset, and
on Windows a reset discards the client's close_notify that is still
unread, so the server failed with WinError 10054 (or 10053).

Wait for the client's close_notify with recv() before calling unwrap().
A ragged EOF without close_notify still makes unwrap() raise SSLError,
so the test keeps catching the pythongh-98078 regression.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
@asvetlov asvetlov added needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes labels Oct 28, 2026
@asvetlov

Copy link
Copy Markdown
Contributor Author

@vstinner I believe this tiny change fixes the flaky test on Windows.
Maybe SSL support still has some hidden problems, but the stable test suite is the first priority.

@asvetlov

asvetlov commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor Author

ASan/TSan failures are not related to the PR (only sock.recv() was added to test).

Restarting.

@asvetlov
asvetlov merged commit e5fad04 into python:main Oct 29, 2026
97 of 99 checks passed
@miss-islington-app

Copy link
Copy Markdown

Thanks @asvetlov for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14, 3.15.
🐍🍒⛏🤖

@miss-islington-app

Copy link
Copy Markdown

Sorry, @asvetlov, I could not cleanly backport this to 3.15 due to a conflict.

Please backport manually with cherry_picker, see the devguide for more information.

cherry_picker e5fad0433fc76cf8a500668cc1ae9f2fd2d6988f 3.15

@miss-islington-app

Copy link
Copy Markdown

Sorry, @asvetlov, I could not cleanly backport this to 3.14 due to a conflict.

Please backport manually with cherry_picker, see the devguide for more information.

cherry_picker e5fad0433fc76cf8a500668cc1ae9f2fd2d6988f 3.14

@asvetlov asvetlov removed needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes labels Oct 29, 2026
@asvetlov

Copy link
Copy Markdown
Contributor Author

The test doesn't exist on 3.15, don't backport

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

Labels

tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant