Skip to content

Commit 0f05a3c

Browse files
Close the async streaming response on the event loop thread (#941)
StreamingResponseSource.close() ran the producer task cancel and response.close() from the parser executor thread. Under TLS that raced the event loop handling a server connection abort and could raise AttributeError from asyncio's SSL shutdown instead of the real StreamFailureError. Schedule the cleanup with call_soon_threadsafe. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent c0bf7d0 commit 0f05a3c

3 files changed

Lines changed: 64 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
- The native streaming response buffer again detects mid-stream server exceptions proactively. Its in-band exception scan built the markers as `__exception__<tag>` and `<tag>__exception__`, but the server separates `__exception__` from the tag with a CRLF on both markers (`__exception__\r\n<tag>` ... `<tag>\r\n__exception__`), so the scan never matched and the exception block was only recovered by the last-chunk fallback in `NativeTransform.parse_response`. When the block spanned a transport-chunk boundary that fallback saw just a fragment and surfaced a truncated or garbled error instead of the real ClickHouse exception. Both the pure Python and compiled Cython buffers are corrected. Closes [#915](https://github.com/ClickHouse/clickhouse-connect/issues/915).
2323
- DB-API `Cursor.description` now reports the result type's top-level nullability instead of hardcoding `null_ok=True`, including the implicit null values supported by `Variant`, `Dynamic`, and `SimpleAggregateFunction` over a nullable element type. Existing `type_code` values are unchanged, and types whose nullability is unknown report `None`. The empty-result metadata probe also recognizes leading ClickHouse comments, including nested block comments, and is best effort, so a failed probe leaves `description` empty instead of raising after the original query succeeded. Closes [#902](https://github.com/ClickHouse/clickhouse-connect/issues/902), [#907](https://github.com/ClickHouse/clickhouse-connect/issues/907), and [#909](https://github.com/ClickHouse/clickhouse-connect/issues/909).
2424
- Compound values stored in JSON shared data, such as arrays of objects, heterogeneous arrays, and nested arrays, are now decoded to Python objects instead of being returned as raw bytes. `Date`, `DateTime`, and `DateTime64` values in shared data, both as scalars and inside arrays, now decode as well. Closes [#897](https://github.com/ClickHouse/clickhouse-connect/issues/897).
25+
- `AsyncClient` no longer tears down the aiohttp response from the parser's executor thread when a query fails mid-stream. The synchronous cleanup cancelled the producer task and closed the response directly, which raced with the event loop handling the server's connection abort and could surface an `AttributeError` from asyncio's SSL shutdown on TLS connections instead of the real `StreamFailureError`. Cleanup is now scheduled onto the event loop with `call_soon_threadsafe`.
2526

2627
## 1.6.0, 2026-07-23
2728

clickhouse_connect/driver/streaming.py

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ def __init__(self, response, encoding: str | None = None, exception_tag: str | N
4646
# Multiple accesses to .gen must return the same generator, not create new ones
4747
self._gen_cache: Iterator[bytes] | None = None
4848

49+
self._loop: asyncio.AbstractEventLoop | None = None
4950
self._producer_task: asyncio.Task | None = None
5051
self._producer_started = threading.Event()
5152
self._producer_error: Exception | None = None
@@ -90,6 +91,7 @@ async def producer():
9091
self.queue.shutdown()
9192
self._release_lease()
9293

94+
self._loop = loop
9395
self._producer_task = loop.create_task(producer())
9496
self._producer_started.set()
9597

@@ -196,12 +198,22 @@ def close(self):
196198
"""Synchronous cleanup resources"""
197199
self.queue.shutdown()
198200

199-
if self._producer_task and not self._producer_task.done():
200-
self._producer_task.cancel()
201+
def cleanup():
202+
if self._producer_task and not self._producer_task.done():
203+
self._producer_task.cancel()
204+
if self.response and not self.response.closed:
205+
if not self._producer_completed:
206+
self.response.close()
201207

202-
if self.response and not self.response.closed:
203-
if not self._producer_completed:
204-
self.response.close()
208+
# Task cancellation and aiohttp response teardown must run on the event
209+
# loop thread. close() is normally called from an executor thread.
210+
if self._loop is not None and not self._loop.is_closed():
211+
try:
212+
self._loop.call_soon_threadsafe(cleanup)
213+
except RuntimeError:
214+
cleanup()
215+
else:
216+
cleanup()
205217
self._release_lease()
206218

207219

tests/unit_tests/test_streaming_source.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import asyncio
22
import gzip
3+
import threading
34
import time
45
import zlib
56
from unittest.mock import Mock
@@ -352,6 +353,51 @@ def slow_consume():
352353
assert result == chunks
353354

354355

356+
@pytest.mark.asyncio
357+
async def test_sync_close_runs_on_event_loop_thread():
358+
"""close() from an executor thread runs response teardown on the loop thread."""
359+
loop_thread = threading.current_thread()
360+
close_thread = None
361+
362+
class StalledContent:
363+
@staticmethod
364+
async def read(n=-1):
365+
await asyncio.sleep(3600)
366+
367+
class StalledResponse:
368+
def __init__(self):
369+
self.content = StalledContent()
370+
self.headers = {}
371+
self.closed = False
372+
373+
def close(self):
374+
nonlocal close_thread
375+
close_thread = threading.current_thread()
376+
self.closed = True
377+
378+
response = StalledResponse()
379+
380+
source = StreamingResponseSource(response, encoding=None)
381+
loop = asyncio.get_running_loop()
382+
await source.start_producer(loop)
383+
384+
await loop.run_in_executor(None, source.close)
385+
386+
for _ in range(100):
387+
if response.closed:
388+
break
389+
await asyncio.sleep(0.01)
390+
391+
assert response.closed
392+
assert close_thread is loop_thread
393+
394+
if source._producer_task is not None:
395+
try:
396+
await source._producer_task
397+
except asyncio.CancelledError:
398+
pass
399+
400+
355401
class MockTransform:
356402
"""Mock NativeTransform."""
357403

0 commit comments

Comments
 (0)