nodes/olivv-cs-ComfyUI-FunPack
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: give the in-flight guard its own dedicated flag instead of reusing state.gen

Browse Source 
Round 4 of adversarial review found that the previous commit's overlap
guard, keyed off state.gen.state, produced false refusals: Export, Save to
Media bin and Render Final all set state.gen to "running" for their own
entirely local work, so clicking H3 Combo Sweep's "Run sweep" button while
any of those was in progress got refused with a message borrowed from that
unrelated job ("Stopped at ...: Exporting 3 clips…") -- not silent, but
actively misleading about the real cause.

Switching the check to GenerateBridge's own run.state.phase (considered as
the obvious fix) would have reopened the very race round 3 closed: phase
stays "idle" for the whole check() network round trip, before run.start()
ever fires, so a second call could still slip through during that window.

Fixed with `_genRunActive`, a flag dedicated to exactly one question --
"does GenerateBridge have a run of this page's making in flight" -- set
synchronously the instant a call is accepted (before any await, so two
calls fired back to back still serialize) and cleared either by the
terminal-phase subscription (covers a run that actually started, click or
adopted) or by _generateRun's own `finally` (covers a run refused before
ever leaving "idle", which that subscription never sees).

Live-verified: a state.gen "running" left over from an unrelated job no
longer blocks a real generate() call, while two genuinely concurrent
generate() calls still correctly serialize (one proceeds, one is refused
with the accurate reason). 691/692 JS tests and 548/548 Python tests pass
(the one JS failure is the pre-existing, unrelated projects.test.js
import-race flake).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
digital-garbage
2026-09-15 12:10:23 +03:00
co-authored by Claude Sonnet 5
parent f803b1a329
commit a742318cc0
1 changed files with 40 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/legacy/store.js
+40 -5
View File
@@ -3409,6 +3409,16 @@
// phase would kill it after run 1 of N and never restart it for the rest.
// Only an adopted run has nobody else who will ever stop it.
let _genAdoptedRun = false;
// Whether GenerateBridge has a run of THIS page's making in flight --
// dedicated to that question alone, and nothing else's. state.gen.state
// looks like it would answer this but doesn't: Export, Save to Media bin
// and Render Final all set it to "running" for their own, entirely local
// work, and asking GenerateBridge's own run.state.phase instead has a real
// gap of its own (it stays "idle" for the whole check() round trip before
// run.start() ever fires). Flipped synchronously the moment a call is
// accepted -- before any await -- so two calls fired back to back without
// an await between them still serialize correctly.
let _genRunActive = false;
// One persistent subscription translating GenerateBridge's real run state
// (app/shell/run.js -- ComfyUI's own /prompt + /ws, not a second queue of
@@ -3456,6 +3466,7 @@
_genRunPrefix = "Generation";
_genRunSceneIds = (sceneIds && sceneIds.length) ? sceneIds : (sceneId ? [sceneId] : []);
_genAdoptedRun = true;
_genRunActive = true;
pollStart = Date.now();
_genClockStart("all", true); // true: approximate — we never saw the press
if (projectId && (!state.project || state.project.id !== projectId)
@@ -3475,9 +3486,14 @@
// of N and never restarted it for the rest. An adopted run has nobody
// else who will ever stop it (see the "adopt" handler above for why it
// needs starting here at all), so it alone is cleared once it lands.
if (_genAdoptedRun && (phase === "done" || phase === "failed" || phase === "cancelled")) {
_genAdoptedRun = false;
_genClockStop();
if (phase === "done" || phase === "failed" || phase === "cancelled") {
// _genRunActive is cleared unconditionally here -- covers BOTH an
// adopted run (nothing else ever clears it) and a click-started one
// (_generateRun's own `finally` also clears it, redundantly but
// harmlessly, for the paths that never reach a terminal phase at all
// -- a check() refusal, for one, which this subscription never sees).
_genRunActive = false;
if (_genAdoptedRun) { _genAdoptedRun = false; _genClockStop(); }
}
if (phase === "queued") {
set({ gen: { state: "queuing", promptId, media: [], msg: `${_genRunPrefix}: queuing…` } });
@@ -3703,7 +3719,7 @@
_wireGenerateBridge();
// GenerateBridge holds exactly ONE run, and _genRunPrefix/_genRunSceneIds/
// _genAdoptedRun belong to whichever call is currently driving it. A
// second call reaching here while a first is still queuing/running (the
// second call reaching here while a first is still queued/running (the
// main Generate/Selected buttons are gated against this by `busy(st)` in
// actionbar.js, but H3 Combo Sweep's own "Run sweep" button is not) would
// silently overwrite those with ITS scene/prefix, then get refused deeper
@@ -3711,10 +3727,22 @@
// eventual result recorded onto the WRONG scene when it finishes, and its
// clock stop possibly skipped. Refused here, loudly, before any of that
// shared state is touched, rather than corrupting it silently.
if (state.gen && (state.gen.state === "queuing" || state.gen.state === "running")) {
//
// Asked of _genRunActive, a flag dedicated to exactly this question --
// NOT state.gen.state (Export, Save to Media bin and Render Final also
// set that to "running" for their own, entirely local work -- reading it
// here would wrongly refuse a real sweep run with a message borrowed from
// whichever unrelated job happened to be in progress) and NOT
// GenerateBridge's own run.state.phase either (it stays "idle" for the
// whole check() round trip, before run.start() ever fires -- a real gap
// a second call could still race through). Checked and flipped in the
// same synchronous breath, before any await, so two calls fired back to
// back with nothing awaited between them still serialize correctly.
if (_genRunActive) {
console.warn(`[FunPack] "${prefix || "a generation"}" was refused: another generation is already in flight.`);
return false;
}
_genRunActive = true;
// A run started through the normal click path owns its own clock
// lifecycle (the _clockedX wrapper's `finally`, spanning the whole
// batch) — any stale flag left over from a PRIOR adopted run must not
@@ -3730,6 +3758,7 @@
}
const targetSceneIds = (sceneIds || []).filter(Boolean);
if (!targetSceneIds.length) {
_genRunActive = false; // never actually started — nothing to hold the flag for
set({ gen: { state: "error", promptId: null, media: [], msg: "No scenes to generate." } });
return false;
}
@@ -3771,6 +3800,12 @@
pollStart = Date.now();
return (await waiting) === GB.DONE;
} finally {
// Redundant with the GB.subscribe terminal-phase clearing above for a
// run that actually reached one, and the only place that clears it at
// all for a run that got refused before ever transitioning (check()
// saying no, or GB.generate() throwing) -- that subscription never
// sees either of those, since nothing ever left "idle".
_genRunActive = false;
_clearGenInFlight(targetSceneIds);
}
}