You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
stdio_client with strict decoding withholds fatal read errors until peer stdout EOF #3714
Explicit encoding_error_handler="strict" decoding of invalid UTF-8 leaves the stdio consumer waiting until the peer closes stdout.
#2454 requests changing default decoding policy. This reproduction explicitly selects strict decoding, so changing the default to replacement would leave this failure-propagation case intact. Termination should become observable without requiring peer exit.
stdout_reader() enters its finally drain before closing the read-stream writer. _drain_stdout() waits for further raw bytes under a cancellation shield. A peer retaining stdout therefore blocks both stream closure and propagation of UnicodeDecodeError.
Example Code
Self-contained failing test reusing the module's existing in-memory FakeProcess and spawn monkeypatch. A byte peer is necessary because a typed server cannot emit malformed UTF-8. Test-only branch; no production changes.
After checking out that branch:
uv run --frozen pytest tests/client/test_stdio.py::test_strict_decode_failure_reaches_consumer_before_peer_stdout_closes -q
Event barriers prove consumption of malformed bytes. After tasks settle, the consumer has not failed. Releasing EOF then surfaces UnicodeDecodeError and completes clean shutdown. The explicit delayed-error assertion fails, without a timeout.
Python & MCP Python SDK
Python 3.14.7, Linux x86_64. Latest stable 2.3.0 (2118f14f8a19bc158d8a1cf90af58d85d187f849) and main 754254c08fe1c3eda18b69da3d336106fe8d72d6: three repetitions on asyncio and Trio all fail identically. The other 35 stdio tests pass against each revision.
AI-assisted investigation and test reproduction, with a human in the loop.
The ordering is the whole bug, and it is worth stating plainly because it reads as a decoding issue and it is not one. Strict decoding is behaving as specified: the decoder raises on invalid UTF-8. What goes wrong is that the raise cannot reach anyone, because the path that would propagate it runs behind a drain that is waiting on the peer.
stdout_reader() enters its finally drain before closing the read-stream writer, and _drain_stdout() waits for further raw bytes under a cancellation shield. A peer that keeps stdout open therefore holds the writer open, the drain never returns, and the UnicodeDecodeError sits inside a task that cannot complete. The consumer sees a client that has stopped producing rather than a client that has failed, and the difference matters, because the first looks like a slow peer and the second is actionable.
The fix I would argue for separates the two reasons a drain exists:
On the error path, do not drain. A decode failure is not a graceful shutdown, and the drain's purpose (consume what the peer already wrote so the process exits cleanly) does not apply. Bound it or skip it, and let the read-stream writer close so the exception propagates and the consumer learns the stream is dead.
Keep the shield where it belongs. A cancellation shield exists to make shutdown deterministic; using it on the failure path converts a deterministic error into a hang. Distinguishing the two call sites is enough, and it does not require changing the shield's behavior for the normal case.
Make termination observable without peer exit, which is your stated requirement and the right one. A client whose stdout peer is still alive should still be able to report that its own read side has failed.
On #2454: you are right that changing the default handler would not cover this, and the failing test on your branch is the better artifact, since it holds the strict path to the same termination guarantee. Keeping it as a regression test alongside whatever happens to the default is what stops a future policy change from reintroducing the hang.
I would like to take this. The failing test makes the behavior reproducible, and I plan to make the strict-decode failure observable before peer stdout closes while preserving the existing cleanup behavior. I will add focused regression coverage for the affected async backends and submit a small PR. Could a maintainer assign the issue so I can proceed under the contribution guidelines?
Thanks @Vamsi-2581! I'm happy for you to take the fix forward, subject to maintainer assignment; I'm not reserving it for myself.
The key behavior is that the consumer can observe the strict-decoding failure while the peer still holds stdout open. Please retain the event-barrier reproduction on asyncio and Trio alongside cleanup coverage, so the fix doesn't merely move the wait elsewhere in shutdown.
Activity
Ben-Home commentedon Oct 16, 2026
The ordering is the whole bug, and it is worth stating plainly because it reads as a decoding issue and it is not one. Strict decoding is behaving as specified: the decoder raises on invalid UTF-8. What goes wrong is that the raise cannot reach anyone, because the path that would propagate it runs behind a drain that is waiting on the peer.
stdout_reader()enters itsfinallydrain before closing the read-stream writer, and_drain_stdout()waits for further raw bytes under a cancellation shield. A peer that keeps stdout open therefore holds the writer open, the drain never returns, and theUnicodeDecodeErrorsits inside a task that cannot complete. The consumer sees a client that has stopped producing rather than a client that has failed, and the difference matters, because the first looks like a slow peer and the second is actionable.The fix I would argue for separates the two reasons a drain exists:
On the error path, do not drain. A decode failure is not a graceful shutdown, and the drain's purpose (consume what the peer already wrote so the process exits cleanly) does not apply. Bound it or skip it, and let the read-stream writer close so the exception propagates and the consumer learns the stream is dead.
Keep the shield where it belongs. A cancellation shield exists to make shutdown deterministic; using it on the failure path converts a deterministic error into a hang. Distinguishing the two call sites is enough, and it does not require changing the shield's behavior for the normal case.
Make termination observable without peer exit, which is your stated requirement and the right one. A client whose stdout peer is still alive should still be able to report that its own read side has failed.
On #2454: you are right that changing the default handler would not cover this, and the failing test on your branch is the better artifact, since it holds the strict path to the same termination guarantee. Keeping it as a regression test alongside whatever happens to the default is what stops a future policy change from reintroducing the hang.
Vamsi-2581 commentedon Oct 16, 2026
I would like to take this. The failing test makes the behavior reproducible, and I plan to make the strict-decode failure observable before peer stdout closes while preserving the existing cleanup behavior. I will add focused regression coverage for the affected async backends and submit a small PR. Could a maintainer assign the issue so I can proceed under the contribution guidelines?
inanna-malick commentedon Oct 16, 2026
Thanks @Vamsi-2581! I'm happy for you to take the fix forward, subject to maintainer assignment; I'm not reserving it for myself.
The key behavior is that the consumer can observe the strict-decoding failure while the peer still holds stdout open. Please retain the event-barrier reproduction on asyncio and Trio alongside cleanup coverage, so the fix doesn't merely move the wait elsewhere in shutdown.
AI-assisted follow-up.