Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions framework/py/flwr/supercore/task_process/agent/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import json
import time
from collections.abc import Sequence
from queue import Empty, Full, Queue
from queue import Empty, Queue
from threading import Lock, Thread
from typing import Literal, cast

Expand Down Expand Up @@ -106,11 +106,8 @@ def close(self, timeout: float | None = None) -> None:
self._raise_worker_error()
return

try:
self._queue.put_nowait(_EVENT_PUBLISH_STOP)
except Full:
pass # The worker will still stop due to the `_closed` flag.
self._closed = True
self._queue.put(_EVENT_PUBLISH_STOP)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor the close timeout while enqueueing the stop marker

When the 256-entry queue is full because PushTaskEvents is slow or stalled, this blocking put occurs before join(timeout), so the timeout does not bound close at all. In particular, run_agentapp.py calls agent_events.close(1) during exit, but shutdown can instead wait for the Runtime HTTP request timeout (or indefinitely if the worker has terminated unexpectedly). Apply the same deadline to inserting the stop marker, or otherwise avoid blocking before the timed join.

Useful? React with 👍 / 👎.

self._worker.join(timeout)
if self._worker.is_alive():
raise TimeoutError("Timed out waiting for Agent event publisher to stop.")
Comment thread
panh99 marked this conversation as resolved.
Expand All @@ -127,7 +124,7 @@ def _flush(self, batch: list[TaskEvent]) -> None:

def _run(self) -> None:
"""Upload queued events in small batches."""
while not self._closed:
while True:
item = self._queue.get()
if item is _EVENT_PUBLISH_STOP:
return
Expand Down
25 changes: 25 additions & 0 deletions framework/py/flwr/supercore/task_process/agent/session_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,31 @@ def test_emit_event_pushes_task_event() -> None:
)


def test_close_drains_events_before_worker_stops() -> None:
"""Close should publish queued events before stopping the worker."""
stub = Mock()
with patch("flwr.supercore.task_process.agent.session.Thread") as thread_cls:
thread_cls.return_value.is_alive.return_value = False
events = RuntimeAgentEvents(stub)
worker_target = thread_cls.call_args.kwargs["target"]
thread_cls.return_value.join.side_effect = lambda _timeout: worker_target()

event: JSONObject = {
"type": "response.output_text.delta",
"delta": "Hello",
}
events.emit(event)
events.close()

expected_event = TaskEvent(
event="response.output_text.delta",
data='{"type":"response.output_text.delta","delta":"Hello"}',
)
stub.PushTaskEvents.assert_called_once_with(
PushTaskEventsRequest(events=[expected_event])
)


def test_emit_event_requires_type() -> None:
"""Emit should reject events without a valid type."""
stub = Mock()
Expand Down
Loading