Skip to content

Commit 9f462bf

Browse files
ryan-williamsclaude
andcommitted
fix: _wait_for_co_outputs skips cross-level co-outputs
`ParallelExecutor._wait_for_co_outputs` had the primary block on every co-output of its cmd, drawn from `_cmd_artifact_paths[cmd]` which spans **all** artifacts across all levels. If a co-output was scheduled for a later level, its `_execute_artifact` hadn't been called yet, its `_dvc_done_events[co_path]` could never be set, and the primary parked on `Event.wait()` forever. Main thread parked in `as_completed` waiting for the primary's future. Deadlock; the pool ran to timeout. Concrete failure mode: hudcostreets/path's `path-data months` produces 8 co-outputs — 6 land in Level 1 (they share the same per-year `.pqt` deps as the primary `www/public/all.pqt`) but 2 land in Level 2 (`data/all.pqt` + `data/all.xlsx`, whose `.dvc` deps reference the `data/*.pqt` outputs). The `path-data` daily-updater GHA has hung at its 6h timeout every day since 2026-06-20 as a result. Fix: track `_scheduled_paths` — a set populated at the start of each `_execute_level` with that level's paths. `_wait_for_co_outputs` filters `_cmd_artifact_paths[cmd]` down to co-outputs already in `_scheduled_paths`, so cross-level entries are silently skipped. Their .dvcs still get written when their own level runs (via `_handle_co_output`); the primary just doesn't wait for them. `test_cross_level_co_output_does_not_deadlock` sets up A (Level 1) + B (Level 2, depends on A) sharing a cmd, runs the executor in a thread, and asserts `t.join(timeout=15)` returns. Without the fix it deadlocks and hits the 15s timeout; with it, both artifacts complete cleanly. Full suite: 238 passed, 4 skipped (S3 integration). No regressions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 09ba612 commit 9f462bf

2 files changed

Lines changed: 77 additions & 3 deletions

File tree

src/dvx/run/executor.py

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,11 @@ def __init__(
162162
self._dvc_done_events: dict[str, threading.Event] = {
163163
a.path: threading.Event() for a in artifacts if a.computation is not None
164164
}
165+
# Populated at the start of each level with that level's artifact
166+
# paths. `_wait_for_co_outputs` filters against this so the primary
167+
# only blocks on same-level co-outputs; cross-level co-outputs
168+
# haven't been submitted to the pool yet and can never signal.
169+
self._scheduled_paths: set[str] = set()
165170

166171
def execute(self) -> list[ExecutionResult]:
167172
"""Execute all artifacts, respecting dependencies.
@@ -311,6 +316,12 @@ def _execute_level(self, artifacts: list[Artifact]) -> list[ExecutionResult]:
311316
Returns:
312317
List of ExecutionResult, one per artifact
313318
"""
319+
# Track which artifacts are in-flight this level so `_wait_for_co_outputs`
320+
# can filter out cross-level co-outputs (their `_dvc_done_events` won't
321+
# be armed until a later level submits them — waiting would deadlock).
322+
for a in artifacts:
323+
self._scheduled_paths.add(a.path)
324+
314325
if len(artifacts) == 1:
315326
# Single artifact - run directly without thread pool overhead
316327
return [self._execute_artifact(artifacts[0])]
@@ -686,16 +697,26 @@ def _signal_dvc_done(self, path: str) -> None:
686697
ev.set()
687698

688699
def _wait_for_co_outputs(self, cmd: str | None, my_path: str) -> list[str]:
689-
"""Block until every co-output of ``cmd`` (other than ``my_path``) has
690-
finished writing its .dvc. Returns the list of co-output paths.
700+
"""Block until every same-level co-output of ``cmd`` (other than
701+
``my_path``) has finished writing its .dvc. Returns those paths.
691702
692703
Called from the primary stage's thread before commit + cache push so
693704
that ``git add -u`` captures every co-output's md5 update and the
694705
push manifest includes every co-output's blob.
706+
707+
Cross-level co-outputs are filtered out via ``_scheduled_paths``:
708+
their ``_execute_artifact`` won't be invoked until a later level
709+
submits them, so their ``_dvc_done_events`` can never be set —
710+
waiting would deadlock the pool (as it did on hccs/path when
711+
``data/all.{pqt,xlsx}`` landed a level below their ``www/public/``
712+
siblings despite sharing ``path-data months``).
695713
"""
696714
if not cmd:
697715
return []
698-
co_paths = [p for p in self._cmd_artifact_paths.get(cmd, []) if p != my_path]
716+
co_paths = [
717+
p for p in self._cmd_artifact_paths.get(cmd, [])
718+
if p != my_path and p in self._scheduled_paths
719+
]
699720
for co_path in co_paths:
700721
ev = self._dvc_done_events.get(co_path)
701722
if ev is not None:

tests/test_executor.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,59 @@ def test_multi_output_partial_failure(tmp_workdir):
178178
assert "not created" in failures[0].reason or "not produced" in failures[0].reason
179179

180180

181+
def test_cross_level_co_output_does_not_deadlock(tmp_workdir):
182+
"""Co-outputs of one cmd that end up in different levels must not deadlock.
183+
184+
When artifact B declares artifact A as a dep AND they share the same cmd,
185+
``_group_into_levels`` places A in Level 1 and B in Level 2 despite
186+
them being co-outputs. Before the fix, Level 1's primary called
187+
``_wait_for_co_outputs`` and blocked on B's ``_dvc_done_event`` — which
188+
could never fire because B's ``_execute_artifact`` doesn't run until
189+
Level 2 is submitted. Deadlock: primary parked on B's event, main
190+
thread parked in ``as_completed`` waiting for the primary. Repro
191+
matched hccs/path's ``path-data months`` outputs (see 2026-06-19+
192+
daily-update runs that hung at 6h GHA timeout).
193+
"""
194+
script = tmp_workdir / "multi.sh"
195+
script.write_text(
196+
"#!/bin/bash\n"
197+
"echo l1 > a.txt\n"
198+
"echo l2 > b.txt\n"
199+
)
200+
script.chmod(0o755)
201+
cmd = f"bash {script}"
202+
203+
# a.txt: no deps → Level 1. b.txt: dep on a.txt → Level 2. Same cmd.
204+
a = Artifact(path="a.txt", computation=Computation(cmd=cmd, deps=[]))
205+
b = Artifact(
206+
path="b.txt",
207+
computation=Computation(cmd=cmd, deps=[a]),
208+
)
209+
210+
# Run in a thread so a deadlock surfaces as a timeout, not a hung suite.
211+
import threading
212+
executor = ParallelExecutor([a, b], ExecutionConfig(max_workers=2), StringIO())
213+
results: list = []
214+
errors: list = []
215+
216+
def _run():
217+
try:
218+
results.extend(executor.execute())
219+
except Exception as e: # pragma: no cover - surfaced via assertion
220+
errors.append(e)
221+
222+
t = threading.Thread(target=_run, daemon=True)
223+
t.start()
224+
t.join(timeout=15)
225+
assert not t.is_alive(), (
226+
"executor deadlocked — primary waited on a cross-level co-output "
227+
"whose _dvc_done_event was never set"
228+
)
229+
assert not errors, f"executor raised: {errors[0]!r}"
230+
assert len(results) == 2
231+
assert all(r.success for r in results), [r.reason for r in results]
232+
233+
181234
def test_multi_output_command_failure(tmp_workdir):
182235
"""Test handling when the shared command fails."""
183236
cmd = "exit 1"

0 commit comments

Comments
 (0)