mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git

boli
23.33.2008 43337ecac9365309ad8cd593611e619f36fab39f
Fixed an issue where deadlocks could occur in the LockManager when the server is in heavy add/mod load. An add operation takes a read lock on the parent DN and a write lock on the target DN in that order. However, mod operations first takes an write lock on the target DN then inadvertantly tries takes an read lock on the parent DN through the entryExists method. This could cause a deadlock in the following case with the fair ordering reentrant read write lock:

Thread 1 is performing a modify operation on cn=dep2,cn=dep1,dc=example,dc=com (entry 2)
Thread 2 is performing an add operation on cn=dep2,cn=dep1,dc=example,dc=com
Thread 3 is performing an modify operation on cn=dep1,dc=example,dc=com (entry 1)

Thread 1 takes a write lock on target entry 2
Thread 2 takes a read lock on parent entry 1
Thread 3 tries to acquire write lock on target entry 1
Thread 2 blocks trying to acquire write lock on target entry 2
Thread 1 blocks trying to acquire read lock on parent entry 1

Threads 1, 2, and 3 deadlocks since thread 3's write lock request is before thread 1's read request in the wait queue.

The entryExists method in Backend.java does not need to acquire an read lock on the DN before checking for the entry's existance in the DB, much like the getEntry method. Removing the lock acquisition in entryExists ensures all locks are acquired down the DIT.

The DependencyTest unit-test now passes and is enabled.

Fix for issue 2852
3 files modified
112 ■■■■■ changed files
opendj-sdk/opends/src/server/org/opends/server/api/Backend.java 30 ●●●●● patch | view | raw | blame | history
opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java 80 ●●●●● patch | view | raw | blame | history
opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/DependencyTest.java 2 ●●● patch | view | raw | blame | history
opendj-sdk/opends/src/server/org/opends/server/api/Backend.java
@@ -33,7 +33,6 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.locks.Lock;
import org.opends.server.admin.Configuration;
import org.opends.server.config.ConfigException;
@@ -56,7 +55,6 @@
import org.opends.server.types.LDIFExportConfig;
import org.opends.server.types.LDIFImportConfig;
import org.opends.server.types.LDIFImportResult;
import org.opends.server.types.LockManager;
import org.opends.server.types.RestoreConfig;
import org.opends.server.types.SearchFilter;
import org.opends.server.types.WritabilityMode;
@@ -486,33 +484,7 @@
  public boolean entryExists(DN entryDN)
         throws DirectoryException
  {
    Lock lock = null;
    for (int i=0; i < 3; i++)
    {
      lock = LockManager.lockRead(entryDN);
      if (lock != null)
      {
        break;
      }
    }
    if (lock == null)
    {
      Message message =
          ERR_BACKEND_CANNOT_LOCK_ENTRY.get(String.valueOf(entryDN));
      throw new DirectoryException(
                     DirectoryServer.getServerErrorResultCode(),
                     message);
    }
    try
    {
      return (getEntry(entryDN) != null);
    }
    finally
    {
      LockManager.unlock(entryDN, lock);
    }
    return (getEntry(entryDN) != null);
  }
opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
@@ -399,7 +399,39 @@
        try
        {
          // Get the entry to modify.  If it does not exist, then fail.
          getEntryToModify();
          currentEntry = backend.getEntry(entryDN);
          if (currentEntry == null)
          {
            setResultCode(ResultCode.NO_SUCH_OBJECT);
            appendErrorMessage(ERR_MODIFY_NO_SUCH_ENTRY.get(
                String.valueOf(entryDN)));
            // See if one of the entry's ancestors exists.
            try
            {
              DN parentDN = entryDN.getParentDNInSuffix();
              while (parentDN != null)
              {
                if (DirectoryServer.entryExists(parentDN))
                {
                  setMatchedDN(parentDN);
                  break;
                }
                parentDN = parentDN.getParentDNInSuffix();
              }
            }
            catch (Exception e)
            {
              if (debugEnabled())
              {
                TRACER.debugCaught(DebugLogLevel.ERROR, e);
              }
            }
            break modifyProcessing;
          }
          // Check to see if there are any controls in the request.  If so, then
          // see if there is any special processing required.
@@ -746,52 +778,6 @@
  /**
   * Gets the entry to modify.
   *
   *
   * @throws  DirectoryException  If a problem occurs while trying to get the
   *                              entry, or if the entry doesn't exist.
   */
  private void getEntryToModify()
          throws DirectoryException
  {
    currentEntry = backend.getEntry(entryDN);
    if (currentEntry == null)
    {
      // See if one of the entry's ancestors exists.
      DN matchedDN = null;
      DN parentDN = entryDN.getParentDNInSuffix();
      while (parentDN != null)
      {
        try
        {
          if (DirectoryServer.entryExists(parentDN))
          {
            matchedDN = parentDN;
            break;
          }
        }
        catch (Exception e)
        {
          if (debugEnabled())
          {
            TRACER.debugCaught(DebugLogLevel.ERROR, e);
          }
          break;
        }
        parentDN = parentDN.getParentDNInSuffix();
      }
      throw new DirectoryException(ResultCode.NO_SUCH_OBJECT,
                     ERR_MODIFY_NO_SUCH_ENTRY.get(String.valueOf(entryDN)),
                     matchedDN, null);
    }
  }
  /**
   * Processes any controls contained in the modify request.
   *
   * @throws  DirectoryException  If a problem is encountered with any of the
opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/DependencyTest.java
@@ -91,7 +91,7 @@
   * all those entries is also correctly ordered.
   */
  @SuppressWarnings("unchecked")
  @Test(enabled=false, groups="slow")
  @Test(enabled=true, groups="slow")
  public void addModDelDependencyTest() throws Exception
  {
    ReplicationServer replServer = null;