[#730] Fix import/export context leak on failed initializeRemote validation (#731)

Author: vharseko

opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationDomain.java

@@ -1442,24 +1442,64 @@ protected void initializeRemote(int serverToInitialize,
       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
     after their import and with the right genId
     - 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())
       {
         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 @@ protected void initializeRemote(int serverToInitialize,
     }
     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 @@ protected void initializeRemote(int serverToInitialize,
               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)

opendj-server-legacy/src/test/java/org/opends/server/replication/InitOnLineTest.java

@@ -656,6 +656,7 @@ public void initializeTargetExport() throws Exception
       }
 
       // 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 @@ public void initializeTargetExportAll() throws Exception
       }
 
       // 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 @@ public void initializeTargetExportMultiSS() throws Exception
 
       // 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 @@ public void initializeTargetExportMultiSS() throws Exception
     }
   }
 
+  /**
+   * 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 @@ public void initializeSimultaneous() throws Exception
    * 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 @@ private void afterTest(String testCase) throws Exception
         }
         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 @@ private void afterTest(String testCase) throws Exception
 
     Arrays.fill(replServerPort, 0);
     log("Successfully cleaned " + testCase);
+
+    assertFalse(ieStillRunning, "ReplicationDomain: Import/Export is not expected to be running");
   }
 
   /**

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 @@ public Entry call() throws Exception
       }
     }
 
-    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
     {