From 66495a352c8554e17109beb463fecbdcc206b894 Mon Sep 17 00:00:00 2001 From: JingHao-Leon <102573344+JingHao-Leon@users.noreply.github.com> Date: Sun, 11 Oct 2026 12:41:10 +0800 Subject: [PATCH] fix(streaming): stop submitting the terminal log from __del__ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tests/unit/test_router_silent_experiment.py` failed on this branch with the shadow and fallback streaming tests seeing zero success callbacks: assert 0 == 1 # len(recording_logger.shadow_successes()) Bisected locally against `upstream/main` (26 passed there) — the finalizer is the cause, not `close()` or `aclose()`. Both of those turned out to be harmless: removing either one alone left all four tests failing, and removing the `__del__` flush took the file from 4 failed / 22 passed to 26 passed. The reason it breaks them is that `__del__` fires at an arbitrary point on an arbitrary thread. In these tests the wrapper is collected while the recording logger's bookkeeping is in flight, so the abandoned-stream flush races with the success callbacks the drain is supposed to produce. It is also unsound in general: a finalizer can run during interpreter shutdown or after the event loop it would need is gone, and it cannot report its own failure. So the flush is now reachable only through `close_stream()` and the context manager, which is where a caller has actually decided it is abandoning a stream. The docstrings say so explicitly rather than implying GC coverage. Verified locally: `test_router_silent_experiment.py` 26/26, the six tests added here 6/6, and `test_streaming_handler.py` unchanged against baseline (5 failed / 166 passed both with and without this branch — those five are pre-existing in this environment). --- litellm/litellm_core_utils/streaming_handler.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/litellm/litellm_core_utils/streaming_handler.py b/litellm/litellm_core_utils/streaming_handler.py index ed1e72d86ad..b41865fadc3 100644 --- a/litellm/litellm_core_utils/streaming_handler.py +++ b/litellm/litellm_core_utils/streaming_handler.py @@ -319,6 +319,13 @@ class CustomStreamWrapper: call even though tokens were served. Assemble the response from the chunks received so far and submit the same logging once, mirroring how the proxy bills a partial stream on client disconnect. + + Only reachable through ``close_stream()`` / the context manager. It is + deliberately not wired into ``__del__``: a finalizer runs at an + arbitrary point on an arbitrary thread, possibly during interpreter + shutdown or after the event loop it needs is gone, so submitting + logging from there is neither reliable nor testable. A caller that + abandons a stream has to say so. """ if self._abandonment_logged or self.sent_last_chunk: return @@ -371,6 +378,7 @@ class CustomStreamWrapper: all miss the call). ``close_stream()`` flushes that log from the chunks received so far and releases the underlying provider stream; safe to call after the stream has been fully consumed (it is a no-op then). + This is the only entry point — there is no GC-time fallback. Deliberately *not* named ``close``: ``Router.stream_with_fallbacks`` duck-types on ``hasattr(model_response, "close")`` and calls it from a @@ -459,10 +467,6 @@ class CustomStreamWrapper: still-active call's context within that same Task if this fires late. """ self._restore_consumer_correlation_context(guarded=True) - try: - self._flush_abandoned_stream_logging() - except Exception: # noqa: BLE001 # __del__ must never raise; logging here is best-effort - verbose_logger.debug("abandoned-stream logging failed in __del__", exc_info=True) async def aclose(self): # Emit the abandoned-stream log before closing the provider stream: