Skip to content

Exp 288: two streams, one 29-bit key - #323

Open
danReynolds wants to merge 5 commits into
mainfrom
exp-288-stream-key-collision
Open

danReynolds wants to merge 5 commits into
mainfrom
exp-288-stream-key-collision

Conversation

@danReynolds

@danReynolds danReynolds commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Hypothesis

db.stream(sql, params) deduplicates: two calls with the same SQL and parameters share one StreamEntry, one SQLite query, and one re-run per write. While reading that registration path for churn cost I noticed what the registry was keyed on:

final Map<int, StreamEntry> _entries = {};
final key = Object.hash(sql, Object.hashAll(params));
if (_entries[key] case StreamEntry entry) return _subscribe(entry);

Object.hash is 29 bits wide — exp 033 wrote that down in April when it retired the function from result hashing — and nothing checked that the entry found under a hash actually had the same SQL and parameters. So any two distinct queries that happen to share a hash share a stream: the second subscriber is seeded from the first query's rows, re-runs the first query's SQL, and keeps receiving someone else's current rows for as long as the first stream stays open. Nothing errors and nothing looks stale, which is why it survived 287 experiments and every stream suite in the repo.

The pair is not exotic. Searching SELECT id, name, value FROM items WHERE id = ? over consecutive integer ids finds the first collision within a few tens of thousands of values. Two colliding streams have to be live at the same time, which keeps it rare — about n²/2³⁰ for n concurrent keyed streams, roughly one in 107,000 for 100 concurrent keyed streams — per screen, per open, across every install. StreamEntry also used the same key as its hashCode/==, so the engine's sets could not hold a colliding pair either, and 1 == 1.0 in Dart let an INTEGER and a REAL bind share a stream.

The bet: equality is what the hash was standing in for. Give the registry a key that hashes the same way and compares SQL and parameters structurally, and the bug closes with no change to how streams are dispatched, hashed or re-run.

Approach

_entries is now keyed by a private _StreamKey holding the SQL and parameters. It hashes with the same Object.hash(sql, Object.hashAll(params)) — so a colliding pair now reaches the equality check instead of the wrong entry — and compares SQL and parameters element-wise with ==, keeping int and double apart because they bind as different SQLite types. Parameter == is value equality for the bindable scalars and identity for blobs, exactly the distinctions the hash already drew, so everything that deduplicated before still does.

The key stored in the map takes its own fixed-length copy of the parameter list, since a caller's list could change after stream() returns; the entry's params (what every re-run binds) is now that copy too, so a mutated caller list can no longer change what a live stream re-runs. StreamEntry drops its hashCode/== override and falls back to identity, which is what its sets always meant. No public API change; doc/arch/architecture.md's StreamEngine paragraph is updated.

Two regression tests in test/stream_test.dart. The first finds a colliding pair of ids at test time using the engine's hash formula (Object.hash is seeded per process, so a pair cannot be hard-coded), inserts both rows, opens a stream on each, keeps both subscribed, and asserts each reports its own id with two entries registered. The second binds typeof(?) with 1 and 1.0 and expects integer and real.

Full detail in experiments/288-stream-key-collision.md.

Results

without fix with fix
colliding-key regression test fails — stream for id 27098 receives row 25336; 1 entry registered passes
int-vs-double regression test fails — the [1.0] stream reports integer; 1 entry registered passes
test/stream_test.dart 34 / 34
stream / database / write / transaction / reader-pool suites 167 / 167

The cost of the key in isolation (AOT, 100 live streams, no database): a dedup hit goes from 35 ns to 106 ns and a miss-then-register from 38 ns to 75 ns. A hit then allocates a StreamController; a miss then pays the ~3.3 µs reader-pool round trip (claim 282.2) before any SQLite work — so the check is two digits of nanoseconds against a microsecond floor and not visible from any public lane.

Headline release run (run_release.dart exp288-stream-key-collision --repeat=5 --fail-on-regression --fail-on-memory-regression, from the committed tree, compared explicitly against the last release anchor 2026-08-09T20-36-47-exp266-headline-refresh.md — auto-compare would have picked a focused-harness markdown without a sidecar and skipped): exit 0, 0 wins / 0 regressions / 169 neutral. Every streaming lane this change could touch is flat, including both column-granularity rows (resqlite re-emits 0 disjoint / 10 overlapping, identical to the anchor). The comparison's two flagged re-emit rows (−134 / +673) belong to the sqlite_async peer. check_peer_drift.dart --since=2026-08-09 reads −2.6% median / 71% agreement across the month — within tolerance. The anchor had no memory section, so the memory gate had no baseline; this run records values for the next one.

Outcome

Accepted: a silent wrong-rows bug in the library's primary feature, closed at the registry with no public API change and no measurable cost. Going forward, any registry in the runtime keyed on a hash must carry the equality it stands in for — the audit for this run found this was the only bare-hash key (the C statement cache compares SQL bytes; the Dart schema, row-size and SQL-UTF-8 caches key on the String).

Also recorded for future runners: Database.open was probed as a cold-start candidate and is not one — 0.69 ms to open plus 0.16 ms to the first read in AOT (claim 288.3).

Test plan

  • dart analyze --fatal-infos on lib/ and the touched tests
  • dart test test/stream_test.dart (34, including the two new regression tests, which fail on the pre-fix engine)
  • dart test across the stream, database, write-coalescing, trigger-cascade, reader-pool, transaction, dependency-shape, cache-hit, invalidation-coalescing, overflow-fallback, encryption and stmt-cache-pressure suites (167)
  • check_knowledge_links.dart, check_experiment_signals.dart, check_experiment_dispositions.dart, tool/knowledge/impact.dart
  • headline release run (run_release.dart --repeat=5 --fail-on-regression --fail-on-memory-regression) — see Results
  • finalize_experiment.dart

danReynolds and others added 4 commits September 10, 2026 18:49
`StreamEngine._entries` was keyed on the bare
`Object.hash(sql, Object.hashAll(params))` with no equality check, so two
distinct queries sharing a 29-bit hash shared one StreamEntry and the second
subscriber received the first query's rows. Key the registry by a structural
`_StreamKey` (same hash, real equality, int and double kept apart), give the
stored key its own copy of the parameter list, and let StreamEntry fall back
to identity equality. Two regression tests find a colliding pair at run time.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@danReynolds danReynolds added type: correctness Correctness guard / public-API audit; no performance claim approved Experiment succeeded: a kept win or a passing guard labels Sep 10, 2026
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Belief impact

Learned

  • 288.1 · Two streams, one 29-bit key
    StreamEngine._entries keyed on the bare 29-bit Object.hash(sql, Object.hashAll(params)) aliased distinct queries: two (sql, params) pairs sharing…
  • 288.2 · Two streams, one 29-bit key
    The structural stream key costs +70 ns on a dedup hit (35 -> 106 ns) and +37 ns on a miss including the fixed-length parameter copy the stored key ta…
  • 288.3 · Two streams, one 29-bit key
    Database.open on an existing file costs ~0.69 ms median (0.61 ms min) in AOT and the first select() issued immediately after it completes ~0.16 m…

What this changed

Stream deduplication was believed to share an entry only between calls with the same SQL and parameters. It shared an entry between any two calls whose 29-bit Object.hash(sql, Object.hashAll(params)) collided, with no equality check behind the map, so the second of a colliding pair received the first query's rows — current rows for the wrong query — for as long as the first stream lived. Exp 033 had already identified Object.hash as 29-bit when it retired it from result hashing; the registry key was never revisited. The registry now compares SQL and parameters structurally, StreamEntry is identity-equal, and an int and a double parameter no longer alias. doc/arch/architecture.md's StreamEngine lifecycle paragraph is updated; no other documented passage rested on the old key.

The class of bug is worth naming: a lookup keyed on a hash with nothing standing behind it is correct until the population is large enough to collide, and the failure — self-consistent rows for a neighbouring key — is invisible to the subscriber. This was the only bare-hash key in the runtime; the C statement cache compares SQL bytes and the Dart schema, row-size and SQL-UTF-8 caches key on the String itself.

Database.open is not a cold-start target: on this host an AOT open returns in ~0.69 ms and the first read after it completes ~0.16 ms later, so the temporary open isolate, the four-reader pool spawn and the writer spawn together cost under a millisecond per process. No lane measures it and none needs to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Experiment succeeded: a kept win or a passing guard codex codex-automation type: correctness Correctness guard / public-API audit; no performance claim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant