Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 45 additions & 27 deletions packages/signals/tests/action-completion-race.test.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,43 @@
// #2916: when an action's done() restores activeTransition (without adopting
// the ambient batch) and the shared transition is still incomplete, an
// ordinary write in the microtask window before the scheduled flush lands in
// the detached ambient batch. The incomplete-transition stash then replaced
// that batch wholesale, stranding the queued pending node: the write never
// committed and every later write to the same signal stayed frozen (dev
// INV-7).
// #2916: a write between action completion and a pending scheduler flush must
// commit and leave the signal writable, even while another action is open.
// The original stranded pending node violated INV-7 on the next flush.

import { describe, expect, it } from "vitest";
import { afterEach, describe, expect, it, vi } from "vitest";
import { action, createSignal, flush } from "../src/index.js";
import * as scheduler from "../src/core/scheduler.js";

const tick = () => Promise.resolve();
afterEach(() => vi.restoreAllMocks());

// Hold scheduler flushes while Promise callbacks complete the action.
// Await its result, inject the operation, then release the held callbacks.
// The callbacks can come from joining or completing a transaction.
async function inDoneWindow(done: Promise<void>, complete: () => void, operation: () => void) {
const queued: VoidFunction[] = [];
const events: string[] = [];
const microtask = vi.spyOn(globalThis, "queueMicrotask").mockImplementation(callback => {
queued.push(() => {
events.push("flush");
callback();
});
});
try {
complete();
await done;
events.push("done");
expect(queued.length).toBeGreaterThan(0);
expect(events).toEqual(["done"]);
operation();
events.push("operation");
expect(events).toEqual(["done", "operation"]);
while (queued.length) queued.shift()!();
expect(events[2]).toBe("flush");
} finally {
microtask.mockRestore();
while (queued.length) queued.shift()!();
}
}

describe("post-action completion race (#2916)", () => {
it("commits an ambient write made between an action's done() and its scheduled flush", async () => {
it("commits an ambient write after an action completes and before a pending flush", async () => {
const [y, setY] = createSignal(0);

let resolveA!: () => void;
Expand All @@ -32,23 +56,17 @@ describe("post-action completion race (#2916)", () => {

const aDone = A();
const bDone = B();
flush(); // stash the shared incomplete transition

resolveA();
flush();
await Promise.resolve(); // drain the initial scheduled flush before interception

// Write in the window where A's done() has restored activeTransition but
// its scheduled flush has not run. The internal read only makes the
// timing deterministic; the write itself is an ordinary application
// write.
let wrote = false;
for (let i = 0; i < 16; i++) {
await tick();
if (!wrote && scheduler.activeTransition !== null) {
wrote = true;
setY(7);
}
}
expect(wrote).toBe(true);
let bCompleted = false;
void bDone.then(() => {
bCompleted = true;
});
await inDoneWindow(aDone, resolveA, () => {
expect(bCompleted).toBe(false);
setY(7);
});

resolveB();
await Promise.all([aDone, bDone]);
Expand Down
109 changes: 55 additions & 54 deletions packages/signals/tests/action-done-window.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,6 @@
/**
* The post-action done() window (#2916 shape): an async-generator action's
* done() runs from an iterator-result microtask, restoring activeTransition
* with no synchronous flush after it. Until the scheduled flush runs,
* globalQueue._batch was a detached ambient batch — so anything registered in
* that window (ordinary writes held by a merged transition, optimistic
* overrides, affects() marks) landed in a batch that nothing ever finalized.
* done() now re-adopts the batch through initTransition, the same path every
* other transition-resumption site uses.
*
* Each test polls microtasks until it observes the restored transition
* (scheduler.activeTransition !== null) and injects its work exactly there.
*/
import { describe, expect, it } from "vitest";
// #2916: inject work after an async action completes but before a pending
// flush. Assert both the ordering and the eventual write/revert/release.
import { afterEach, describe, expect, it, vi } from "vitest";
import {
action,
affects,
Expand All @@ -23,10 +12,40 @@ import {
flush,
isPending
} from "../src/index.js";
import * as scheduler from "../src/core/scheduler.js";

const tick = () => Promise.resolve();

afterEach(() => vi.restoreAllMocks());

// Hold scheduler flushes while Promise callbacks complete the action.
// Await its result, inject the operation, then release the held callbacks.
// The callbacks can come from joining or completing a transaction.
async function inDoneWindow(done: Promise<void>, complete: () => void, operation: () => void) {
const queued: VoidFunction[] = [];
const events: string[] = [];
const microtask = vi.spyOn(globalThis, "queueMicrotask").mockImplementation(callback => {
queued.push(() => {
events.push("flush");
callback();
});
});
try {
complete();
await done;
events.push("done");
expect(queued.length).toBeGreaterThan(0);
expect(events).toEqual(["done"]);
operation();
events.push("operation");
expect(events).toEqual(["done", "operation"]);
while (queued.length) queued.shift()!();
expect(events[2]).toBe("flush");
} finally {
microtask.mockRestore();
while (queued.length) queued.shift()!();
}
}

describe("post-action done() window", () => {
it("a completed action's write survives another action resuming in its done-window", async () => {
const [x, setX] = createSignal(0);
Expand Down Expand Up @@ -55,23 +74,19 @@ describe("post-action done() window", () => {
const aDone = A();
flush(); // stash T_A
await tick();
const bDone = B(); // fresh transition T_B (T_A stashed, activeTransition null)
const bDone = B(); // B remains open until its controlled thenable resumes.
flush(); // stash T_B

resolveA();

// Land in A's done-window and resume B there, so initTransition(T_B)
// merges the restored T_A into T_B. T_A's held write must survive the
// merge and commit when T_B settles.
let resumed = false;
for (let i = 0; i < 16; i++) {
await tick();
if (!resumed && scheduler.activeTransition !== null && hasResumeB) {
resumed = true;
resumeB(undefined);
}
}
expect(resumed).toBe(true);
await tick(); // drain the initial scheduled flushes
let bCompleted = false;
void bDone.then(() => {
bCompleted = true;
});
await inDoneWindow(aDone, resolveA, () => {
expect(hasResumeB).toBe(true);
expect(bCompleted).toBe(false);
resumeB(undefined);
});

await Promise.all([aDone, bDone]);
await new Promise(r => setTimeout(r, 0));
Expand Down Expand Up @@ -103,17 +118,10 @@ describe("post-action done() window", () => {
const aDone = A();
flush();

resolveA();

let wrote = false;
for (let i = 0; i < 16; i++) {
await tick();
if (!wrote && scheduler.activeTransition !== null) {
wrote = true;
setOpt(5);
}
}
expect(wrote).toBe(true);
await tick(); // drain the initial scheduled flush
await inDoneWindow(aDone, resolveA, () => {
setOpt(5);
});

await aDone;
await new Promise(r => setTimeout(r, 0));
Expand Down Expand Up @@ -143,25 +151,18 @@ describe("post-action done() window", () => {
const aDone = A();
flush();

resolveA();

let marked = false;
for (let i = 0; i < 16; i++) {
await tick();
if (!marked && scheduler.activeTransition !== null) {
marked = true;
affects(count);
}
}
expect(marked).toBe(true);
await tick(); // drain the initial scheduled flush
await inDoneWindow(aDone, resolveA, () => {
affects(count);
});

await aDone;
await new Promise(r => setTimeout(r, 0));
flush();
flush();

// The mark now belongs to the restored transaction and releases at its
// settle; before the fix it landed in the detached ambient batch, where
// The mark must release at settle; before the fix it landed in a
// detached ambient batch, where
// (in combination with other pending work) it could leak forever
// (isPending stuck true, INV-10 on the next quiescent flush).
expect(isPending(() => count())).toBe(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function deferred<T = void>() {
}

function board(read: (drag: () => string | undefined, cards: Card[]) => unknown) {
const gates = new Map<string, ReturnType<typeof deferred>>();
const gates = new Map<string, ReturnType<typeof deferred<void>>>();
let move!: (id: string, column: number) => Promise<void>;
let setDrag!: (v: string) => void;
let drag!: () => string | undefined;
Expand Down
2 changes: 1 addition & 1 deletion packages/signals/tests/attribution-navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ describe("interaction — a router that awaited before writing hands the click b
const [location, setLocation] = createSignal("/a", { name: "location" });
createRoot(() => createEffect(location, () => {}, { name: "reader" }));
flush();
let first: ReturnType<typeof OBSERVE.attribution.currentOrigin>;
let first: ReturnType<NonNullable<typeof OBSERVE>["attribution"]["currentOrigin"]>;
OBSERVE!.attribution.withInteraction({ type: "click", target: "a.first" }, () => {
first = OBSERVE!.attribution.currentOrigin();
});
Expand Down
5 changes: 3 additions & 2 deletions packages/signals/tests/createRoot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {
refresh,
type Accessor,
type Owner,
type Signal
type Signal,
type SourceAccessor
} from "../src/index.js";

afterEach(() => flush());
Expand Down Expand Up @@ -48,7 +49,7 @@ it("should not resurrect a dependency-free computation queued before disposal (#
// resurrecting the node (post-unmount runs, leaked cleanups).
let runs = 0;
let cleanups = 0;
let read!: Accessor<number>;
let read!: SourceAccessor<number>;
let dispose!: () => void;

createRoot(d => {
Expand Down
5 changes: 3 additions & 2 deletions packages/signals/tests/diagnostics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ describe("diagnostics", () => {
const capture = OBSERVE!.diagnostics.capture();

createRoot(() => {
expect(() => createEffect(() => 1)).toThrow(
// Deliberately omit the required effect callback to exercise the runtime guard.
expect(() => (createEffect as unknown as (compute: () => number) => void)(() => 1)).toThrow(
/createEffect requires both a compute function and an effect function/
);
});
Expand Down Expand Up @@ -279,7 +280,7 @@ describe("diagnostics console footer", () => {
expect(url).toMatch(
/^https:\/\/github\.com\/solidjs\/solid\/blob\/main\/.*SKILL\.md#strict_read_untracked$/
);
expect(DEV!.guideUrl("WIDE_WRITE")).toBe(url.replace(/#.*$/, "#wide_write"));
expect(DEV!.guideUrl("SILENT_HOLD")).toBe(url.replace(/#.*$/, "#silent_hold"));
});

afterEach(() => {
Expand Down
4 changes: 2 additions & 2 deletions packages/signals/tests/effect-mainline-ownership-3412.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ function setup(opts: { boundary: boolean; on: boolean; unconditional: boolean })
const [show, _setShow] = createSignal(true);
setCount = _setCount;
setShow = _setShow;
const copy = createMemo(async () => count(), undefined, { name: "copy" });
const details = createMemo(() => delay(1500, copy()), undefined, { name: "details" });
const copy = createMemo(async () => count(), { name: "copy" });
const details = createMemo(() => delay(1500, copy()), { name: "details" });
createRenderEffect(
() => String(show()),
v => {
Expand Down
17 changes: 11 additions & 6 deletions packages/signals/tests/flatten-async-iterable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,25 +255,30 @@ describe("promise-of-AsyncIterable flattening (deferred posture)", () => {
});

it("surfaces a stream error through the memo", async () => {
const errors: unknown[] = [];
const failure = new Error("stream boom");
const gate = deferred();
let memo!: () => number;
const failing: AsyncIterable<number> = {
[Symbol.asyncIterator]: () => ({
next: () => Promise.reject(new Error("stream boom"))
next: () => Promise.reject(failure)
})
};
createRoot(() => {
memo = createMemo(() => gate.promise.then(() => failing) as unknown as number);
createEffect(
() => memo(),
() => {},
{ error: () => {} }
);
createEffect(() => memo(), {
effect: () => {},
error: error => {
errors.push(error);
}
});
});
flush();
gate.resolve();
await tick();
flush();
expect(errors).toHaveLength(1);
expect(errors[0]).toBe(failure);
expect(() => memo()).toThrow("stream boom");
});

Expand Down
20 changes: 14 additions & 6 deletions packages/signals/tests/gc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,13 @@ if (global.gc) {
ref!: WeakRef<any>;

const dispose = createRoot(dispose => {
createEffect($x, () => {
ref = new WeakRef(getOwner()!);
});
createEffect(
() => {
ref = new WeakRef(getOwner()!);
return $x();
},
() => {}
);

return dispose;
});
Expand Down Expand Up @@ -159,9 +163,13 @@ if (global.gc) {
for (const mode of ["latest", "isPending"] as const) {
it(`releases an obsolete leaf value after a ${mode}() reader is disposed (#3503)`, async () => {
const fixture = createRoot(dispose => {
const [state, setState] = createStore(() => {}, {} as { value?: object }, {
shallow: true
});
const [state, setState] = createStore<{ value?: object }>(
() => {},
{},
{
shallow: true
}
);
return { state, setState, dispose };
});
const ref = (() => {
Expand Down
Loading
Loading