[2.0 rc.14 / next] GRAPH_GROWTH reports a leak when a route's content differs per visit
Los mantenedores suelen responder en 1 día
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 4/5
- Tiempo estimado
- 3-5 días
- Aptitud para principiantes
- 45/100
- Tipo de issue
- Error
- Claridad
- Bien especificado
- Estado de actividad
- Activo
- Stack tecnológico
- typescript
- Área
- observability
Línea de trabajo
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.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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)
- Lenguaje dominante
- TypeScript
- Estrellas
- 36.1k
- Forks
- 1.1k
- Merge medio
- 10 h 42 min
- PR fusionados (30 d)
- 361
Preparar el entorno
- Sin Dockerfile ni archivo de Docker Compose
- Tiene una plantilla de pull request
- Leer la guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de solidjs/solid
-
Dificultad 4/5 3-5 días Aptitud para principiantes 20/100
Los mantenedores suelen responder en 1 día
-
Dificultad 4/5 3-5 días Aptitud para principiantes 15/100
Los mantenedores suelen responder en 1 día
-
bug
Dificultad 4/5 3-5 días Aptitud para principiantes 15/100
Los mantenedores suelen responder en 1 día
-
bug
Dificultad 4/5 3-5 días Aptitud para principiantes 25/100
Los mantenedores suelen responder en 1 día
-
Dificultad 5/5 Más de una semana Aptitud para principiantes 8/100
Los mantenedores suelen responder en 1 día
Todos los issues de solidjs/solid
Issues similares
-
Create/import wallet form shows raw "password must be at least 1 characters" instead of the translated messagePosiblemente ocupada @CapThunder19 la tomó hoy. Abiertobug
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
joinmarket-webui/jam#1538 ·
Los mantenedores suelen responder en 1 día
-
t0ggles
Dificultad 2/5 1-3 horas Aptitud para principiantes 70/100
nolimits4web/swiper#8240 · 1 comentario ·
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
modelcontextprotocol/inspector#2723 ·
Los mantenedores suelen responder en 1 día
-
status:approved type:bug
Dificultad 1/5 Menos de una hora Aptitud para principiantes 70/100
egdev6/quorb#109 · 1 comentario ·
Los mantenedores suelen responder en 1 día