Skip to content

gh-160217: Fix premature client socket cleanup in test_socket - #160218

Open
NikitaMikhailov wants to merge 2 commits into
python:mainfrom
NikitaMikhailov:codex/socket-client-cleanup-race
Open

NikitaMikhailov wants to merge 2 commits into
python:mainfrom
NikitaMikhailov:codex/socket-client-cleanup-race

Conversation

@NikitaMikhailov

Copy link
Copy Markdown

Fixes gh-160217.

NetworkConnectionAttributesTest registers client socket close callbacks on the shared TestCase cleanup stack. The server thread can run these callbacks before it reaches done.wait, closing the socket while the client is still checking it. This caused testSourceAddress to fail with EBADF in the MSan job for gh-159859.

Remove the five redundant client socket close callbacks. clientTearDown() already closes the socket in the client thread, and clientRun() always calls it in a finally block. All address and timeout assertions remain in place.

Validation:

  • A deterministic reproducer pauses the client's getsockname() until the server reaches done.wait. It fails with EBADF before the change and passes after it.
  • The complete NetworkConnectionAttributesTest class passes 100 consecutive runs (600 tests).
  • The full test_socket suite passes: 753 tests run, 262 skipped on macOS.
  • test_socket -m NetworkConnectionAttributesTest -R 3:3 passes with no reference or file descriptor leaks in the measured repetitions.
  • pre-commit checks pass for Lib/test/test_socket.py.

This changes only tests, so no NEWS entry is needed.

AI assistance: Codex helped diagnose the race, prepare the change and deterministic reproducer, and run validation.

@bedevere-app

bedevere-app Bot commented Oct 22, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app

bedevere-app Bot commented Nov 3, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

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

Labels

awaiting review tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_socket: client socket cleanups race with NetworkConnectionAttributesTest client thread

1 participant