Repository navigation
test_socket: client socket cleanups race with NetworkConnectionAttributesTest client thread #160217
Description
Bug description
NetworkConnectionAttributesTest registers client socket close() calls through self.addCleanup() from the client thread. TestCase runs those callbacks in the server thread. They can run before the client thread has finished checking the socket, causing a spurious OSError: [Errno 9] Bad file descriptor in testSourceAddress.
Observed in the MSan CI job for gh-159859:
https://github.com/python/cpython/actions/runs/39374885370/job/118470772770
The failure is in _testSourceAddress, at self.cli.getsockname()[1]. The email tests in that job pass.
ThreadableTest._setUp() registers self.done.wait as a cleanup. A later self.addCleanup(self.cli.close) from the client is popped first by the server thread's LIFO cleanup stack, so it can close the socket before waiting for the client to finish. The client already closes its socket in clientTearDown(), which runs in clientRun()'s finally block, so these additional callbacks are redundant.
The same cross-thread cleanup registration occurs in _testFamily, _testTimeoutDefault, _testTimeoutNone, and _testTimeoutValueNonamed.
The problem can be made deterministic by pausing the client's getsockname() until the server reaches the done.wait cleanup. Before that wait, the server has already run the socket's close callback. Removing the redundant callbacks allows the client to finish its assertions and close its own socket before setting done.
Deterministic reproducer
Run this script with the built CPython interpreter from a source checkout. It uses a real loopback TCP connection and events to control the interleaving.
"""Force server cleanup to run before the client's getsockname check."""
import socket
import sys
import threading
import unittest
from unittest import mock
from test.test_socket import NetworkConnectionAttributesTest
class ControlledSourceAddressTest(NetworkConnectionAttributesTest):
def __init__(self, methodName='testSourceAddress'):
self.allow_check = threading.Event()
self.check_started = threading.Event()
super().__init__(methodName)
def testSourceAddress(self):
super().testSourceAddress()
if not self.check_started.wait(10):
raise AssertionError('client did not reach getsockname')
def _callCleanup(self, function, /, *args, **kwargs):
# Client socket cleanups, if registered, run before done.wait.
# Release getsockname only when those cleanups have completed.
if function == self.done.wait:
self.allow_check.set()
return super()._callCleanup(function, *args, **kwargs)
class DelayedSocket:
def __init__(self, sock, gate, started):
self.sock = sock
self.gate = gate
self.started = started
def __getattr__(self, name):
return getattr(self.sock, name)
def getsockname(self):
self.started.set()
if not self.gate.wait(10):
raise AssertionError('server cleanup did not reach done.wait')
return self.sock.getsockname()
case = ControlledSourceAddressTest()
real_create_connection = socket.create_connection
def create_connection(*args, **kwargs):
return DelayedSocket(real_create_connection(*args, **kwargs),
case.allow_check, case.check_started)
with mock.patch('socket.create_connection', side_effect=create_connection):
result = unittest.TextTestRunner(verbosity=2).run(unittest.TestSuite([case]))
sys.exit(not result.wasSuccessful())CPython versions tested on
CPython main branch, macOS arm64. The reported CI failure occurred on Linux under MSan.
AI assistance: Codex helped investigate the CI failure, prepare a deterministic reproducer, and test a fix.
Activity