|
| 1 | +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Ludovic Henry <git@ludovic.dev> |
| 3 | +Date: Fri, 25 Sep 2026 00:00:00 +0000 |
| 4 | +Subject: [PATCH] test: let the stream be freed before stopping the local |
| 5 | + server on free-threaded builds |
| 6 | + |
| 7 | +TestClient and TestAsyncClient's _test_stream_lives_until_complete drop |
| 8 | +every local reference to the stream and the connection, wait for the |
| 9 | +response, then call _stop_server(). That calls HTTPServer.shutdown(), and |
| 10 | +the single-threaded HTTPServer is still inside handle() for the HTTP/1.1 |
| 11 | +keep-alive connection. So shutdown() only returns once the client socket |
| 12 | +closes, which happens only when the Python stream (which holds the |
| 13 | +connection) is deallocated. |
| 14 | + |
| 15 | +With the GIL, the stream's last reference is dropped by the DECREFs in |
| 16 | +s_on_stream_complete on the native event-loop thread, and the stream, |
| 17 | +connection and socket are freed right there. On a free-threaded build, an |
| 18 | +object's owner thread is the one that created it (here the test's main |
| 19 | +thread). A DECREF from any other thread that takes the shared refcount |
| 20 | +negative only queues the object for its owner to merge. The owner merges |
| 21 | +the next time it runs bytecode, i.e. when the eval breaker fires. If the main |
| 22 | +thread is already parked in shutdown()'s Event.wait() by then, nothing ever |
| 23 | +merges the refcount. The stream never dies, the socket never closes and |
| 24 | +the job hangs until the CI timeout. Whether it hangs depends on whether |
| 25 | +the main thread gets from the completion wake-up into shutdown() before |
| 26 | +the native thread's final DECREF. |
| 27 | + |
| 28 | +That is what happened on this repo's riscv64 runners. 0.36.4 cp314t hung in |
| 29 | +test_http_client's test_stream_lives_until_complete_http, and on a |
| 30 | +later run 0.36.3 cp314t hung in test_aiohttp_client's. Each version |
| 31 | +passes on other runs, and test/ is byte-identical across 0.36.3, 0.36.4 |
| 32 | +and 0.37.0. It is not specific to riscv64: with upstream's own |
| 33 | +awscrt 0.36.3 cp314-cp314t x86_64 wheel from PyPI, a reproducer that |
| 34 | +reaches shutdown() before the response arrives hangs 3/3 on 3.14.7t, |
| 35 | +while cp314 returns in 0.5s 3/3. |
| 36 | + |
| 37 | +Keep a weakref to the stream and, after the completion check, run |
| 38 | +bytecode until it is freed (bounded by the test's own timeout) before |
| 39 | +stopping the server. Nothing else changes: the tests still check that the |
| 40 | +stream completes after every local reference is gone, and now also |
| 41 | +wait for it to be released instead of relying on the GIL's immediate |
| 42 | +deallocation. |
| 43 | + |
| 44 | +Upstream-Status: To upstream [latent free-threading deadlock in upstream's own test harness, reproduced on x86_64 with upstream's PyPI wheel; not yet sent, since that needs a contributor-signed upstream PR, which this repo's automation does not open] |
| 45 | + |
| 46 | +Signed-off-by: Ludovic Henry <git@ludovic.dev> |
| 47 | +--- |
| 48 | +diff --git a/test/test_aiohttp_client.py b/test/test_aiohttp_client.py |
| 49 | +index 8d24c39..57facfb 100644 |
| 50 | +--- a/test/test_aiohttp_client.py |
| 51 | ++++ b/test/test_aiohttp_client.py |
| 52 | +@@ -3,6 +3,7 @@ |
| 53 | + |
| 54 | + import time |
| 55 | + import socket |
| 56 | ++import weakref |
| 57 | + import sys |
| 58 | + import asyncio |
| 59 | + import unittest |
| 60 | +@@ -235,6 +236,8 @@ class TestAsyncClient(AsyncLocalServerTestBase): |
| 61 | + # Schedule task to collect response but don't await it yet |
| 62 | + collect_task = asyncio.create_task(response.collect_response(stream)) |
| 63 | + |
| 64 | ++ stream_ref = weakref.ref(stream) |
| 65 | ++ |
| 66 | + # Delete references to stream and connection |
| 67 | + del stream |
| 68 | + del connection |
| 69 | +@@ -243,6 +246,14 @@ class TestAsyncClient(AsyncLocalServerTestBase): |
| 70 | + status_code = await collect_task |
| 71 | + self.assertEqual(200, status_code) |
| 72 | + |
| 73 | ++ # On free-threaded builds the stream's last reference is dropped on the |
| 74 | ++ # native thread, and CPython defers freeing it to this (owning) thread. |
| 75 | ++ # Run bytecode until that happens: the stream keeps the keep-alive |
| 76 | ++ # connection open, and _stop_server() would otherwise block forever. |
| 77 | ++ deadline = time.monotonic() + self.timeout |
| 78 | ++ while stream_ref() is not None and time.monotonic() < deadline: |
| 79 | ++ await asyncio.sleep(0.01) |
| 80 | ++ |
| 81 | + finally: |
| 82 | + self._stop_server() |
| 83 | + |
| 84 | +diff --git a/test/test_http_client.py b/test/test_http_client.py |
| 85 | +index 5364d7e..b2733d4 100644 |
| 86 | +--- a/test/test_http_client.py |
| 87 | ++++ b/test/test_http_client.py |
| 88 | +@@ -3,6 +3,7 @@ |
| 89 | + |
| 90 | + import time |
| 91 | + import socket |
| 92 | ++import weakref |
| 93 | + import sys |
| 94 | + import subprocess |
| 95 | + from urllib.parse import urlparse |
| 96 | +@@ -267,6 +268,7 @@ class TestClient(LocalServerTestBase): |
| 97 | + stream = connection.request(request) |
| 98 | + stream.activate() |
| 99 | + completion_future = stream.completion_future |
| 100 | ++ stream_ref = weakref.ref(stream) |
| 101 | + |
| 102 | + # delete all local references |
| 103 | + del stream |
| 104 | +@@ -275,6 +277,14 @@ class TestClient(LocalServerTestBase): |
| 105 | + # stream should still complete successfully |
| 106 | + completion_future.result(self.timeout) |
| 107 | + |
| 108 | ++ # On free-threaded builds the stream's last reference is dropped on the |
| 109 | ++ # native thread, and CPython defers freeing it to this (owning) thread. |
| 110 | ++ # Run bytecode until that happens: the stream keeps the keep-alive |
| 111 | ++ # connection open, and _stop_server() would otherwise block forever. |
| 112 | ++ deadline = time.monotonic() + self.timeout |
| 113 | ++ while stream_ref() is not None and time.monotonic() < deadline: |
| 114 | ++ time.sleep(0.01) |
| 115 | ++ |
| 116 | + finally: |
| 117 | + self._stop_server() |
| 118 | + |
0 commit comments