fix(runtime): make a DataView read the backing store, not its construction snapshot - #8083
Merged
proggeramlug merged 2 commits intoAug 14, 2026
Merged
Conversation
…ction snapshot A DataView did not see writes made through a multi-byte typed array over the same ArrayBuffer. `new Uint32Array(ab)` then `w[0] = 0x01020304` left `new DataView(ab)` reading the bytes present when the DataView was built: `dv.getUint8(0..3)` summed to 0 where node says 10. No throw, no null — just numbers from before the write. Root cause. `js_data_view_new` (buffer/from.rs:894) gives the DataView its own BufferHeader, copies the window's bytes in, and registers it in the buffer view registry. That local copy is refreshed only by writes routed THROUGH the registry (js_buffer_set, js_buffer_write, a sibling DataView's set*). A Uint16Array/Uint32Array/Float64Array element store does not go through it: typedarray_view::register_view_meta makes typedarray::data_ptr_mut resolve straight into the backing ArrayBuffer, so the store lands there with nothing mirroring it into the DataView. `read_bytes` in buffer/dataview.rs then read the DataView's inline bytes and returned the snapshot. The fix is one line: read_bytes resolves through view::resolve_data_ptr, the canonical view-resolving accessor read_buffer_byte has used for Uint8Array/Buffer receivers since PerryTS#1205 and every native-span consumer since PerryTS#6515. Writes were already correct — write_bytes mirrors into the backing via propagate_written_range_from_receiver — which is why the bug was direction-specific. The DataView keeps its own storage (codegen still geps against a buffer pointer, and write_bytes still writes it); only which copy is AUTHORITATIVE on the read changed. Three things made it read like something else, all now covered by tests: element width looked like the trigger (a Perry Uint8Array over an ArrayBuffer is a BufferHeader view whose writes DO mirror, so only the TypedArrayHeader kinds bypassed the registry); construction order looked like the trigger (writing before `new DataView(ab)` worked, because the constructor copies); and buffer identity was already correct (w.buffer === dv.buffer, byteOffset, byteLength). Fixed with it, because it is the same reasoning: TextDecoder.decode(dataView) built its byte slice as `buf + sizeof(BufferHeader)` under a comment asserting the bytes are "stored inline". They are not, for a registered view. decode(dv) of a buffer a Uint32Array had just written returned "\0\0\0\0" where node returns "ABCD", while decode(ab) on the same buffer was correct. It now resolves through buffer::resolve_span_data_ptr, which also covers the Buffer.from(ab) / subarray receivers sharing that branch. Testing - test-files/test_gap_dataview_buffer_aliasing_7219.ts — byte-exact against node 26.5.1 across every element width, both directions, DataView built before AND after the writes, a windowed `new DataView(ab, 4, 8)`, getInt16/getInt32/getFloat64 with and without the little-endian flag, two DataViews over one buffer, a DataView over a typed array's materialized .buffer, module scope as well as function scope, and a loop-carried mix. 8 of its 11 lines were wrong at base. - Three cargo-test-visible unit tests, each watched fail with its fix reverted: buffer::tests::data_view_reads_multi_byte_typed_array_writes, buffer::tests::multi_byte_typed_array_reads_windowed_data_view_writes ("DataView byte 0 lags the typed-array write: left 0.0, right 2.0"), and text::tests::text_decoder_reads_backing_store_of_a_data_view (left "\0\0\0\0", right "ABCD"). Each asserts the typed-array store actually reached the ArrayBuffer first, so a store that never landed cannot pass them vacuously (0 == 0). - RUST_TEST_THREADS=1 cargo test --release -p perry-runtime: 2317 passed, 0 failed. rustfmt and scripts/check_file_size.sh clean. Left unfixed (pre-existing, unrelated to the byte-read path): a DataView over a detached buffer throws RangeError where node throws TypeError; resizable ArrayBuffers are unimplemented; `dv instanceof DataView` is false; and Array.prototype.join formats a denormal (1.3597e-320) as a full decimal expansion instead of exponential. Also verified NOT a regression: test_gap_dataview_2878_2877_2879.ts diverges from node on three TypedArray.prototype.copyWithin lines (Int16Array/Float64Array) on macOS. Byte-identical output before and after this change, so it is pre-existing and unrelated; the test is absent from test-parity/gap_snapshot.json (the Linux baseline), so it is either macOS-specific or a stale snapshot entry.
changelog.d/README.md keys fragments on the PR number so two in-flight PRs can never collide on one filename.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughDataView and TextDecoder reads now resolve current bytes from shared ChangesBuffer aliasing reads
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The PR corrects DataView and TextDecoder reads to observe the shared backing store, with the described tests passing. No actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
proggeramlug
approved these changes
Aug 14, 2026
proggeramlug
left a comment
proggeramlug
left a comment
Contributor
There was a problem hiding this comment.
Approved at exact head 30877d7eab84d005a612b8e54394bc1fcf2aadee. The read paths now use the existing canonical backing/window resolver; the three focused runtime regressions passed independently (DataView after multi-byte typed-array writes, windowed reverse direction, and TextDecoder over DataView). Formatting and landing diff are clean. No CI result used.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A
DataViewdoes not see writes made through aUint16Array,Uint32ArrayorFloat64Arrayover the sameArrayBuffer. It reads the bytes that existed when it was constructed.TextDecoder.decode(dataView)has the same bug.The write really does land in the shared buffer — a
Uint8Arrayreader over the sameArrayBuffersees it. Only the DataView misses it.Root cause
js_data_view_newgives every DataView its ownBufferHeaderand copies the window's bytes into it. That copy is refreshed only by writes routed through the buffer-view registry:js_buffer_set,js_buffer_write, a sibling DataView'sset*.A multi-byte typed-array element store does not go through the registry.
register_view_metamakestypedarray::data_ptr_mutresolve straight into the backingArrayBuffer, so the store lands there and nothing mirrors it into the DataView's private copy.read_bytes(crates/perry-runtime/src/buffer/dataview.rs) read that private copy. The registry edge to the backing store was already there —read_buffer_bytehas followed it forUint8Array/Bufferreceivers since #1205.read_bytessimply never used it.Why it looked like an element-width bug, and why it isn't
The trigger is not width, it is which header the writer gets. A Perry
Uint8Arrayover anArrayBufferis aBufferHeaderview, and its writes go through the registry, so they mirror.Uint16Array/Uint32Array/Float64ArrayareTypedArrayHeaderkinds that bypass it.That also explains the other bounds:
--disable-buffer-fast-pathdid not rescue it.Changes
crates/perry-runtime/src/buffer/dataview.rsread_bytesresolves throughresolve_data_ptrinstead of reading its own inline bytescrates/perry-runtime/src/text.rsTextDecoder.decode(dv)built its slice asbuf + sizeof(BufferHeader)under a comment asserting the bytes were "stored inline" — the same stale readcrates/perry-runtime/src/buffer/mod.rs,text.rstest-files/test_gap_dataview_buffer_aliasing_7219.tsThe construction-time copy is deliberately not removed: codegen still
geps against buffer pointers andwrite_bytesstill writes it. Only which copy is authoritative on the read changed.Test plan
Every case byte-compared against node 26.5.1, the version
.node-versionpins.1,2,3,42,1,4,3/4,3,2,1/…248,634,3,2,1 | 8,7,6,5new DataView(ab, 4, 8)4,3,2,1,8,7,6,5getInt16/32/Float64± the LE flag258,513,-50462977,…61374,613740,04,3,2,1w.buffer === dv.buffer, byteOffset/byteLengthtrue,true,4,4,4.buffer-materialized,Buffer.from(ab), subarrayTextDecoder.decode(dv)ABCD\0\0\0\0RUST_TEST_THREADS=1 cargo test --release -p perry-runtime→ 2317 passed, 0 failed. (That flag is required — the runtime's tests share process-global side tables and are not parallel-safe.)test_gap_6386_dataview_concat_regex_fastpaths.tsstill matches. rustfmt,check_file_size.shand the addr-class ratchet are clean.Verified by sabotage, each fix reverted independently:
Each test first asserts the typed-array store actually reached the
ArrayBuffer, so a store that never landed cannot pass it vacuously on0 == 0. The windowed test's DataView→typed half stayed green under sabotage, which independently confirms that direction was never broken.Related issue
Refs #7219. That issue's own reproducer was fixed by #7827; this is a sibling in the same family that fix could not reach, since it is the runtime's buffer model rather than codegen's proven-view tiers.
Pre-existing gaps deliberately left alone
RangeError, node throwsTypeError. Detaching zeroes every registered view's length, so the read is rejected as out-of-bounds before it can report the detach. The value read before detaching is now correct.ArrayBufferis unimplemented —ab.resizeis not a function, so a length-tracking DataView has nothing to track.dv instanceof DataViewisfalsewhere node saystrue. A separate brand-check gap, untouched here.test_gap_dataview_2878_2877_2879.tsfails 3 lines on macOS (copyWithinfor Int16/Float64). A/B'd as byte-identical before and after this change. Worth a look separately: it is absent fromtest-parity/gap_snapshot.json, so the Linux baseline expects it to pass — meaning it is either macOS-specific or the snapshot is stale.No version bump, no
CLAUDE.mdedit, noCHANGELOG.mdedit, per the template.Summary by CodeRabbit
Bug Fixes
DataViewreads after writes through typed-array views sharing the same buffer.TextDecoder.decode()to consistently read current data fromDataView,ArrayBuffer,Uint8Array, andBufferviews.Tests