From 1af0a1247d3e2d503a53723dd1d844c90c43ccc6 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Sat, 19 Sep 2026 08:56:47 +0000
Subject: [PATCH] [#993] Register an entry container's configuration listeners only once it has opened (#999)
---
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java | 69 ++++++++++++++++++++++++++++++++++
1 files changed, 68 insertions(+), 1 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java
index ad1dcd6..e1dbac5 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java
@@ -17,6 +17,7 @@
*/
package org.opends.server.backends.pluggable;
+import static org.forgerock.util.Utils.closeSilently;
import static org.opends.messages.BackendMessages.*;
import static org.opends.server.util.StaticUtils.*;
@@ -129,9 +130,12 @@
*/
void open(final AccessMode accessMode) throws StorageRuntimeException, ConfigException
{
+ boolean opened = false;
+ boolean storageOpened = false;
try
{
storage.open(accessMode);
+ storageOpened = true;
storage.write(new WriteOperation()
{
@Override
@@ -144,6 +148,7 @@
// after the write, never inside it: a compressed schema migration is only worth reporting
// once the transaction that copied it has committed, and a replayed operation runs twice
compressedSchema.reportMigration();
+ opened = true;
}
catch(StorageRuntimeException e)
{
@@ -153,6 +158,52 @@
{
throw new StorageRuntimeException(e);
}
+ finally
+ {
+ if (!opened)
+ {
+ giveUpAfterFailedOpen(storageOpened);
+ }
+ }
+ }
+
+ /**
+ * Gives back what a root container which failed to open took. Nothing else will: the caller
+ * throws it away - {@link BackendImpl#newRootContainer} lets every failure through without a
+ * reference to it left anywhere - and {@code BackendConfigManager} releases the backend's shared
+ * lock without calling {@code closeBackend()} for a backend which never opened. What is left here
+ * is left for the life of the JVM: entry containers and this root container go on answering the
+ * configuration changes of a backend which is not running, and every later attempt to enable that
+ * backend adds another set of them.
+ * <p>
+ * The failure being given up after is the one worth reporting, so nothing here is allowed to
+ * replace it.
+ *
+ * @param storageOpened whether the storage opened, which is false on one road only: its
+ * {@code open()} threw. A storage whose open failed is not one this container can
+ * close - what that open took before it failed is the storage's own to give back, as
+ * {@code PDBStorage}, {@code JEStorage} and {@code JDBCStorage} do - and there is no other: every
+ * root container is opened over a storage no root container holds, since
+ * {@code BackendImpl} opens one, read only or not, only while it has none.
+ */
+ private void giveUpAfterFailedOpen(boolean storageOpened)
+ {
+ try
+ {
+ for (DN baseDN : entryContainers.keySet())
+ {
+ closeSilently(unregisterEntryContainer(baseDN));
+ }
+ config.removePluggableChangeListener(this);
+ if (storageOpened)
+ {
+ storage.close();
+ }
+ }
+ catch (Exception e)
+ {
+ logger.traceException(e);
+ }
}
/**
@@ -221,12 +272,28 @@
private void openAndRegisterEntryContainers(WriteableTransaction txn, Set<DN> baseDNs, AccessMode accessMode)
throws StorageRuntimeException, InitializationException, ConfigException
{
+ // Give up what a previous, rolled back attempt registered: this runs inside the write
+ // Storage.write may replay, and an entry container left registered fails the attempt which
+ // replaces it with ERR_ENTRY_CONTAINER_ALREADY_REGISTERED - so a write-write conflict, which
+ // the replay is there to absorb, would leave the backend unopened instead - while keeping the
+ // configuration listeners it opened with, which only its close() takes back. The same shape as
+ // BackendImpl.changeBaseDNTrees, which opens its entry containers inside a write for the same
+ // reason. Nothing else can reach these containers: the backend registers its base DNs only once
+ // this has returned, so they are closed without being locked.
+ for (DN baseDN : baseDNs)
+ {
+ closeSilently(unregisterEntryContainer(baseDN));
+ }
+
EntryID highestID = null;
for (DN baseDN : baseDNs)
{
EntryContainer ec = openEntryContainer(baseDN, txn, accessMode);
- EntryID id = ec.getHighestEntryID(txn);
+ // Registered before anything else here can throw: a container which opened has registered
+ // every listener it ever will, and only what this map holds is given back when the open of
+ // the root container fails.
registerEntryContainer(baseDN, ec);
+ EntryID id = ec.getHighestEntryID(txn);
if (highestID == null || id.compareTo(highestID) > 0)
{
highestID = id;
--
Gitblit v1.10.0