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

Ouverte
  • Quelque chose ne va pas ?
#4,680 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub

Les mainteneurs répondent en général sous 1 jour

Personne n'a encore pris cette issue.

Évaluation

Difficulté
4/5
Temps estimé
3-5 jours
Accessibilité débutants
45/100
Type d'issue
Bug
Clarté
Plutôt claire
Activité
Active
Stack technique
javascript, typescript
Domaine
frontend, performance

Piste de recherche

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.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Description

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.

Langage dominant
TypeScript
Étoiles
36.2k
Forks
1.1k
Merge moyen
8 h 30 min
PR mergées (30 j)
433

Préparer son environnement

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Autres issues de solidjs/solid

Toutes les issues de solidjs/solid

Issues similaires

Plus d'issues TypeScript

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.