cleanNode throws "observerSlots is null" when a source has observers but no slots

Aperta
  • Qualcosa non va?
#4,680 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
45/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
javascript, typescript
Ambito
frontend, performance

Direzione di ricerca

Start by reading cleanNode and readSignal, where the issue reports failures when observerSlots is null, and compare how they maintain observer and slot back-pointers. The report includes a hand-built reproduction that sets the field to null before disposal; use it to understand the failure, then decide whether recovery is appropriate. Done means establishing a safe behavior for this inconsistent state and covering it with a regression test; the issue does not name a test file.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

1.x
Describe the bug

In production, solid-js 1.9.17 threw TypeError: can't access property "pop", t.observerSlots is null inside cleanNode, at source.observerSlots.pop().

The update started in an async callback that runs batch(() => setStore("data", reconcile(rows, { key: "symbol" }))). An outer <For> removed a row. The row's nested <For> over columns then disposed its item roots in its onCleanup, and cleanNode threw on one of their sources. The nearest ErrorBoundary caught the error, so the whole route stopped rendering.

For cleanNode to throw there, a source must have a non-empty observers array while its observerSlots is null. A new read of that source also throws, on this.observerSlots.push() in readSignal.

We could not find a path in Solid that leaves a source in this state.

  • createSignal, createMemo and devComponent set both fields to null together.
  • readSignal writes both fields as a pair. Nothing between the two writes can throw or call user code.
  • cleanNode pops and swaps both arrays as a pair, with no user code in that loop.

We also wrote a seeded fuzzer that checks the observer and slot back-pointers after every step. It ran 3000 random graphs of 200 steps each on main (1.9.18) and found no violation. The graphs had memos and comparators that throw, catchError, roots disposed from computations and from onCleanup, store reconcile inside batch, nested mapArray with index signals, indexArray, createSelector and runWithOwner. They had no Transitions.

We saw the error once. The Firefox tab had been open for about 8 days. We suspect a fault outside Solid's JavaScript, but we can't prove it. We're filing this to ask whether you have seen it before, and whether you want cleanNode and readSignal to recover from this state.

Your Example Website or App

No public example. The public API can't reach this state, so the repro below sets it by hand.

Steps to Reproduce the Bug or Issue
import { createComputed, createRoot, createSignal, getOwner } from "solid-js";

const [count] = createSignal(0);
let observer: any;
const dispose = createRoot(dispose => {
  createComputed(() => {
    observer = getOwner();
    count();
  });
  return dispose;
});

observer.sources[0].observerSlots = null;
dispose();
  1. Run the snippet with solid-js 1.9.18.
  2. dispose() throws TypeError: Cannot read properties of null (reading 'pop').

If a new computation reads count after the observerSlots = null line, the read throws Cannot read properties of null (reading 'push') instead.

Expected behavior

One broken source should not crash the whole update. Disposing the observer should unsubscribe it, and later writes should still reach the other observers.

Platform
  • Browser: Firefox, version unknown
  • solid-js: 1.9.17 in production, reproduced by hand on 1.9.18
Additional context

We patched our copy of solid-js to rebuild the slots when they are missing. readSignal calls this function before it pushes, and cleanNode calls it before it pops. It only runs when observerSlots is null, so the normal path does not change.

function repairObserverSlots(source) {
  source.observerSlots = source.observers.map((observer, slot) => {
    const sources = observer.sources;
    if (!sources) return -1;
    for (let i = sources.length - 1; i >= 0; i--) {
      if (sources[i] === source && observer.sourceSlots[i] === slot) return i;
    }
    return sources.lastIndexOf(source);
  });
}

A bare null check is not enough. It would leave the disposed computation in observers, so the next write would run it again.

We can open a PR with this change and a test if you want it. We would also understand if you would rather not guard a state that Solid should never reach.

#2062 also fails inside cleanNode, but there the cause is re-entrant disposal, and the error comes from owned, not observerSlots.

Lingua principale
TypeScript
Stelle
36.2k
Fork
1.1k
Merge medio
9h 58m
PR unite (30g)
474

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di solidjs/solid

Tutte le issue di solidjs/solid

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.