From 0fadebe1af25fab00b0ae082129687116cfcf550 Mon Sep 17 00:00:00 2001 From: hg-ms <53219833+hg-ms@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:58:15 +0100 Subject: [PATCH 1/3] Reject storing and committing after a failed store until clear() A store() that threw left its partially serialized graph in the storer; a later commit, or a BatchStorer flush or close, persisted it: dangling references, a half-written entity and never-written objects registered as stored. The storage could not be restarted. The storer now records the first failure of item processing and rejects further stores and commits until clear(). A BatchStorer reports a failed batch once and does not flush it. BatchStorer.clear() and reinitialize() wait for an in-flight store. Serializer and TypedSerializer clear their internal storer when storing or committing fails. --- .../binary/types/BinaryStorer.java | 195 +++++++++++++----- .../org/eclipse/serializer/Serializer.java | 13 +- .../eclipse/serializer/TypedSerializer.java | 21 +- 3 files changed, 170 insertions(+), 59 deletions(-) diff --git a/persistence/binary/src/main/java/org/eclipse/serializer/persistence/binary/types/BinaryStorer.java b/persistence/binary/src/main/java/org/eclipse/serializer/persistence/binary/types/BinaryStorer.java index 8cbf472c..e621ffcb 100644 --- a/persistence/binary/src/main/java/org/eclipse/serializer/persistence/binary/types/BinaryStorer.java +++ b/persistence/binary/src/main/java/org/eclipse/serializer/persistence/binary/types/BinaryStorer.java @@ -197,6 +197,13 @@ protected static int defaultSlotSize() */ protected boolean isProcessingItems; + /* + * First failure of item processing. A failed store leaves partially serialized data (unprocessed items, + * a half-written entity, commit listeners of the failed store), so the storer must not store or commit + * until it is cleared. Guarded by objectRegistryMonitor. + */ + private Throwable storeFailure; + /* * item hashing structures get initialized lazily for the following reasons: * - the storer instance can commit (be cleared) and be reinitialized multiple times. @@ -426,6 +433,7 @@ protected void internalInitialize(final int hashLength) this.itemCount = 0; this.pinCount = 0; this.hashSlots = new Item[hashLength]; + this.storeFailure = null; this.hashRange = hashLength - 1; // initializing/clearing item chain @@ -603,27 +611,24 @@ protected long internalStore(final Object root) * - handle all registered graph objects recursively (but transformed to an iteration). * Note that this is NOT the same as apply, which does NOT store if the instance is already registry-known. */ + if(!this.isProcessingItems) + { + this.validateNoFailedStore(); + } + long rootOid; if(Swizzling.isFoundId(rootOid = this.lookupOid(root))) { return rootOid; } - + // initial registration. After that, storing adds via recursion the graph and processing items iteratively. rootOid = this.registerGuaranteed(notNull(root)); // repeatedly calling #store to add an instance to the item chain is fine, but processing may only happen once. if(!this.isProcessingItems) { - try - { - this.isProcessingItems = true; - this.processItems(); - } - finally - { - this.isProcessingItems = false; - } + this.processItemsOutermost(); } return rootOid; @@ -666,6 +671,65 @@ protected final void storeItem(final Item item) item.typeHandler.store(chunk, item.instance, item.oid, this); } + /** + * Processes the item chain as the outermost store call and records the first failure. + */ + protected final void processItemsOutermost() + { + try + { + this.isProcessingItems = true; + this.processItems(); + } + catch(final Throwable t) + { + synchronized(this.objectRegistryMonitor) + { + if(this.storeFailure == null) + { + this.storeFailure = t; + } + } + throw t; + } + finally + { + this.isProcessingItems = false; + } + } + + /** + * Rejects storing and committing after a failed store: committing would persist the partially + * serialized graph. Only {@link #clear()} resets the storer. + *

+ * Must be called under {@code objectRegistryMonitor} and not during item processing. + */ + protected final void synchValidateNoFailedStore() + { + if(this.storeFailure != null) + { + throw new PersistenceException( + "A store of this storer failed and left partially serialized data." + + " Committing it would persist an inconsistent object graph." + + " The storer must be cleared before it can be used again.", + this.storeFailure + ); + } + } + + protected final Throwable synchStoreFailure() + { + return this.storeFailure; + } + + protected final void validateNoFailedStore() + { + synchronized(this.objectRegistryMonitor) + { + this.synchValidateNoFailedStore(); + } + } + @Override public final long store(final Object root) { @@ -797,6 +861,9 @@ public Object commit() LazyArg(this::size) // use lazy here, #size() locks ); + // before anything is written, merged or notified: the storer keeps its state until cleared + this.validateNoFailedStore(); + // isEmpty locks internally if(!this.isEmpty()) { @@ -1455,29 +1522,26 @@ protected final long internalStore(final Object root, final long objectId) * - handle all registered graph objects recursively (but transformed to an iteration). * Note that this is NOT the same as apply, which does NOT store if the instance is already registry-known. */ + if(!this.isProcessingItems) + { + this.validateNoFailedStore(); + } + final long rootOid; if(Swizzling.isFoundId(rootOid = this.lookupOid(root))) { return rootOid; } - + // initial registration. After that, storing adds via recursion the graph and processing items iteratively. //rootOid = this.registerGuaranteed(notNull(root)); - + this.registerGuaranteed(objectId, root, null); // repeatedly calling #store to add an instance to the item chain is fine, but processing may only happen once. if(!this.isProcessingItems) { - try - { - this.isProcessingItems = true; - this.processItems(); - } - finally - { - this.isProcessingItems = false; - } + this.processItemsOutermost(); } return objectId; @@ -1489,7 +1553,12 @@ protected final long internalStore(final Object root, final long objectId) public void forceRootStore(final PersistenceRoots pendingStoreRoot) { logger.debug("Storing updated root"); - + + if(!this.isProcessingItems) + { + this.validateNoFailedStore(); + } + long rootOid; if(Swizzling.isFoundId(rootOid = this.lookupOid(pendingStoreRoot))) { @@ -1499,18 +1568,10 @@ public void forceRootStore(final PersistenceRoots pendingStoreRoot) { { rootOid = this.registerGuaranteed(notNull(pendingStoreRoot)); } - + if(!this.isProcessingItems) { - try - { - this.isProcessingItems = true; - this.processItems(); - } - finally - { - this.isProcessingItems = false; - } + this.processItemsOutermost(); } } @@ -1681,6 +1742,7 @@ public final class Batching extends Default implements BatchStorer private final BatchStorer.Controller controller ; private final BackgroundFlusher flusher ; private long pendingSinceNanos ; + private boolean failureReported ; private volatile boolean closed ; /* @@ -1761,19 +1823,15 @@ public long store(final Object instance, final long objectId) { throw new IllegalStateException("BatchStorer is already closed."); } + if(!this.isProcessingItems) + { + this.validateNoFailedStore(); + } this.registerGuaranteed(objectId, instance, null); if(!this.isProcessingItems) { - try - { - this.isProcessingItems = true; - super.processItems(); - } - finally - { - this.isProcessingItems = false; - } + this.processItemsOutermost(); } } @@ -1850,6 +1908,10 @@ protected long internalStore(final Object root) { throw new IllegalStateException("BatchStorer is already closed."); } + if(!this.isProcessingItems) + { + this.validateNoFailedStore(); + } final long localOid = this.lookupOid(root); @@ -1868,15 +1930,7 @@ protected long internalStore(final Object root) if(!this.isProcessingItems) { - try - { - this.isProcessingItems = true; - super.processItems(); - } - finally - { - this.isProcessingItems = false; - } + this.processItemsOutermost(); } } @@ -1908,10 +1962,33 @@ protected void processItems() @Override public void clear() { - synchronized(this.objectRegistryMonitor) + // commitLock: an in-flight store of another thread must not continue into the cleared chain and chunks + synchronized(this.commitLock) + { + synchronized(this.objectRegistryMonitor) + { + super.clear(); + this.pendingSinceNanos = 0L; + this.failureReported = false; + } + } + } + + @Override + public PersistenceStorer reinitialize() + { + synchronized(this.commitLock) { - super.clear(); - this.pendingSinceNanos = 0L; + return super.reinitialize(); + } + } + + @Override + public PersistenceStorer reinitialize(final long initialCapacity) + { + synchronized(this.commitLock) + { + return super.reinitialize(initialCapacity); } } @@ -1995,6 +2072,18 @@ private void optFlush() return; } + // a failed store blocks committing until clear(): report it once instead of failing on every tick + final Throwable storeFailure = this.synchStoreFailure(); + if(storeFailure != null) + { + if(!this.failureReported) + { + this.failureReported = true; + logger.error("Batch not flushed: a store failed, the pending data is kept until clear()", storeFailure); + } + return; + } + final long nowNanos = System.nanoTime(); if(this.pendingSinceNanos == 0L) { diff --git a/serializer/src/main/java/org/eclipse/serializer/Serializer.java b/serializer/src/main/java/org/eclipse/serializer/Serializer.java index b6a83fe6..73bb7bda 100644 --- a/serializer/src/main/java/org/eclipse/serializer/Serializer.java +++ b/serializer/src/main/java/org/eclipse/serializer/Serializer.java @@ -207,8 +207,17 @@ public static class Default implements Serializer @Override public synchronized M serialize(final Object object) { - this.storer.store(object); - this.storer.commit(); + try + { + this.storer.store(object); + this.storer.commit(); + } + catch(final Throwable t) + { + // the internal storer would carry the partial graph into the next serialize() call + this.storer.clear(); + throw t; + } return this.toMedium.apply(this.output); } diff --git a/serializer/src/main/java/org/eclipse/serializer/TypedSerializer.java b/serializer/src/main/java/org/eclipse/serializer/TypedSerializer.java index 71c8f624..d047cb88 100644 --- a/serializer/src/main/java/org/eclipse/serializer/TypedSerializer.java +++ b/serializer/src/main/java/org/eclipse/serializer/TypedSerializer.java @@ -259,8 +259,7 @@ public long getLastTypeInfoTimeStamp() public synchronized M serialize(final Object object) { //serialize data - this.storer.store(object); - this.storer.commit(); + this.storeAndCommit(object); final ByteBuffer[] dataBuffers = this.output.buffers(); final ByteBuffer[] typeInfoBuffers = this.updateTypeInfo(); @@ -291,12 +290,26 @@ public synchronized M serialize(final Object object) return this.toMedium.apply(ChunksWrapper.New(buffers)); } + private void storeAndCommit(final Object object) + { + try + { + this.storer.store(object); + this.storer.commit(); + } + catch(final Throwable t) + { + // the internal storer would carry the partial graph into the next serialize() call + this.storer.clear(); + throw t; + } + } + private ByteBuffer[] updateTypeInfo() { if(this.typeInfoStrategy.hasUpdate() || this.typeInfoCache == null) { - this.storer.store(this.typeInfoStrategy.get()); - this.storer.commit(); + this.storeAndCommit(this.typeInfoStrategy.get()); this.typeInfoCache = new TypeInfoCache( this.output.buffers(), System.nanoTime()); From 7765833dbed0fcdc821848bee415734b4888517d Mon Sep 17 00:00:00 2001 From: hg-ms <53219833+hg-ms@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:48:23 +0100 Subject: [PATCH 2/3] Document the failed-store contract of Storer and BatchStorer --- .../persistence/binary/types/BinaryStorer.java | 13 +++++++------ .../serializer/persistence/types/BatchStorer.java | 6 ++++++ .../serializer/persistence/types/Storer.java | 13 +++++++++++++ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/persistence/binary/src/main/java/org/eclipse/serializer/persistence/binary/types/BinaryStorer.java b/persistence/binary/src/main/java/org/eclipse/serializer/persistence/binary/types/BinaryStorer.java index e621ffcb..8010a022 100644 --- a/persistence/binary/src/main/java/org/eclipse/serializer/persistence/binary/types/BinaryStorer.java +++ b/persistence/binary/src/main/java/org/eclipse/serializer/persistence/binary/types/BinaryStorer.java @@ -433,9 +433,10 @@ protected void internalInitialize(final int hashLength) this.itemCount = 0; this.pinCount = 0; this.hashSlots = new Item[hashLength]; - this.storeFailure = null; this.hashRange = hashLength - 1; + this.storeFailure = null; + // initializing/clearing item chain (this.tail = this.head).next = null; @@ -621,7 +622,7 @@ protected long internalStore(final Object root) { return rootOid; } - + // initial registration. After that, storing adds via recursion the graph and processing items iteratively. rootOid = this.registerGuaranteed(notNull(root)); @@ -1532,10 +1533,10 @@ protected final long internalStore(final Object root, final long objectId) { return rootOid; } - + // initial registration. After that, storing adds via recursion the graph and processing items iteratively. //rootOid = this.registerGuaranteed(notNull(root)); - + this.registerGuaranteed(objectId, root, null); // repeatedly calling #store to add an instance to the item chain is fine, but processing may only happen once. @@ -1553,7 +1554,7 @@ protected final long internalStore(final Object root, final long objectId) public void forceRootStore(final PersistenceRoots pendingStoreRoot) { logger.debug("Storing updated root"); - + if(!this.isProcessingItems) { this.validateNoFailedStore(); @@ -1568,7 +1569,7 @@ public void forceRootStore(final PersistenceRoots pendingStoreRoot) { { rootOid = this.registerGuaranteed(notNull(pendingStoreRoot)); } - + if(!this.isProcessingItems) { this.processItemsOutermost(); diff --git a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/BatchStorer.java b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/BatchStorer.java index 00b11fd6..eaeeedaf 100644 --- a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/BatchStorer.java +++ b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/BatchStorer.java @@ -36,6 +36,12 @@ * {@code BatchStorer} implements {@link AutoCloseable}. On close, it flushes any remaining * pending data and releases resources. *

+ * If a store call fails, the batch holds incomplete data and is not flushed: further store calls, + * {@link #flush()}, {@link #commit()} and {@link #close()} throw a + * {@link org.eclipse.serializer.persistence.exceptions.PersistenceException}, and the background + * flush skips the batch. This affects the whole batch, including data stored by other threads. + * {@link #clear()} discards the pending data and makes the {@code BatchStorer} usable again. + *

* Usage: * 

  * try (BatchStorer storer = storageManager.batchStorerBuilder()
diff --git a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/Storer.java b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/Storer.java
index 002d057a..49238373 100644
--- a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/Storer.java
+++ b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/Storer.java
@@ -52,6 +52,11 @@ public interface Storer extends PersistenceStoring
 	 * 

 	 * The method may be called at most once on a fresh storer state. To reuse the storer, call
 	 * {@link #clear()} or {@link #reinitialize()} afterwards.
+	 * 

+	 * If a {@code store(...)} call on this storer failed, the accumulated data is incomplete. In that
+	 * case {@code commit()} persists nothing and throws a
+	 * {@link org.eclipse.serializer.persistence.exceptions.PersistenceException} with the store
+	 * failure as its cause, until {@link #clear()} discards the accumulated data.
 	 *
 	 * @return implementation-specific status information about the commit, or {@code null} if the
 	 *         implementation does not provide any.
@@ -69,6 +74,8 @@ public interface Storer extends PersistenceStoring
 	 * This rolls back uncommitted in-memory state only; data already committed by a previous
 	 * {@link #commit()} call is unaffected. After {@code clear()} the storer is ready to start a new
 	 * collection cycle.
+	 * 

+	 * After a failed {@code store(...)} call, {@code clear()} is the only way to use the storer again.
 	 *
 	 * @see #commit()
 	 * @see #reinitialize()
@@ -264,6 +271,12 @@ public default boolean isEmpty()
 	 * {@link PersistenceObjectRegistry} and nothing is written to the underlying storage layer.
 	 * Calling {@code store(...)} on the same instance multiple times before {@code commit()} has no
 	 * additional effect.
+	 * 

+	 * If this call fails, for example because the graph contains an instance that cannot be persisted,
+	 * the data accumulated by this storer is incomplete. Further {@code store(...)} and
+	 * {@link #commit()} calls then throw a
+	 * {@link org.eclipse.serializer.persistence.exceptions.PersistenceException} until
+	 * {@link #clear()} is called.
 	 *
 	 * @see #commit()
 	 * @see #skip(Object)

From b17101b5aac40a8c208bf186c3f01c6a29a6090e Mon Sep 17 00:00:00 2001
From: hg-ms <53219833+hg-ms@users.noreply.github.com>
Date: Fri, 30 Oct 2026 13:34:31 +0100
Subject: [PATCH 3/3] Document that a closed BatchStorer cannot recover a
 failed batch

---
 .../org/eclipse/serializer/persistence/types/BatchStorer.java | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/BatchStorer.java b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/BatchStorer.java
index eaeeedaf..4b16f4a6 100644
--- a/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/BatchStorer.java
+++ b/persistence/persistence/src/main/java/org/eclipse/serializer/persistence/types/BatchStorer.java
@@ -40,7 +40,9 @@
  * {@link #flush()}, {@link #commit()} and {@link #close()} throw a
  * {@link org.eclipse.serializer.persistence.exceptions.PersistenceException}, and the background
  * flush skips the batch. This affects the whole batch, including data stored by other threads.
- * {@link #clear()} discards the pending data and makes the {@code BatchStorer} usable again.
+ * {@link #clear()} discards the pending data and makes the {@code BatchStorer} usable again, unless
+ * it was already closed: {@link #close()} releases its resources even when the final flush is
+ * rejected, so a failed batch must be cleared before closing it to keep using it.
  * 

  * Usage:
  *