[#730] Fix import/export context leak on failed initializeRemote validation...

Valery Kharseko

10 hours ago 06e91a18f48ddf1cae6cbc9946688dea6a161425

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

 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)

 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");
  }

  /**

 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
    {