coalesce concurrent session spawns onto one future (T-374)
Orchestrator.spawn() check-then-acts on the session registry across two awaits (transcript-tail read, process start) — two racing callers for the same id both passed the check and the loser's live claude process was orphaned, never killed, never observed. The first caller now installs the spawn future synchronously; later callers await the same future, and a failed spawn clears the entry so a retry proceeds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -3999,3 +3999,5 @@ INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, chang
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHD098CV2N73823KX4Z99P4', 'status', 'in_progress', 'done', NULL, '2026-06-11 23:04:46', '2026-06-11 23:04:46', '2026-06-11 23:04:46', NULL, '5d8d8c7a17da4894510db957e7af3c3f', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHCP03EJ9CDBGZGRPD19N8W', 'status', 'backlog', 'in_progress', NULL, '2026-06-11 23:05:16', '2026-06-11 23:05:16', '2026-06-11 23:05:16', NULL, '252a8c28446f3c86876ec826fce03987', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHCP03EJ9CDBGZGRPD19N8W', 'status', 'in_progress', 'done', NULL, '2026-06-11 23:06:26', '2026-06-11 23:06:26', '2026-06-11 23:06:26', NULL, '4e1cc82f703d717f7592890e230caf00', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHC90B72A270CAKA7AP1ZX8', 'status', 'backlog', 'in_progress', NULL, '2026-06-11 23:07:01', '2026-06-11 23:07:01', '2026-06-11 23:07:01', NULL, '795c139edb5acd0d2a187dac6a3066c7', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHC90B72A270CAKA7AP1ZX8', 'status', 'in_progress', 'done', NULL, '2026-06-11 23:08:13', '2026-06-11 23:08:13', '2026-06-11 23:08:13', NULL, 'f83a2fc13f3839070957bea6ac7fdc5f', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
|
||||
@@ -4099,3 +4099,13 @@ INSERT INTO tickets (record_id, type, parent_record_id, title, description, stat
|
||||
Fix: pass the active workspace root as the spawn cwd (and re-derive it on project switch for new panes).
|
||||
|
||||
Acceptance: test that a terminal pane''s SpawnSpec.cwd equals the workspace root, not Directory.current.', 'done', 'medium', NULL, NULL, NULL, '2026-06-11 21:59:42', '2026-06-11 23:06:26', NULL, '8cad068c28a7a0cb6772a5fbdcbc34c3', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash);
|
||||
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHC90B72A270CAKA7AP1ZX8', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'Orchestrator.spawn() races itself — concurrent spawns for one id leak a live claude process', 'lib/builtin/claude/src/session_orchestrator.dart:191-249 — spawn() does check-then-act on the sessions map across two awaits (transcript-tail read, process start). Two concurrent spawns for the same session id both pass the check; the loser''s live claude process is orphaned, never killed, never observed.
|
||||
|
||||
Fix: hold a Map<String, Future<ManagedSession>> — first caller installs the future synchronously, later callers await the same future; remove the entry on failure.
|
||||
|
||||
Acceptance: test issuing two concurrent spawn() calls for one id yields the same ManagedSession instance and exactly one process spawn (count via injected spawner); failure path clears the in-flight entry so a retry can proceed.', 'in_progress', 'medium', NULL, NULL, NULL, '2026-06-11 21:57:56', '2026-06-11 23:07:01', NULL, 'f2df2148315332d48e2bf66cc2942bd3', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash);
|
||||
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHC90B72A270CAKA7AP1ZX8', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'Orchestrator.spawn() races itself — concurrent spawns for one id leak a live claude process', 'lib/builtin/claude/src/session_orchestrator.dart:191-249 — spawn() does check-then-act on the sessions map across two awaits (transcript-tail read, process start). Two concurrent spawns for the same session id both pass the check; the loser''s live claude process is orphaned, never killed, never observed.
|
||||
|
||||
Fix: hold a Map<String, Future<ManagedSession>> — first caller installs the future synchronously, later callers await the same future; remove the entry on failure.
|
||||
|
||||
Acceptance: test issuing two concurrent spawn() calls for one id yields the same ManagedSession instance and exactly one process spawn (count via injected spawner); failure path clears the in-flight entry so a retry can proceed.', 'done', 'medium', NULL, NULL, NULL, '2026-06-11 21:57:56', '2026-06-11 23:08:13', NULL, 'fde3f9e912ea1d25256003ef56dcf491', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash);
|
||||
|
||||
@@ -94,6 +94,11 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit.
|
||||
app process's working directory — `$HOME` for desktop launches, and the
|
||||
wrong repo after a project switch. (T-381)
|
||||
|
||||
- **Two simultaneous spawns of the same Claude session no longer leak a
|
||||
process.** Concurrent spawn calls for one pane id both passed the registry
|
||||
check and the loser's live process was orphaned; spawns for an id are now
|
||||
coalesced onto one in-flight future. (T-374)
|
||||
|
||||
- **Accepting ExitPlanMode now leaves plan mode in the conversation panel.**
|
||||
Approving Claude's plan (the ExitPlanMode tool) transitioned the underlying
|
||||
session out of plan mode, but clide's tracked permission mode didn't follow,
|
||||
|
||||
@@ -188,7 +188,28 @@ class ClaudeSessionOrchestrator extends ChangeNotifier {
|
||||
/// (T-269): the existing session belongs to the old repo, so it is torn down
|
||||
/// and a fresh one spawned for the new repo — a pane must never inherit
|
||||
/// another workspace's conversation.
|
||||
Future<ManagedSession> spawn(SpawnSpec spec) async {
|
||||
Future<ManagedSession> spawn(SpawnSpec spec) {
|
||||
// Serialize concurrent spawns per id (T-374): the body check-then-acts
|
||||
// on _sessions across two awaits, so two racing callers would both
|
||||
// pass the check and the loser's live claude process would be orphaned.
|
||||
// The first caller installs the future synchronously; the rest await
|
||||
// it. (A racing different-cwd spawn for the same id also coalesces —
|
||||
// the workspace-switch flow is sequential, so that pair never races.)
|
||||
final inFlight = _spawning[spec.id];
|
||||
if (inFlight != null) return inFlight;
|
||||
final f = _spawn(spec);
|
||||
_spawning[spec.id] = f;
|
||||
unawaited(
|
||||
f.then<void>((_) {}, onError: (Object _) {}).whenComplete(() {
|
||||
if (identical(_spawning[spec.id], f)) _spawning.remove(spec.id);
|
||||
}),
|
||||
);
|
||||
return f;
|
||||
}
|
||||
|
||||
final Map<String, Future<ManagedSession>> _spawning = {};
|
||||
|
||||
Future<ManagedSession> _spawn(SpawnSpec spec) async {
|
||||
final existing = _sessions[spec.id];
|
||||
if (existing != null) {
|
||||
if (existing.cwd == spec.cwd) return existing;
|
||||
|
||||
@@ -60,6 +60,32 @@ void main() {
|
||||
expect(created, hasLength(1));
|
||||
});
|
||||
|
||||
// T-374: spawn() check-then-acts across awaits; without the in-flight
|
||||
// map, two CONCURRENT spawns both passed the registry check and the
|
||||
// loser's live claude process was orphaned.
|
||||
test('two concurrent spawns for one id share one session and one process (T-374)', () async {
|
||||
final (a, b) = await (orch.spawn(spec('primary')), orch.spawn(spec('primary'))).wait;
|
||||
expect(identical(a, b), isTrue);
|
||||
expect(created, hasLength(1));
|
||||
});
|
||||
|
||||
test('a failed spawn clears the in-flight entry so a retry can proceed (T-374)', () async {
|
||||
var calls = 0;
|
||||
final flaky = ClaudeSessionOrchestrator(
|
||||
processFactory: ({required sessionArgs, required cwd, env}) async {
|
||||
calls++;
|
||||
if (calls == 1) throw StateError('spawn blew up');
|
||||
final p = _FakeProc();
|
||||
created.add(p);
|
||||
return p;
|
||||
},
|
||||
);
|
||||
await expectLater(flaky.spawn(spec('primary')), throwsStateError);
|
||||
final m = await flaky.spawn(spec('primary'));
|
||||
expect(m.id, 'primary');
|
||||
expect(calls, 2);
|
||||
});
|
||||
|
||||
test('hide keeps the process alive and in the registry; show restores it', () async {
|
||||
await orch.spawn(spec('primary'));
|
||||
orch.hide('primary');
|
||||
|
||||
Reference in New Issue
Block a user