Skip to content

Commit caf3c43

Browse files
committed
fix: protect long-running background runs from eviction and release them on stop
Two related bugs in the in-memory _BACKGROUND_RUNS registry that re-attach reads from. Both surfaced in the PR #13307 review; both have to land together so the registry's eviction policy and its cleanup path agree. A4: _register_background_run used to pop the oldest entry by insertion order when the dict hit _MAX_BACKGROUND_RUNS. A long-running first job got evicted by the 101st short job mid-run; re-attach returned 404 while the still-buffering task appended into an orphaned _BackgroundRun. The new policy prefers evicting the oldest completed entry. If every slot is still running, evict the oldest anyway to keep the registry bounded and log a warning so the situation is visible. A5: stop_workflow revoked the buffer task but left the _BackgroundRun in the registry with done=False. Re-attach readers could hang on _cond.wait() indefinitely (the task that would have called finish() was cancelled mid-execution), and cancelled buffers occupied memory until LRU evicted them. New _clear_background_run helper pops the entry and calls await bg_run.finish() so waiters wake to a clean stream end. stop_workflow calls it after revoke_task. Four unit tests pin both behaviors: eviction prefers completed entries, eviction falls back to the oldest when every run is active, clear pops and finishes the buffer, and clear is a no-op for unknown job ids. All use real _BackgroundRun instances via monkeypatch on the module-level dict; no mocks.
1 parent 47475c9 commit caf3c43

2 files changed

Lines changed: 130 additions & 3 deletions

File tree

src/backend/base/langflow/api/v2/workflow.py

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
from fastapi.sse import format_sse_event
3636
from lfx.events.event_manager import create_default_event_manager
3737
from lfx.graph.graph.base import Graph
38+
from lfx.log.logger import logger
3839
from lfx.schema.schema import InputValueRequest
3940
from lfx.schema.workflow import (
4041
WORKFLOW_EXECUTION_RESPONSES,
@@ -691,11 +692,41 @@ async def replay(self, start_index: int) -> AsyncIterator[bytes]:
691692
_BACKGROUND_RUNS: dict[str, _BackgroundRun] = {}
692693

693694

695+
async def _clear_background_run(job_id: str) -> None:
696+
"""Pop the background run registry entry and wake any re-attach waiters.
697+
698+
Called from ``stop_workflow`` after revoking the buffer task. Without
699+
this, ``_cond.wait()`` inside ``_BackgroundRun.replay`` can hang
700+
indefinitely (the buffer is never marked done because the task that
701+
would have called ``finish()`` was cancelled mid-execution), and the
702+
cancelled ``_BackgroundRun`` lingers in memory until LRU evicts it.
703+
"""
704+
bg_run = _BACKGROUND_RUNS.pop(job_id, None)
705+
if bg_run is not None:
706+
await bg_run.finish()
707+
708+
694709
def _register_background_run(job_id: str, bg_run: _BackgroundRun) -> None:
695-
"""Register a background run, evicting the oldest entry when full."""
710+
"""Register a background run, evicting a completed entry when full.
711+
712+
Prefer evicting the oldest *completed* run so a long-running job's
713+
re-attach handle survives. If every slot is still active, evict the
714+
oldest one anyway to keep the registry bounded, and log a warning.
715+
"""
696716
if len(_BACKGROUND_RUNS) >= _MAX_BACKGROUND_RUNS:
697-
oldest = next(iter(_BACKGROUND_RUNS))
698-
_BACKGROUND_RUNS.pop(oldest, None)
717+
evict_key = next(
718+
(key for key, run in _BACKGROUND_RUNS.items() if run.done),
719+
None,
720+
)
721+
if evict_key is None:
722+
evict_key = next(iter(_BACKGROUND_RUNS))
723+
logger.warning(
724+
"Background run registry full with no completed entries; "
725+
"evicting still-running job %s to make room for %s",
726+
evict_key,
727+
job_id,
728+
)
729+
_BACKGROUND_RUNS.pop(evict_key, None)
699730
_BACKGROUND_RUNS[job_id] = bg_run
700731

701732

@@ -1050,6 +1081,9 @@ async def stop_workflow(
10501081
try:
10511082
revoked = await task_service.revoke_task(job_id)
10521083
await job_service.update_job_status(job_id, JobStatus.CANCELLED)
1084+
# Release the in-memory buffer and wake any re-attach waiters so they
1085+
# see a clean stream-end instead of hanging on ``_cond.wait()``.
1086+
await _clear_background_run(str(job_id))
10531087

10541088
message = f"Job {job_id} cancelled successfully." if revoked else f"Job {job_id} is already cancelled."
10551089
return WorkflowStopResponse(job_id=str(job_id), message=message)

src/backend/tests/unit/api/v2/test_workflow_agui.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -968,3 +968,96 @@ async def test_background_mode_with_agui_protocol_still_buffers_agui_frames(
968968
text = events.text
969969
assert "RUN_STARTED" in text
970970
assert "RUN_FINISHED" in text
971+
972+
973+
class TestBackgroundRunsRegistryEviction:
974+
"""``_register_background_run`` must not evict still-running buffers.
975+
976+
The registry is bounded by ``_MAX_BACKGROUND_RUNS``. The naive policy of
977+
popping the oldest entry by insertion order drops the re-attach handle for
978+
a long-running first job the moment the limit is hit, so the buffer task
979+
keeps appending into an orphaned ``_BackgroundRun`` while re-attach
980+
returns 404. Eviction must prefer completed entries first; falling back
981+
to evicting the oldest only when every slot is occupied by a running run.
982+
"""
983+
984+
def test_eviction_prefers_completed_runs_over_running_ones(self, monkeypatch):
985+
from langflow.api.v2 import workflow as workflow_module
986+
987+
monkeypatch.setattr(workflow_module, "_MAX_BACKGROUND_RUNS", 3)
988+
monkeypatch.setattr(workflow_module, "_BACKGROUND_RUNS", {})
989+
990+
long_running = workflow_module._BackgroundRun(user_id="u", protocol_name="agui")
991+
# done stays False; this is the run we must protect.
992+
workflow_module._register_background_run("long", long_running)
993+
994+
# Fill the rest with completed runs.
995+
for job_id in ("done1", "done2"):
996+
done_run = workflow_module._BackgroundRun(user_id="u", protocol_name="agui")
997+
done_run.done = True
998+
workflow_module._register_background_run(job_id, done_run)
999+
1000+
# Registry is now at the cap (3): [long, done1, done2]. Adding a new
1001+
# entry must evict a completed run, not the still-running ``long``.
1002+
new_run = workflow_module._BackgroundRun(user_id="u", protocol_name="agui")
1003+
workflow_module._register_background_run("new", new_run)
1004+
1005+
assert "long" in workflow_module._BACKGROUND_RUNS, (
1006+
"Still-running background run was evicted in favor of a completed one"
1007+
)
1008+
assert "new" in workflow_module._BACKGROUND_RUNS
1009+
1010+
def test_eviction_falls_back_to_oldest_when_every_run_is_active(self, monkeypatch):
1011+
"""If every slot is occupied by a still-running run, evict the oldest anyway.
1012+
1013+
Unbounded growth would leak memory. The fallback is intentional and
1014+
documented; a warning log makes the situation visible.
1015+
"""
1016+
from langflow.api.v2 import workflow as workflow_module
1017+
1018+
monkeypatch.setattr(workflow_module, "_MAX_BACKGROUND_RUNS", 2)
1019+
monkeypatch.setattr(workflow_module, "_BACKGROUND_RUNS", {})
1020+
1021+
for job_id in ("a", "b"):
1022+
run = workflow_module._BackgroundRun(user_id="u", protocol_name="agui")
1023+
workflow_module._register_background_run(job_id, run)
1024+
1025+
# All running; adding a third must evict the oldest (a).
1026+
third = workflow_module._BackgroundRun(user_id="u", protocol_name="agui")
1027+
workflow_module._register_background_run("c", third)
1028+
1029+
assert "a" not in workflow_module._BACKGROUND_RUNS
1030+
assert "b" in workflow_module._BACKGROUND_RUNS
1031+
assert "c" in workflow_module._BACKGROUND_RUNS
1032+
1033+
1034+
class TestClearBackgroundRun:
1035+
"""``_clear_background_run`` releases a stopped run's buffer and wakes waiters.
1036+
1037+
Without this, ``stop_workflow`` revokes the buffer task but leaves the
1038+
``_BackgroundRun`` registered. Re-attach readers can hang on
1039+
``_cond.wait()`` indefinitely, and up to ``_MAX_BACKGROUND_RUNS`` cancelled
1040+
buffers occupy memory.
1041+
"""
1042+
1043+
async def test_clear_pops_registry_entry_and_finishes_buffer(self, monkeypatch):
1044+
from langflow.api.v2 import workflow as workflow_module
1045+
1046+
monkeypatch.setattr(workflow_module, "_BACKGROUND_RUNS", {})
1047+
1048+
bg_run = workflow_module._BackgroundRun(user_id="u", protocol_name="agui")
1049+
workflow_module._register_background_run("job-1", bg_run)
1050+
assert bg_run.done is False
1051+
1052+
await workflow_module._clear_background_run("job-1")
1053+
1054+
assert "job-1" not in workflow_module._BACKGROUND_RUNS
1055+
assert bg_run.done is True
1056+
1057+
async def test_clear_is_a_noop_for_unknown_job_id(self, monkeypatch):
1058+
from langflow.api.v2 import workflow as workflow_module
1059+
1060+
monkeypatch.setattr(workflow_module, "_BACKGROUND_RUNS", {})
1061+
1062+
# Must not raise even when nothing is registered.
1063+
await workflow_module._clear_background_run("nope")

0 commit comments

Comments
 (0)