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

オープン
  • 何か問題がありますか？
#4,680 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
45/100
issue の種類
バグ
明瞭さ
おおむね明確
活発さ
活発
技術スタック
javascript, typescript
領域
frontend, performance

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

説明

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.

主要言語
TypeScript
スター
36.2k
フォーク
1.1k
平均マージ
9時間 56分
マージ済み PR（30日）
487

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

solidjs/solid のほかの issue

solidjs/solid の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。