Skip to content

Commit 1a38a8d

Browse files
committed
fix(tui): reject a session op that returns its own queued work
1 parent 0f30efd commit 1a38a8d

2 files changed

Lines changed: 53 additions & 8 deletions

File tree

‎src/tui/delivery-queue.ts‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,12 @@ export function createSessionOperationQueue(): SessionOperationQueue {
504504
});
505505
};
506506

507+
// `observed === activeResult` is the adoption of a thenable returned from
508+
// the op. That `.then` runs after AsyncLocalStorage.run returns.
509+
const isSelfWait = (during: number, observed: object): boolean =>
510+
activeId === during &&
511+
(observed === activeResult || (inThisOp() && bodyStillRunning()));
512+
507513
const enqueue = (op: () => Promise<void>): Promise<void> => {
508514
// `tail.then(op, op)` — a failed op must not wedge the chain. The
509515
// returned promise is not that tail: a self-wait rejection has to
@@ -515,9 +521,8 @@ export function createSessionOperationQueue(): SessionOperationQueue {
515521
tail = scheduled;
516522
if (!inThisOp()) return scheduled;
517523
const enqueuedDuring = activeId;
518-
return observeUnlessSelfWait(
519-
scheduled,
520-
() => inThisOp() && activeId === enqueuedDuring && bodyStillRunning(),
524+
return observeUnlessSelfWait(scheduled, (observed) =>
525+
isSelfWait(enqueuedDuring, observed),
521526
);
522527
};
523528

@@ -528,9 +533,8 @@ export function createSessionOperationQueue(): SessionOperationQueue {
528533
// deadlocks the same way as awaiting enqueue.
529534
if (!inThisOp()) return drained;
530535
const awaitedDuring = activeId;
531-
return observeUnlessSelfWait(
532-
drained,
533-
() => inThisOp() && activeId === awaitedDuring && bodyStillRunning(),
536+
return observeUnlessSelfWait(drained, (observed) =>
537+
isSelfWait(awaitedDuring, observed),
534538
);
535539
};
536540

@@ -545,7 +549,7 @@ export function createSessionOperationQueue(): SessionOperationQueue {
545549
*/
546550
function observeUnlessSelfWait(
547551
forward: Promise<void>,
548-
stillRunning: () => boolean,
552+
stillRunning: (observed: object) => boolean,
549553
): Promise<void> {
550554
let reported: SessionOperationSelfWaitError | undefined;
551555
let guarded = false;
@@ -583,7 +587,7 @@ function observeUnlessSelfWait(
583587
): Promise<undefined>;
584588
} = {
585589
then(onFulfilled, onRejected) {
586-
if (stillRunning()) {
590+
if (stillRunning(thenable)) {
587591
guardUnobserved();
588592
return reject().then(onFulfilled, onRejected);
589593
}

‎src/tui/session-operation-queue.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,3 +521,44 @@ test("an outside waiter is not rejected while the enqueuing op is blocked", asyn
521521
expect(observed).toBe("resolved");
522522
expect(log).toEqual(["outer:end", "inner"]);
523523
});
524+
525+
async function settleOrTimeout(promise: Promise<unknown>): Promise<string> {
526+
return Promise.race([
527+
promise.then(
528+
() => "resolved",
529+
(err: unknown) =>
530+
err instanceof SessionOperationSelfWaitError ? "self-wait" : "error",
531+
),
532+
new Promise<string>((resolve) => setTimeout(() => resolve("timeout"), 200)),
533+
]);
534+
}
535+
536+
test("a non-async callback that returns enqueue rejects and the queue drains", async () => {
537+
const log: string[] = [];
538+
const { enqueue, awaitTail } = createSessionOperationQueue();
539+
540+
const outer = enqueue(() => {
541+
log.push("outer");
542+
return enqueue(async () => {
543+
log.push("inner");
544+
});
545+
});
546+
547+
expect(await settleOrTimeout(outer)).toBe("self-wait");
548+
expect(await settleOrTimeout(awaitTail())).toBe("resolved");
549+
expect(log).toEqual(["outer", "inner"]);
550+
});
551+
552+
test("a non-async callback that returns awaitTail rejects and the queue drains", async () => {
553+
const log: string[] = [];
554+
const { enqueue, awaitTail } = createSessionOperationQueue();
555+
556+
const outer = enqueue(() => {
557+
log.push("outer");
558+
return awaitTail();
559+
});
560+
561+
expect(await settleOrTimeout(outer)).toBe("self-wait");
562+
expect(await settleOrTimeout(awaitTail())).toBe("resolved");
563+
expect(log).toEqual(["outer"]);
564+
});

0 commit comments

Comments
 (0)