Add dangling-reference self-healing to BinaryStorer - #294
Merged
Conversation
When enabled (opt-in via the storer creator), a commit rejected by the persistence target for dangling references is repaired transparently: a dangling id is an id whose data is missing while the referenced instance is still alive in memory - the instance is re-serialized under its existing object id in a compensating commit, the original buffers are rewound and the write is retried. Bounded by attempts (channel count + 1), heal depth, and a no-progress guard; unloaded Lazy references' cached ids have no instance and remain unhealable (the store fails as in fail mode). The trusted-object-id capture is extended to record the instance alongside the id (HashMapIdObject) so the healer can reach it; the Lazy path records null = unhealable-if-missing. Note: the former Lazy $link commit-rollback mechanism of this change was superseded by the deferred post-commit linking (#290) and is no longer part of it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds an opt-in “dangling reference self-healing” mechanism to BinaryStorer so that, when a PersistenceTarget rejects a write due to missing referenced object ids, the storer can transparently re-store the still-live referenced instances under their existing OIDs and retry the original write (including chunk-buffer rewinding for byte-identical retries).
Changes:
- Introduces
PersistenceDanglingReferencesso targets can report missing object ids without depending on concrete storage exception types. - Adds
ChunksBuffer.rewindBuffers()to support safe, byte-identical retry after a failed/rolled-back target write. - Extends
BinaryStorerwith an opt-in healing loop (new creator flag) and switches trusted-id capture toHashMapIdObjectfor id→instance capture where available.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| persistence/persistence/src/main/java/org/eclipse/serializer/persistence/exceptions/PersistenceDanglingReferences.java | New interface for target-side exceptions to expose missing object ids for retry/heal logic. |
| persistence/binary/src/main/java/org/eclipse/serializer/persistence/binary/types/ChunksBuffer.java | Adds buffer-position rewind capability for retrying identical writes. |
| persistence/binary/src/main/java/org/eclipse/serializer/persistence/binary/types/BinaryStorer.java | Adds opt-in dangling-reference healing path in commit() write flow; extends creator API and trusted-id capture to include instances. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A storer constructed with healing enabled but capture disabled could never resolve a missing id to its live instance - healing would silently do nothing and the terminal failure would misleadingly suggest unhealable ids. The capture map is now allocated whenever healing is enabled, regardless of the capture flag. The regular embedded wiring is unaffected (the heal policy always implies validation there); this hardens direct Creator usage.
…acts - A healing commit failing for a reason of its own (e.g. an IO error) no longer masks the original dangling-references rejection: the original exception - carrying the missing ids the operator needs - is attached as suppressed to the healing failure. - PersistenceTarget.write now documents the buffer contract the byte-identical retry relies on: positions may be consumed, limits must not be modified (no flip/clear/limit) - a violating custom target would silently write truncated data on retry. - The healing attempt bound documents its cross-side contract: each rejection reports ALL missing ids of its failing channel at once; a target reporting ids one at a time degrades fail-safe but gives up healable ids prematurely.
zdenek-jonas
approved these changes
Jul 7, 2026
This was referenced Jul 7, 2026
fh-ms
added a commit
that referenced
this pull request
Jul 8, 2026
…s success (#296) * Couple healing commits' deferred Lazy $links to the outermost commit's success Fixes the #290/#294 cross-fix interaction of internal#82: the healing storer's compensating commit fired the deferred $link commit listeners of every Lazy it serialized, although the enclosing store's retried write could still fail terminally. A Lazy linked over durable-but- unreachable data is legally clearable (the #292 isUsed() guard covers only used-marked references); once the LazyReferenceManager clears it and the target's GC reclaims the data, a later re-store captures the cached id with a null instance - unhealable, permanent loss. The path arms with reference-validation = heal (store#742), so this must merge before or together with that PR. Healing storers now route registerCommitListener to the ROOT storer whose commit they compensate (transitive healing flattens to the same root): deferred effects fire exactly when the outermost commit succeeds. On success the healed subgraph's Lazies end up properly linked, as if the store had never been rejected; on terminal failure nothing was ever linked, so nothing becomes clearable and no data can be lost. Side effects of the design: - A healing commit never carries local listeners, so the related internal#82 finding - a post-durability listener fault masquerading as a healing failure and aborting a salvageable retry - is structurally unreachable (documented on notifyCommitListeners). - The healing commit's registry merge stays immediate. Its residue after a terminal failure (associations to unreachable healed entities) is benign and self-rescuing: registry knowledge alone makes no Lazy clearable, and a later store trusting such an id either finds the data still present or gets rejected and heals it again from the re-captured instance (documented on writeToTarget). Regression test HealingDeferredLinkTest drives the exact sequence through a scripted PersistenceTarget (reject -> healing write accepted -> retry fails/succeeds), which real storage cannot produce deterministically: the failed-retry case pins the Lazy staying unlinked and unclearable, the success case pins the transferred link firing with the outer commit. * Refactor BinaryStorer to Storer for commitListenerSink to improve type generality
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
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.
What this adds, in plain terms
The merged store-time validation can reject a store whose data references object ids with no existing entity. This PR adds the repair: when enabled, the storer heals such a rejection automatically and transparently.
The key insight making this possible: a dangling trusted id is an id whose DATA is missing while the referenced INSTANCE is still alive in application memory - that is precisely why the id was trusted in the first place (the instance sits in the object registry), and the capture records the instance alongside the id. Healing therefore re-serializes that instance under its EXISTING object id in a small compensating commit, then retries the original write. The already-serialized data stays valid - the reference id never changes - and the caller sees nothing but a WARN log per healing round.
One class of ids is unhealable by nature: an unloaded
Lazyreference's cached id has no in-memory instance - if its data is missing, it is genuinely gone, and the store keeps failing loudly exactly likefailmode.Technical details
commit(). The write loop walks the failure's cause chain for the newPersistenceDanglingReferencesinterface (implemented by the storage-side rejection exception - the serializer stays free of storage types), re-stores the captured instances for the missing ids via a healing storer (same dependencies, registered as a local registry, depth+1), commits it, rewinds the original buffers, retries.ChunksBuffers keepposition=0, limit=contentLength; only the target write advances positions, never limits, and a failing channel throws BEFORE writing. The newChunksBuffer.rewindBuffers()resets positions and the identical bytes are retried - deliberately NOT a re-serialization, which would snapshot state mutated after the originalstore()call.HashMapIdObject); the unloaded-Lazy path records null = unhealable-if-missing.$linkrollback mechanism for failed commits - that is superseded by the deferred linking and no longer part of this PR.healDanglingReferencesflag on theBinaryStorer.Creatorfactory; all existing signatures delegate tofalse.Scope and pairing
reference-validation = heal(storage-side identical tofail: validate, reject, roll back — the healing loop then repairs and retries) and carries the test suite: transparent heal with id stability verified, transitive heal, a 4-channel multi-round heal (proving the attempt cap AND the buffer rewind - peer channels wrote, rolled back, and rewrote identical bytes), unhealable-Lazy escalation, and post-failure recovery. Restart verification in every case.