[2.0 rc.14 / next] GRAPH_GROWTH reports a leak when a route's content differs per visit
メンテナーはふだん 1 日以内に返信
まだ誰も着手していません。
評価
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 初心者へのやさしさ
- 45/100
- issue の種類
- バグ
- 明瞭さ
- 明確に書かれている
- 活発さ
- 活発
- 技術スタック
- typescript
調査の方向性
Start with packages/signals/src/core/attribution.ts, especially trackGraph() and the existing graph-growth tests; the issue describes the proposed survivor-counting approach and reports eight new tests for variants A–F and related cases. Run the graph-growth tests to inspect the current behavior. Done means changing the diagnostic so growing page content alone stays quiet while the leak cases report, with the reported history and documentation updated.
索引モデルが issue の本文から書いたものです。
説明
Summary
GRAPH_GROWTH compares the whole live graph at each arrival on a route pattern, so a page whose correctly owned content is bigger on each visit reads as a leak. Three visits to /rows/:count with 1, 3, then 5 rows report it, although leaving the route disposes its rows and the route in between settles at exactly the same size each time. The message still says, “Each visit left something behind that the next did not reclaim.” Also requiring what outlived each navigation to climb tells the two apart, and it still reports a root each visit leaves behind, even when the page's content shrinks. This reproduces on published solid-js / @solidjs/signals 2.0.0-rc.14 and on next at 6232d18e.
Reproduction
No DOM or router package is needed: a memo owns the current page's rows, a render effect reads them, and OBSERVE.attribution.withOrigin declares navigations through the public API.
{
"private": true,
"type": "module",
"dependencies": { "solid-js": "2.0.0-rc.14" }
}
Save as repro.ts:
import assert from "node:assert/strict";
import {
OBSERVE,
createMemo,
createRenderEffect,
createRoot,
createSignal,
flush,
runWithOwner,
} from "solid-js";
import { attribution, graphSize } from "solid-js/attribution";
// A: growing content, fixed /rest between visits.
// B: A plus an undisposed root per content visit.
// C: growing content, no other route between visits.
// D: both routes grow their owned content, without a leak.
// E: content visits leak, but intervening content shrinks.
// F: content visits leak, but their own content shrinks.
const variant = process.argv[2] ?? "A";
assert.ok(["A", "B", "C", "D", "E", "F"].includes(variant));
const leak = ["B", "E", "F"].includes(variant);
attribution.enable({ log: false });
const settles: unknown[] = [];
const findings: unknown[] = [];
const offGraph = OBSERVE!.records.subscribe("graph", (event) => {
const { roots, owners, computations, signals, edges } = event;
const size = { roots, owners, computations, signals, edges };
assert.deepEqual(size, graphSize());
settles.push({ route: event.navigation.to, ...size });
});
const offDiagnostic = OBSERVE!.diagnostics.subscribe((event) => {
if (event.code === "GRAPH_GROWTH") findings.push(event.data);
});
const leaked: (() => void)[] = [];
const app = createRoot((dispose) => {
const [location, setLocation] = createSignal({ rows: false, count: 0 });
const [shared] = createSignal(1);
const page = createMemo(() => {
const current = location();
const n = current.count;
if (n === 0) return shared;
if (current.rows && leak) {
runWithOwner(null, () =>
createRoot((dispose) => {
// Retained across navigation; cleaned up only after measurement.
leaked.push(dispose);
createRenderEffect(shared, () => {});
}),
);
}
const rows = Array.from({ length: n }, () => createMemo(shared));
return () => rows.reduce((sum, row) => sum + row(), 0);
});
createRenderEffect(
() => page()(),
() => {},
);
return { setLocation, dispose };
});
flush();
for (const [index, n] of (variant === "F" ? [5, 3, 1] : [1, 3, 5]).entries()) {
OBSERVE!.attribution.withOrigin(
{
kind: "navigation",
name: "/rows/:count",
to: `/rows/${n}`,
params: { count: String(n) },
},
() => app.setLocation({ rows: true, count: n }),
);
flush();
if (variant !== "C") {
const other =
variant === "D" ? n : variant === "E" ? [101, 51, 1][index]! : 0;
OBSERVE!.attribution.withOrigin(
{
kind: "navigation",
name: other ? "/other/:count" : "/rest",
to: other ? `/other/${other}` : "/rest",
},
() => app.setLocation({ rows: false, count: other }),
);
flush();
}
}
app.dispose();
for (const dispose of leaked) dispose();
flush();
const afterDispose = graphSize();
assert.deepEqual(afterDispose, {
roots: 0,
owners: 0,
computations: 0,
signals: 0,
edges: 0,
});
offGraph();
offDiagnostic();
attribution.disable();
console.log(
JSON.stringify({ variant, settles, findings, afterDispose }, null, 2),
);
Install with pnpm install. Run each variant in a fresh process, with Node's native TypeScript support:
for variant in A B C D E F; do
node --conditions=browser --conditions=development repro.ts "$variant"
done
For next, the same program was run against packages/solid and its workspace-linked packages/signals at 6232d18e, built with their own pnpm run build scripts. Both published packages in the stock run resolved to RC.14.
Each tuple below is (roots, owners, computations, signals, edges). The counts were identical on all three builds in the second table; only the verdict changed. Each row visit is followed by its intervening visit, except in C.
| Variant | /rows/:count settles, in order |
Intervening settles, in order |
|---|---|---|
| A: content only | (1,4,3,2,4) → (1,6,5,2,8) → (1,8,7,2,12) | /rest: (1,3,2,2,3), identical three times |
| B: a leak | (2,6,4,2,5) → (3,10,7,2,10) → (4,14,10,2,15) | /rest: (2,5,3,2,4) → (3,7,4,2,5) → (4,9,5,2,6) |
| C: content only, consecutive parameter changes | Same as A | None |
| D: both routes' content grows | Same as A | /other/1, /other/3, /other/5: same tuples as A's row visits |
| E: a leak, intervening content shrinks | Same as B | /other/101, /other/51, /other/1: (2,106,104,2,205) → (3,58,55,2,106) → (4,10,6,2,7) |
| F: a leak, its own content shrinks (5, 3, 1) | (2,10,8,2,13) → (3,10,7,2,10) → (4,10,6,2,7) | /rest: same as B |
| Build | A | B | C | D | E | F |
|---|---|---|---|---|---|---|
| Published RC.14 | 1 | 1 | 1 | 1 | 1 | 1 |
next at 6232d18e |
1 | 1 | 1 | 1 | 1 | 1 |
Same next with the possible fix below |
0 | 1 | 0 | 0 | 1 | 1 |
Every finding names /rows/:count, except F's, which names /rest on all three builds: F's row visits stay at 10 owners while its leaked roots pile up. All eighteen runs pass the graph-record consistency and final-disposal assertions. B, E and F intentionally retain their detached roots throughout navigation, disposing them only after measurement.
Cause
In packages/signals/src/core/attribution.ts, trackGraph() stores the whole graph's size at each settle of a route pattern and checks for strictly climbing counts that reach a ratio:
history.push(size);
if (history.length > cfg.visits) history.shift();
if (history.length < cfg.visits) return;
const grew = GRAPH_SERIES.filter((key) => {
for (let i = 1; i < history!.length; i++)
if (history![i][key] <= history![i - 1][key]) return false;
return history![history!.length - 1][key] >= history![0][key] * cfg.ratio;
});
The counts are correct: in A, the current page really has more rows. But a count taken at the arrival includes everything the visit just created, so it cannot tell content that differs per visit from something earlier visits left behind. In A, /rest settles at the same complete size, edges included, every time.
Expected
Correctly owned content that differs per visit should not report; B, E and F should.
This differs from #3989, fixed by #3994: these navigations are synchronous, and the graph records equal the settled graph.
Possible fix
Also require what outlived each navigation to climb. Tested on next at 6232d18e:
- When a non-initial navigation opens (only while
graphGrowthis on), the engine walks the graph once more and adds every node it meets to aWeakSet. - At that navigation's settle, the same walk also counts the nodes from that set that are still alive (
survived; an edge counts when both its ends are), and drops the set. - A series reports only when it climbs (strictly on every visit, to
ratio) both in the totals, as today, and insurvived. A visit's own content is not insurvived, so A, C and D stay quiet. A root, an ownerless effect or a retained signal that a visit leaves behind is alive when the next navigation starts and survives it, whatever the content does: B and E report at/rows/:count, and F at/rest, where both climb. Since it only adds a condition, the fix never reports more than today. - A navigation that stays on the route pattern (only its params or query change) continues the visit and isn't compared: the page it kept outlived the move, so counting it would read the page itself as growth. Only arrivals that enter the route are compared, and a leak made during a visit is still alive when the next visit starts, so it still shows.
data.survivedsits besidedata.history, and the message shows both, e.g.owners 3 → 5 → 7 (of 6 → 10 → 14).
Nothing is charged at node creation or disposal; the cost is a second walk per navigation, observe tier only.
The graph-growth tests all pass: the 11 existing ones, both #3994 held-arrival cases among them, plus 8 new ones. The new ones are A to F as unit tests; a signal kept per visit and read by a long-lived computation, which reports signals and edges; and a page kept across navigations that stay on its route while it grows a row per query asked, which today's rule also reports. The 8 new tests fail without the change. The whole signals suite passes: 5,184 tests, plus 22 expected failures and 2 skipped. The last row of the table above is this build. The change also updates 08-dev-diagnostics.md, the skill's GRAPH_GROWTH section, and adds a changeset.
A leak on only every other visit doesn't climb on every visit, and the fix doesn't report it. Published RC.14 doesn't report it with constant content either (3 rows per visit), only when the content also grows (1, 3, 5, 7, 9 rows).
The check in trackGraph() becomes:
const grew = GRAPH_SERIES.filter((key) => climbs(sizes, key) && climbs(survivors, key));
(submitted by Claude Opus 5.5 on behalf of rvlzzr)
- 主要言語
- TypeScript
- スター
- 36.1k
- フォーク
- 1.1k
- 平均マージ
- 12時間 10分
- マージ済み PR(30日)
- 338
環境構築
- Dockerfile・Docker Compose ファイルなし
- プルリクエストのテンプレートあり
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
solidjs/solid のほかの issue
-
`lazy()`: remounting an already-loaded component renders empty (or the Suspense fallback) since 1.9.14対応中かも @itisrmk が 1 日前に担当しました。 オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 62/100
メンテナーはふだん 1 日以内に返信
-
難易度 4/5 3〜5日 初心者へのやさしさ 18/100
メンテナーはふだん 1 日以内に返信
-
難易度 4/5 3〜5日 初心者へのやさしさ 22/100
メンテナーはふだん 1 日以内に返信
-
難易度 5/5 1週間以上 初心者へのやさしさ 14/100
メンテナーはふだん 1 日以内に返信
-
難易度 4/5 3〜5日 初心者へのやさしさ 35/100
メンテナーはふだん 1 日以内に返信
似ている issue
-
documentation
難易度 2/5 1〜3時間 初心者へのやさしさ 74/100
components-web-app/docs#260 ·
-
難易度 1/5 1時間未満 初心者へのやさしさ 65/100
521xueweihan/HelloGitHub#4025 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
ctrf-io/playwright-ctrf-json-reporter#117 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
voidzero-dev/void#478 ·
メンテナーはふだん 1 日以内に返信