From 7b698c8fb185b83ead53f48881d56e4d7c21013e Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 23 Jun 2026 07:25:19 -0600 Subject: [PATCH] fix: forward isCompatible from _saveTypedStructures to saveStructures _saveTypedStructures called this.saveStructures(structures) without forwarding the isCompatible function that prepareStructures attaches to the returned structures. A saveStructures implementation that runs an optimistic CAS on the second parameter (e.g. Harper's RocksDB override) therefore saw `undefined`, so a concurrent same-length typed-struct save could silently clobber the previously persisted struct. Forward it as the second arg, matching msgpackr's own pack call site (saveStructures(newSharedData, newSharedData.isCompatible)). See HarperFast/harper#1441 / #1442. Co-Authored-By: Claude Opus 4.8 --- index.js | 8 +++++++- tests/test.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 26c0184..4536b63 100644 --- a/index.js +++ b/index.js @@ -162,7 +162,13 @@ export function createStructon(BaseClass) { _saveTypedStructures() { if (typeof this.saveStructures === 'function') { const structures = prepareStructures(this.structures || [], this); - this.saveStructures(structures); + // Forward isCompatible as the second arg, matching msgpackr's own pack call site + // (saveStructures(newSharedData, newSharedData.isCompatible)). prepareStructures attaches + // it to the returned structures; without forwarding it, a saveStructures implementation + // that runs an optimistic CAS on the parameter (e.g. Harper's RocksDB override) sees + // `undefined` and a concurrent same-length save silently clobbers the previously persisted + // struct. See HarperFast/harper#1441. + this.saveStructures(structures, structures.isCompatible); } else if (typeof this.saveShared === 'function') { this.saveShared({ structures: this.structures || [], diff --git a/tests/test.js b/tests/test.js index fc5bf8b..9f47fa9 100644 --- a/tests/test.js +++ b/tests/test.js @@ -217,6 +217,37 @@ suite('structon – structure persistence', function () { assert.strictEqual(result.name, 'Bob'); assert.strictEqual(result.age, 25); }); + + // Regression: _saveTypedStructures must forward isCompatible as the second arg so a saveStructures + // implementation can run its optimistic CAS. Dropping it let a concurrent same-length typed-struct + // save silently clobber the previous one (HarperFast/harper#1441). The fast-path encode goes through + // the base class's own saveStructures call (which already passes isCompatible), so we exercise + // _saveTypedStructures directly — the standalone-path call site this guards. + test('_saveTypedStructures forwards isCompatible as the second argument (harper#1441)', function () { + const calls = []; + const enc = new Structon({ + structures: [], + saveStructures(structures, isCompatible) { + calls.push({ structures, isCompatible }); + return true; + }, + }); + + // Establish a typed struct so prepareStructures wraps {named, typed} and attaches isCompatible. + enc.encode({ name: 'Alice', age: 30 }); + calls.length = 0; // ignore any saves from the encode path; we test _saveTypedStructures itself + + enc._saveTypedStructures(); + + assert.strictEqual(calls.length, 1, '_saveTypedStructures should call saveStructures once'); + const last = calls[0]; + assert.strictEqual(typeof last.isCompatible, 'function', 'isCompatible must be forwarded as the second arg'); + assert.strictEqual( + last.isCompatible, + last.structures.isCompatible, + 'the forwarded isCompatible must be the one prepareStructures attached to the structures' + ); + }); }); // ── binary compatibility with msgpackr's randomAccessStructure ──────────────