Skip to content

Commit 2bfabb3

Browse files
authored
fix(runtime): prevent streaming-bridge deadlock on client disconnect (aws#482) (aws#563)
The async->sync streaming bridge (_async_gen_to_sync_gen) ran its producer coroutine on the single shared worker event loop and used a blocking queue.Queue.put(). When an SSE consumer stopped draining (client disconnect or the 15-min read timeout), the bounded queue filled and the blocking put froze the worker-loop thread process-wide, starving every other session's handler on the microVM. Symptoms: invocations hang with no logs while /ping stays healthy. Fix: - Producer now uses a non-blocking put_nowait and yields the loop via asyncio.sleep when the queue is full, so it never blocks the shared loop. - A stop event, set in the consumer's finally (fires on GeneratorExit at client disconnect), tears the orphaned producer down and frees a queue slot so a parked producer wakes and exits. The source async generator is aclosed. Adds unit tests (worker loop survives an abandoned stream; a second session survives the first's disconnect) and a real-server integration test that reproduces the disconnect over a full HTTP stack.
1 parent 8bbfe18 commit 2bfabb3

3 files changed

Lines changed: 295 additions & 11 deletions

File tree

src/bedrock_agentcore/runtime/app.py

Lines changed: 63 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
"""
55

66
import asyncio
7+
import contextlib
78
import contextvars
89
import functools
910
import inspect
@@ -725,29 +726,80 @@ def _async_gen_to_sync_gen(self, async_gen: Any, ctx: contextvars.Context) -> An
725726
a thread-safe queue and yielded synchronously. Starlette's StreamingResponse
726727
iterates this sync generator via iterate_in_threadpool, so the main event
727728
loop is never blocked.
729+
730+
The producer must never block the shared worker loop, or a full queue
731+
would freeze it and starve every other session. When the consumer stops
732+
(SSE client disconnect), the producer task is cancelled so it is torn
733+
down even while parked awaiting a slow downstream, and a stop event
734+
keeps a queue-full producer from spinning before the cancel lands.
728735
"""
729736
worker_loop = self._ensure_worker_loop()
730737
q: queue.Queue = queue.Queue(maxsize=100)
731738
_DONE = object()
739+
stop = threading.Event()
740+
task_cell: dict[str, asyncio.Task] = {}
732741

733742
async def _produce() -> None:
734743
_restore_context(ctx)
735744
try:
736745
async for chunk in async_gen:
737-
q.put((True, chunk))
738-
q.put((True, _DONE))
746+
await _offer((True, chunk))
747+
if stop.is_set():
748+
return
749+
await _offer((True, _DONE))
750+
except asyncio.CancelledError:
751+
# Consumer disconnected; nothing reads the queue anymore. Let the
752+
# cancellation unwind so the finally below still closes the source.
753+
raise
739754
except BaseException as e:
740-
q.put((False, e))
755+
await _offer((False, e))
756+
finally:
757+
# Ensure the source generator releases its resources if the
758+
# consumer disconnected mid-stream.
759+
aclose = getattr(async_gen, "aclose", None)
760+
if aclose is not None:
761+
with contextlib.suppress(Exception):
762+
await aclose()
763+
764+
async def _offer(item: Any) -> None:
765+
"""Put an item without ever blocking the worker loop.
766+
767+
Retries with a short async sleep while the queue is full so the loop
768+
stays free to run other coroutines. Bails out if the consumer stopped.
769+
"""
770+
while not stop.is_set():
771+
try:
772+
q.put_nowait(item)
773+
return
774+
except queue.Full:
775+
await asyncio.sleep(0.01)
776+
777+
def _start() -> None:
778+
task_cell["task"] = worker_loop.create_task(_produce())
741779

742-
worker_loop.call_soon_threadsafe(lambda: worker_loop.create_task(_produce()))
780+
worker_loop.call_soon_threadsafe(_start)
743781

744-
while True:
745-
ok, value = q.get()
746-
if not ok:
747-
raise value
748-
if value is _DONE:
749-
break
750-
yield value
782+
try:
783+
while True:
784+
ok, value = q.get()
785+
if not ok:
786+
raise value
787+
if value is _DONE:
788+
break
789+
yield value
790+
finally:
791+
# Consumer is done or the client disconnected (GeneratorExit). Set the
792+
# stop flag (unblocks a queue-full producer that hasn't started its
793+
# await yet) and cancel the task so a producer parked in a mid-chunk
794+
# await is torn down immediately instead of leaking until it returns.
795+
stop.set()
796+
797+
def _cancel() -> None:
798+
task = task_cell.get("task")
799+
if task is not None and not task.done():
800+
task.cancel()
801+
802+
worker_loop.call_soon_threadsafe(_cancel)
751803

752804
async def _invoke_handler(self, handler: Callable, request_context: Any, takes_context: bool, payload: Any) -> Any:
753805
"""Dispatch handler execution based on handler type.

tests/bedrock_agentcore/runtime/test_app.py

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import asyncio
2+
import concurrent.futures
23
import contextlib
4+
import contextvars
35
import json
46
import os
57
import threading
@@ -3061,3 +3063,134 @@ def test_x_amzn_prefix_is_blocked(self):
30613063
def test_authorization_is_allowed(self):
30623064
# Authorization is normalized to canonical casing by the loop, but is_forwardable_header must not block it
30633065
assert is_forwardable_header("Authorization") is True
3066+
3067+
3068+
class TestStreamingBridgeDeadlock:
3069+
"""Regression tests for the async->sync streaming bridge deadlock.
3070+
3071+
When a streaming (async generator) handler produces faster than the SSE
3072+
consumer drains, the bounded queue used by ``_async_gen_to_sync_gen`` fills
3073+
up. The producer coroutine runs on the single shared worker loop, so a
3074+
blocking put on a full queue freezes that loop process-wide and starves
3075+
every other session's handler. If the consumer goes away entirely (client
3076+
disconnect), the producer must be torn down rather than left spinning.
3077+
"""
3078+
3079+
@staticmethod
3080+
def _abandon_after(sync_gen, n):
3081+
"""Consume ``n`` chunks from a bridged sync generator, then close it.
3082+
3083+
Mimics an SSE client that reads a bit and disconnects. Runs in a daemon
3084+
thread so a deadlocked ``q.get()`` can never hang the test thread.
3085+
"""
3086+
for _ in range(n):
3087+
next(sync_gen)
3088+
sync_gen.close() # StreamingResponse triggers GeneratorExit on client disconnect
3089+
3090+
def _run_bounded(self, target, timeout):
3091+
"""Run ``target`` in a daemon thread; return True iff it finished within ``timeout``."""
3092+
done = threading.Event()
3093+
error = {}
3094+
3095+
def _wrap():
3096+
try:
3097+
target()
3098+
except BaseException as e: # noqa: BLE001 - surface in assertion
3099+
error["e"] = e
3100+
finally:
3101+
done.set()
3102+
3103+
t = threading.Thread(target=_wrap, daemon=True)
3104+
t.start()
3105+
finished = done.wait(timeout)
3106+
if finished and "e" in error:
3107+
raise error["e"]
3108+
return finished
3109+
3110+
def test_worker_loop_survives_abandoned_stream(self):
3111+
"""A consumer that stops early and disconnects must not freeze the worker loop.
3112+
3113+
The producer emits far more than the bridge's queue
3114+
maxsize; the consumer reads a few chunks then closes (SSE disconnect).
3115+
Afterwards a fresh coroutine must still run on the shared worker loop.
3116+
"""
3117+
app = BedrockAgentCoreApp()
3118+
3119+
async def big_stream():
3120+
# Far beyond the bridge's bounded queue so a blocking producer wedges the loop.
3121+
for i in range(10_000):
3122+
yield f"chunk-{i}"
3123+
3124+
sync_gen = app._async_gen_to_sync_gen(big_stream(), contextvars.copy_context())
3125+
# Abandon the stream in a bounded thread (guards against a hang here too).
3126+
assert self._run_bounded(lambda: self._abandon_after(sync_gen, 3), timeout=10.0), (
3127+
"abandoning the stream hung — consumer blocked on the bridge"
3128+
)
3129+
time.sleep(0.5) # let the orphaned producer either exit or wedge
3130+
3131+
# The shared worker loop must still be able to run new work.
3132+
loop = app._ensure_worker_loop()
3133+
3134+
async def _noop():
3135+
return "alive"
3136+
3137+
fut = asyncio.run_coroutine_threadsafe(_noop(), loop)
3138+
try:
3139+
alive = fut.result(timeout=5.0) == "alive"
3140+
except (concurrent.futures.TimeoutError, TimeoutError):
3141+
fut.cancel()
3142+
alive = False
3143+
assert alive, "worker loop is wedged after an abandoned stream — bridge deadlocked"
3144+
3145+
def test_second_session_survives_first_sessions_abandoned_stream(self):
3146+
"""After session A abandons its stream, session B's streaming handler must still run."""
3147+
app = BedrockAgentCoreApp()
3148+
3149+
async def big_stream():
3150+
for i in range(10_000):
3151+
yield f"a-{i}"
3152+
3153+
gen_a = app._async_gen_to_sync_gen(big_stream(), contextvars.copy_context())
3154+
assert self._run_bounded(lambda: self._abandon_after(gen_a, 1), timeout=10.0), "abandoning session A hung"
3155+
time.sleep(0.5)
3156+
3157+
async def small_stream():
3158+
for i in range(3):
3159+
yield f"b-{i}"
3160+
3161+
gen_b = app._async_gen_to_sync_gen(small_stream(), contextvars.copy_context())
3162+
collected = []
3163+
# Consume B in a bounded thread: if the loop is starved, q.get() blocks forever
3164+
# and this returns False rather than hanging the suite.
3165+
completed = self._run_bounded(lambda: collected.extend(list(gen_b)), timeout=5.0)
3166+
assert completed and collected == ["b-0", "b-1", "b-2"], (
3167+
f"session B did not complete — worker loop starved by session A; completed={completed} got={collected}"
3168+
)
3169+
3170+
def test_disconnect_tears_down_producer_blocked_on_slow_downstream(self):
3171+
"""Disconnecting while the handler is parked mid-await must cancel the producer.
3172+
3173+
The source generator yields one chunk then blocks on a long await (a slow
3174+
downstream). On client disconnect the producer must be cancelled so the
3175+
generator's ``finally`` runs and its resources are released — otherwise
3176+
the task and the upstream connection leak until the await returns.
3177+
"""
3178+
app = BedrockAgentCoreApp()
3179+
cleaned_up = threading.Event()
3180+
3181+
async def stalled_stream():
3182+
try:
3183+
yield "first"
3184+
await asyncio.sleep(3600) # park on a slow/hung downstream
3185+
yield "never"
3186+
finally:
3187+
cleaned_up.set() # runs only if the coroutine is closed/cancelled
3188+
3189+
sync_gen = app._async_gen_to_sync_gen(stalled_stream(), contextvars.copy_context())
3190+
# Read the first chunk, then disconnect while the producer is parked in the await.
3191+
assert self._run_bounded(lambda: self._abandon_after(sync_gen, 1), timeout=10.0), "abandoning hung"
3192+
3193+
assert cleaned_up.wait(timeout=5.0), (
3194+
"producer parked in a mid-await was not torn down on disconnect — "
3195+
"source generator's finally never ran (task leaked)"
3196+
)
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
"""Integration test for the streaming-bridge deadlock.
2+
3+
Boots a real uvicorn server running a BedrockAgentCoreApp with a streaming
4+
(async generator) entrypoint, then drives it with real HTTP requests:
5+
6+
1. Session A opens the SSE stream, reads a few chunks, then DISCONNECTS.
7+
2. Session B (fresh) must still receive a normal response.
8+
9+
Before the fix, A's disconnect fills the bridge's bounded queue and blocks the
10+
shared worker loop, so B hangs and its handler never runs. After the fix, the
11+
orphaned producer is torn down and B succeeds.
12+
13+
No mocks: full ASGI stack over a loopback socket.
14+
"""
15+
16+
import socket
17+
import threading
18+
import time
19+
20+
import httpx
21+
import pytest
22+
import uvicorn
23+
24+
from bedrock_agentcore.runtime import BedrockAgentCoreApp
25+
26+
27+
def _free_port():
28+
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
29+
s.bind(("127.0.0.1", 0))
30+
port = s.getsockname()[1]
31+
s.close()
32+
return port
33+
34+
35+
@pytest.fixture()
36+
def live_server():
37+
"""Run a streaming BedrockAgentCoreApp on a real uvicorn server in a thread."""
38+
app = BedrockAgentCoreApp()
39+
40+
@app.entrypoint
41+
async def handler(payload):
42+
n = int(payload.get("chunks", 5000))
43+
for i in range(n):
44+
yield f"data: chunk {i}\n\n"
45+
46+
port = _free_port()
47+
config = uvicorn.Config(app, host="127.0.0.1", port=port, log_level="warning")
48+
server = uvicorn.Server(config)
49+
thread = threading.Thread(target=server.run, daemon=True)
50+
thread.start()
51+
52+
# Wait for the server to accept connections.
53+
base = f"http://127.0.0.1:{port}"
54+
deadline = time.time() + 10
55+
while time.time() < deadline:
56+
try:
57+
httpx.get(f"{base}/ping", timeout=1.0)
58+
break
59+
except Exception:
60+
time.sleep(0.1)
61+
else:
62+
server.should_exit = True
63+
pytest.fail("live server did not start")
64+
65+
yield base
66+
67+
server.should_exit = True
68+
thread.join(timeout=5)
69+
70+
71+
def test_ping_stays_healthy_and_fresh_session_works_after_disconnect(live_server):
72+
"""After a mid-stream client disconnect: /ping stays healthy AND a fresh session responds."""
73+
base = live_server
74+
75+
# Session A: open stream, read a few chunks, then disconnect by leaving the context.
76+
read = 0
77+
with httpx.Client(timeout=30.0) as c:
78+
with c.stream("POST", f"{base}/invocations", json={"sessionId": "A", "chunks": 5000}) as r:
79+
for _ in r.iter_lines():
80+
read += 1
81+
if read >= 3:
82+
break
83+
assert read >= 3
84+
85+
time.sleep(2) # let the (now-orphaned) producer fill the queue if it's going to block
86+
87+
# The main loop must still answer health checks (it always did — proves the split).
88+
ping = httpx.get(f"{base}/ping", timeout=5.0)
89+
assert ping.status_code == 200
90+
91+
# The worker loop must still serve a fresh session. Before the fix this times out.
92+
resp = httpx.post(
93+
f"{base}/invocations",
94+
json={"sessionId": "B", "chunks": 3},
95+
headers={"Accept": "text/event-stream"},
96+
timeout=15.0,
97+
)
98+
assert resp.status_code == 200
99+
assert "chunk 0" in resp.text

0 commit comments

Comments
 (0)