Skip to content

Commit e75ab50

Browse files
kriszypclaude
andcommitted
fix: cap maxOwnStructures at 32 to avoid two-byte record corruption
RecordEncoder set maxOwnStructures=256 (the OOM cap). With msgpackr's default maxSharedStructures of 32, that pushes maxOwn+maxShared past 64, flipping on msgpackr's two-byte record-id encoding. That path mis-serializes over-cap "own" structures when a shared-structures store is present (every primary store has one): it writes an out-of-range record-id reference instead of inlining the structure, so once a table exceeds the shared cap, new records become undecodable ("Record id is not defined for N") — on BOTH the typed default and the randomAccessFields=false opt-out. Confirmed via a high-cardinality repro (2900+/3000 records undecodable at maxOwn=256; 0 at maxOwn<=32). Lowering to 32 keeps msgpackr on the one-byte path, which inlines over-cap shapes correctly and bounds memory at least as tightly. Adds a regression test that writes 500 distinct shapes through a shared store and asserts every record decodes, on both the typed and readOnlyStructures paths. The deeper msgpackr two-byte fix (to make >32 shared structures correct) is tracked separately. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent cd7962a commit e75ab50

2 files changed

Lines changed: 54 additions & 4 deletions

File tree

resources/RecordEncoder.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,18 @@ export class RecordEncoder extends Encoder {
8888
name: string;
8989
constructor(options) {
9090
options.useBigIntExtension = true;
91-
// Bound the per-encoder typed-structure dictionary. It is append-only and pinned on the
92-
// long-lived primary store, so a wide/sparse schema (whose records vary by per-field value
93-
// width) can grow it unbounded and exhaust memory. Caller-overridable; default caps it.
94-
options.maxOwnStructures ??= 256;
91+
// Bound the per-encoder structure dictionary. It is append-only and pinned on the long-lived
92+
// primary store, so a wide/sparse schema (whose records vary by per-field value width) can grow
93+
// it unbounded and exhaust memory. Caller-overridable; default caps it.
94+
//
95+
// Must stay <= 32: with msgpackr's default maxSharedStructures (32), maxOwnStructures + 32 > 64
96+
// flips on the two-byte record-id encoding, which mis-serializes over-cap "own" structures when a
97+
// shared-structures store is present (the primary store always has one) — it writes an out-of-range
98+
// record-id reference instead of inlining the structure, so records become undecodable
99+
// ("Record id is not defined for N") once a table exceeds the shared cap. The one-byte path
100+
// (<= 32) inlines over-cap shapes correctly and bounds memory at least as tightly. (A prior 256
101+
// triggered this on both the typed default and the randomAccessFields=false opt-out.)
102+
options.maxOwnStructures ??= 32;
95103
// When random-access fields are disabled (storage.randomAccessFields=false), write records as
96104
// classic shared structures instead of typed random-access structures. randomAccessStructure stays
97105
// on so reads still decode either form — existing typed-struct data remains readable; only new

unitTests/resources/recordEncoder.test.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,3 +177,45 @@ describe('RecordEncoder random-access fields opt-out (readOnlyStructures)', () =
177177
});
178178
});
179179
});
180+
181+
describe('RecordEncoder structure dictionary cap (no two-byte over-cap corruption)', () => {
182+
// Regression: harper's default maxOwnStructures must keep msgpackr on the one-byte record path
183+
// (maxOwnStructures + the default maxSharedStructures of 32 must stay <= 64). A larger value (the
184+
// prior 256) flips on the two-byte path, which mis-serializes over-cap "own" structures when a
185+
// shared-structures store is present — records written after a table exceeds the shared cap become
186+
// undecodable ("Record id is not defined for N"), on both the typed default and the
187+
// randomAccessFields=false opt-out. These write far more distinct shapes than the shared cap through
188+
// a shared store and assert every record still decodes.
189+
function diverseRecords(count) {
190+
const records = [];
191+
for (let i = 0; i < count; i++) {
192+
const rec = { id: i };
193+
const attrCount = 1 + (i % 6);
194+
for (let a = 0; a < attrCount; a++) rec['attr_' + ((i * 7 + a) % 200)] = i % 2 ? 's' + i : i;
195+
records.push(rec);
196+
}
197+
return records;
198+
}
199+
200+
function assertAllDecode(extra) {
201+
const store = sharedStore();
202+
const writer = makeEncoder(store, extra);
203+
const reader = makeEncoder(store, extra);
204+
const records = diverseRecords(500); // far exceeds the 32 shared-structure cap
205+
const buffers = records.map((r) => Buffer.from(writer.encode(r)));
206+
let failures = 0;
207+
for (let i = 0; i < buffers.length; i++) {
208+
const decoded = reader.decode(buffers[i]);
209+
if (!decoded || decoded.id !== records[i].id) failures++;
210+
}
211+
assert.strictEqual(failures, 0, `all ${records.length} records should decode after exceeding the shared cap`);
212+
}
213+
214+
it('decodes every record past the shared cap on the typed path (default)', () => {
215+
assertAllDecode(undefined);
216+
});
217+
218+
it('decodes every record past the shared cap with randomAccessFields off (readOnlyStructures)', () => {
219+
assertAllDecode({ readOnlyStructures: true });
220+
});
221+
});

0 commit comments

Comments
 (0)