From 06e91a18f48ddf1cae6cbc9946688dea6a161425 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Sat, 11 Jul 2026 10:36:01 +0000
Subject: [PATCH] [#730] Fix import/export context leak on failed initializeRemote validation (#731)
---
opendj-server-legacy/src/test/java/org/opends/server/replication/InitOnLineTest.java | 72 +++++++++++++++++++++++
opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationDomain.java | 58 ++++++++++++++----
opendj-server-legacy/src/test/java/org/opends/server/replication/ReplicationTestCase.java | 9 ++-
3 files changed, 121 insertions(+), 18 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationDomain.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationDomain.java
index 8781e70..a63f74a 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationDomain.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationDomain.java
@@ -1442,8 +1442,6 @@
int serverRunningTheTask, Task initTask, int initWindow)
throws DirectoryException
{
- final ImportExportContext ieCtx = acquireIEContext(false);
-
/*
We manage the list of servers to initialize in order :
- to test at the end that all expected servers have reconnected
@@ -1451,7 +1449,12 @@
- to update the task with the server(s) where this test failed
*/
- Map<Integer, DSInfo> replicaInfos = getReplicaInfos();
+ // Validate the request before acquiring the import/export context: a
+ // validation failure must not leave the context acquired, otherwise
+ // ieRunning() would remain true forever and the domain would reject every
+ // subsequent total update as a simultaneous import/export.
+ final Map<Integer, DSInfo> replicaInfos = getReplicaInfos();
+ final DSInfo targetDsi;
if (serverToInitialize == RoutableMsg.ALL_SERVERS)
{
if (replicaInfos.isEmpty())
@@ -1459,7 +1462,44 @@
throw new DirectoryException(UNWILLING_TO_PERFORM,
ERR_FULL_UPDATE_NO_REMOTES.get(getBaseDN(), getServerId()));
}
+ targetDsi = null;
+ }
+ else
+ {
+ targetDsi = getDsInfoOrNull(replicaInfos.values(), serverToInitialize);
+ if (targetDsi == null)
+ {
+ throw new DirectoryException(UNWILLING_TO_PERFORM,
+ ERR_FULL_UPDATE_MISSING_REMOTE.get(getBaseDN(), getServerId(), serverToInitialize));
+ }
+ }
+ final ImportExportContext ieCtx = acquireIEContext(false);
+ try
+ {
+ initializeRemote(ieCtx, replicaInfos, targetDsi, serverToInitialize,
+ serverRunningTheTask, initTask, initWindow);
+ }
+ finally
+ {
+ // Release the context whatever the outcome, otherwise ieRunning() would
+ // remain true forever (resolves the historical "FIXME should not this
+ // be in a finally?").
+ releaseIEContext();
+ }
+ }
+
+ /**
+ * Performs the remote initialization with the import/export context already
+ * acquired - and released - by the caller.
+ */
+ private void initializeRemote(ImportExportContext ieCtx,
+ Map<Integer, DSInfo> replicaInfos, DSInfo targetDsi,
+ int serverToInitialize, int serverRunningTheTask, Task initTask,
+ int initWindow) throws DirectoryException
+ {
+ if (serverToInitialize == RoutableMsg.ALL_SERVERS)
+ {
logger.info(NOTE_FULL_UPDATE_ENGAGED_FOR_REMOTE_START_ALL,
countEntries(), getBaseDN(), getServerId());
@@ -1475,18 +1515,11 @@
}
else
{
- DSInfo dsi = getDsInfoOrNull(replicaInfos.values(), serverToInitialize);
- if (dsi == null)
- {
- throw new DirectoryException(UNWILLING_TO_PERFORM,
- ERR_FULL_UPDATE_MISSING_REMOTE.get(getBaseDN(), getServerId(), serverToInitialize));
- }
-
logger.info(NOTE_FULL_UPDATE_ENGAGED_FOR_REMOTE_START, countEntries(),
getBaseDN(), getServerId(), serverToInitialize);
ieCtx.startList.add(serverToInitialize);
- ieCtx.setAckVal(dsi.getDsId(), 0);
+ ieCtx.setAckVal(targetDsi.getDsId(), 0);
}
DirectoryException exportRootException = null;
@@ -1625,9 +1658,6 @@
ERR_INIT_NO_SUCCESS_END_FROM_SERVERS.get(getGenerationID(), ieCtx.failureList));
}
- // Don't forget to release IEcontext acquired at beginning.
- releaseIEContext(); // FIXME should not this be in a finally?
-
final String cause = exportRootException == null ? ""
: exportRootException.getLocalizedMessage();
if (serverToInitialize == RoutableMsg.ALL_SERVERS)
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/replication/InitOnLineTest.java b/opendj-server-legacy/src/test/java/org/opends/server/replication/InitOnLineTest.java
index e2d0836..64f1695 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/replication/InitOnLineTest.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/replication/InitOnLineTest.java
@@ -656,6 +656,7 @@
}
// Launch in S1 the task that will initialize S2
+ waitForRemoteReplicas(server2ID);
addTask(taskInitTargetS2, ResultCode.SUCCESS, null);
// Signal RS we just entered the full update status
@@ -714,6 +715,7 @@
}
// Launch in S1 the task that will initialize S2
+ waitForRemoteReplicas(server2ID, server3ID);
addTask(taskInitTargetAll, ResultCode.SUCCESS, null);
// Tests that entries have been received by S2
@@ -1004,6 +1006,7 @@
// Launch in S1 the task that will initialize S2
log(testCase + " add task " + Thread.currentThread());
+ waitForRemoteReplicas(server2ID);
addTask(taskInitTargetS2, ResultCode.SUCCESS, null);
log(testCase + " " + server2.getServerId() + " wait target " + Thread.currentThread());
@@ -1027,6 +1030,47 @@
}
}
+ /**
+ * Tests that an InitializeTarget task failing its validation (the remote
+ * replica is unknown to the domain) does not leave the import/export
+ * context acquired (issue #730): the domain used to reject every subsequent
+ * total update as a simultaneous import/export.
+ */
+ @Test(enabled=true)
+ public void initializeTargetUnknownRemote() throws Exception
+ {
+ String testCase = "initializeTargetUnknownRemote";
+ log("Starting " + testCase);
+ try
+ {
+ replServer1 = createReplicationServer(replServer1ID, testCase);
+ connectServer1ToReplServer(replServer1ID);
+ addTestEntriesToDB();
+
+ // DS(42424) is unknown in the topology: the task must fail to start...
+ Entry taskInitTargetUnknown = TestCaseUtils.makeEntry(
+ "dn: ds-task-id=" + UUID.randomUUID() + ",cn=Scheduled Tasks,cn=Tasks",
+ "objectclass: top",
+ "objectclass: ds-task",
+ "objectclass: ds-task-initialize-remote-replica",
+ "ds-task-class-name: org.opends.server.tasks.InitializeTargetTask",
+ "ds-task-initialize-domain-dn: " + EXAMPLE_DN,
+ "ds-task-initialize-replica-server-id: " + 42424);
+ addTask(taskInitTargetUnknown, ResultCode.SUCCESS, null);
+ waitTaskState(taskInitTargetUnknown, STOPPED_BY_ERROR, 20000, null);
+
+ // ...but must not leave the import/export context acquired
+ assertFalse(replDomain.ieRunning(),
+ "ReplicationDomain: Import/Export is not expected to be running after a failed InitializeTarget");
+
+ log("Successfully ending " + testCase);
+ }
+ finally
+ {
+ afterTest(testCase);
+ }
+ }
+
private void waitForInitializeTargetMsg(String testCase,
ReplicationBroker server) throws Exception
{
@@ -1293,9 +1337,30 @@
* Disconnect broker and remove entries from the local DB
* @param testCase The name of the test case.
*/
+ /**
+ * Waits until the local replication domain sees the provided replicas in
+ * its topology view, so that an InitializeTarget task does not race the
+ * topology propagation and fail to start with "the remote directory server
+ * DS(x) is unknown" - which, combined with the import/export context leak
+ * (issue #730), used to poison the domain for the rest of the test class.
+ */
+ private void waitForRemoteReplicas(Integer... serverIds) throws Exception
+ {
+ for (int serverId : serverIds)
+ {
+ for (int i = 0; i < 100 && !replDomain.getReplicaInfos().containsKey(serverId); i++)
+ {
+ sleep(100);
+ }
+ assertTrue(replDomain.getReplicaInfos().containsKey(serverId),
+ "DS(" + serverId + ") is not known to the local replication domain");
+ }
+ }
+
private void afterTest(String testCase) throws Exception
{
// Check that the domain has completed the import/export task.
+ boolean ieStillRunning = false;
if (replDomain != null)
{
// race condition could cause the main thread to reach
@@ -1309,7 +1374,10 @@
}
sleep(500);
}
- assertFalse(replDomain.ieRunning(), "ReplicationDomain: Import/Export is not expected to be running");
+ // asserted only after the cleanup below: failing before it would leak
+ // the domain config, the brokers and the replication servers into the
+ // following tests and cascade the failure over the whole class
+ ieStillRunning = replDomain.ieRunning();
}
// Remove domain config
super.cleanConfigEntries();
@@ -1330,6 +1398,8 @@
Arrays.fill(replServerPort, 0);
log("Successfully cleaned " + testCase);
+
+ assertFalse(ieStillRunning, "ReplicationDomain: Import/Export is not expected to be running");
}
/**
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/replication/ReplicationTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/replication/ReplicationTestCase.java
index 3f302f8..54d858c 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/replication/ReplicationTestCase.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/replication/ReplicationTestCase.java
@@ -13,6 +13,7 @@
*
* Copyright 2006-2010 Sun Microsystems, Inc.
* Portions Copyright 2011-2016 ForgeRock AS.
+ * Portions Copyrighted 2026 3A Systems, LLC.
*/
package org.opends.server.replication;
@@ -726,11 +727,13 @@
}
}
- if (expectedTaskState == RUNNING && taskState == COMPLETED_SUCCESSFULLY)
+ if (expectedTaskState == RUNNING
+ && (taskState == COMPLETED_SUCCESSFULLY || taskState == STOPPED_BY_ERROR))
{
// We usually wait the running state after adding the task
- // and if the task is fast enough then it may be already done
- // and we can go on.
+ // and if the task is fast enough then it may already be done
+ // (successfully or not) and we can go on: callers interested in the
+ // final state wait for it explicitly afterwards.
}
else
{
--
Gitblit v1.10.0