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

Matthew Swift
10.18.2015 c25504d1f9e2d02afb0f14093a0d16d6b6efb913
OPENDJ-1878: remove unnecessary entry read locks.

Some code was using read-locks before calling Backend.getEntry(DN), but read-locks are no longer required.
11 files modified
233 ■■■■■ changed files
opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/AciHandler.java 13 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/controls/ProxiedAuthV1Control.java 16 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/controls/ProxiedAuthV2Control.java 19 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/extensions/CRAMMD5SASLMechanismHandler.java 16 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/extensions/PlainSASLMechanismHandler.java 16 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/extensions/SASLContext.java 13 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/extensions/SubjectEqualsDNCertificateMapper.java 17 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/types/Entry.java 49 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendBindOperation.java 21 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java 22 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/replication/ReplicationTestCase.java 31 ●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/authorization/dseecompat/AciHandler.java
@@ -30,8 +30,6 @@
import java.util.LinkedList;
import java.util.List;
import java.util.SortedSet;
import java.util.concurrent.locks.Lock;
import org.forgerock.i18n.LocalizableMessage;
import org.forgerock.i18n.slf4j.LocalizedLogger;
import org.forgerock.opendj.config.server.ConfigException;
@@ -914,13 +912,6 @@
   */
  private boolean aciCheckSuperiorEntry(DN superiorDN, ModifyDNOperation op)
  {
    final Lock entryLock = LockManager.lockRead(superiorDN);
    if (entryLock == null)
    {
      logger.warn(WARN_ACI_HANDLER_CANNOT_LOCK_NEW_SUPERIOR_USER, superiorDN);
      return false;
    }
    try
    {
      Entry superiorEntry = DirectoryServer.getEntry(superiorDN);
@@ -936,10 +927,6 @@
    {
      return false;
    }
    finally
    {
      LockManager.unlock(superiorDN, entryLock);
    }
  }
opendj-server-legacy/src/main/java/org/opends/server/controls/ProxiedAuthV1Control.java
@@ -27,8 +27,6 @@
package org.opends.server.controls;
import java.io.IOException;
import java.util.concurrent.locks.Lock;
import org.forgerock.i18n.LocalizableMessage;
import org.opends.server.api.AuthenticationPolicyState;
import org.opends.server.core.DirectoryServer;
@@ -283,15 +281,6 @@
    }
    final Lock entryLock = LockManager.lockRead(authzDN);
    if (entryLock == null)
    {
      throw new DirectoryException(ResultCode.BUSY,
          ERR_PROXYAUTH1_CANNOT_LOCK_USER.get(authzDN));
    }
    try
    {
      Entry userEntry = DirectoryServer.getEntry(authzDN);
      if (userEntry == null)
      {
@@ -329,11 +318,6 @@
      // If we've made it here, then the user is acceptable.
      return userEntry;
    }
    finally
    {
      LockManager.unlock(authzDN, entryLock);
    }
  }
opendj-server-legacy/src/main/java/org/opends/server/controls/ProxiedAuthV2Control.java
@@ -27,8 +27,6 @@
package org.opends.server.controls;
import java.io.IOException;
import java.util.concurrent.locks.Lock;
import org.forgerock.i18n.LocalizableMessage;
import org.opends.server.api.AuthenticationPolicyState;
import org.opends.server.api.IdentityMapper;
@@ -236,22 +234,12 @@
          authzDN = actualDN;
        }
        final Lock entryLock = LockManager.lockRead(authzDN);
        if (entryLock == null)
        {
          throw new DirectoryException(ResultCode.BUSY,
              ERR_PROXYAUTH2_CANNOT_LOCK_USER.get(authzDN));
        }
        try
        {
          Entry userEntry = DirectoryServer.getEntry(authzDN);
          if (userEntry == null)
          {
            // The requested user does not exist.
            LocalizableMessage message = ERR_PROXYAUTH2_NO_SUCH_USER.get(lowerAuthzID);
            throw new DirectoryException(ResultCode.AUTHORIZATION_DENIED,
                                         message);
          throw new DirectoryException(ResultCode.AUTHORIZATION_DENIED, message);
          }
          // FIXME -- We should provide some mechanism for enabling debug
@@ -261,11 +249,6 @@
          // If we've made it here, then the user is acceptable.
          return userEntry;
        }
        finally
        {
          LockManager.unlock(authzDN, entryLock);
        }
      }
    }
    else if (lowerAuthzID.startsWith("u:"))
    {
opendj-server-legacy/src/main/java/org/opends/server/extensions/CRAMMD5SASLMechanismHandler.java
@@ -31,8 +31,6 @@
import java.text.ParseException;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.locks.Lock;
import org.forgerock.i18n.LocalizableMessage;
import org.opends.server.admin.server.ConfigurationChangeListener;
import org.opends.server.admin.std.server.CramMD5SASLMechanismHandlerCfg;
@@ -318,16 +316,6 @@
        userDN = rootDN;
      }
      // Acquire a read lock on the user entry.  If this fails, then so will the
      // authentication.
      final Lock readLock = LockManager.lockRead(userDN);
      if (readLock == null)
      {
        bindOperation.setResultCode(ResultCode.BUSY);
        bindOperation.setAuthFailureReason(INFO_SASLCRAMMD5_CANNOT_LOCK_ENTRY.get(userDN));
        return;
      }
      try
      {
        userEntry = DirectoryServer.getEntry(userDN);
@@ -342,10 +330,6 @@
        bindOperation.setAuthFailureReason(message);
        return;
      }
      finally
      {
        LockManager.unlock(userDN, readLock);
      }
    }
    else
    {
opendj-server-legacy/src/main/java/org/opends/server/extensions/PlainSASLMechanismHandler.java
@@ -32,8 +32,6 @@
import static org.opends.server.util.StaticUtils.*;
import java.util.List;
import java.util.concurrent.locks.Lock;
import org.forgerock.i18n.LocalizableMessage;
import org.opends.server.admin.server.ConfigurationChangeListener;
import org.opends.server.admin.std.server.PlainSASLMechanismHandlerCfg;
@@ -229,16 +227,6 @@
        userDN = rootDN;
      }
      // Acquire a read lock on the user entry.  If this fails, then so will the
      // authentication.
      final Lock readLock = LockManager.lockRead(userDN);
      if (readLock == null)
      {
        bindOperation.setResultCode(ResultCode.BUSY);
        bindOperation.setAuthFailureReason(INFO_SASLPLAIN_CANNOT_LOCK_ENTRY.get(userDN));
        return;
      }
      try
      {
        userEntry = DirectoryServer.getEntry(userDN);
@@ -253,10 +241,6 @@
        bindOperation.setAuthFailureReason(message);
        return;
      }
      finally
      {
        LockManager.unlock(userDN, readLock);
      }
    }
    else
    {
opendj-server-legacy/src/main/java/org/opends/server/extensions/SASLContext.java
@@ -34,8 +34,6 @@
import java.security.PrivilegedExceptionAction;
import java.util.HashMap;
import java.util.List;
import java.util.concurrent.locks.Lock;
import javax.security.auth.Subject;
import javax.security.auth.callback.*;
import javax.security.auth.login.LoginContext;
@@ -861,13 +859,6 @@
   */
  private void getAuthEntry(final DN userDN)
  {
    final Lock readLock = LockManager.lockRead(userDN);
    if (readLock == null)
    {
      setCallbackMsg(INFO_SASL_CANNOT_LOCK_ENTRY.get(userDN));
      return;
    }
    try
    {
      authEntry = DirectoryServer.getEntry(userDN);
@@ -878,10 +869,6 @@
      setCallbackMsg(ERR_SASL_CANNOT_GET_ENTRY_BY_DN.get(
          userDN, SASL_MECHANISM_DIGEST_MD5, e.getMessageObject()));
    }
    finally
    {
      LockManager.unlock(userDN, readLock);
    }
  }
opendj-server-legacy/src/main/java/org/opends/server/extensions/SubjectEqualsDNCertificateMapper.java
@@ -30,8 +30,6 @@
import java.security.cert.Certificate;
import java.security.cert.X509Certificate;
import java.util.concurrent.locks.Lock;
import javax.security.auth.x500.X500Principal;
import org.forgerock.i18n.LocalizableMessage;
@@ -140,16 +138,6 @@
      throw new DirectoryException(ResultCode.INVALID_CREDENTIALS, message);
    }
    // Acquire a read lock on the user entry.  If this fails, then so will the
    // certificate mapping.
    final Lock readLock = LockManager.lockRead(subjectDN);
    if (readLock == null)
    {
      throw new DirectoryException(ResultCode.BUSY, ERR_SEDCM_CANNOT_LOCK_ENTRY.get(subjectDN));
    }
    // Retrieve the entry with the specified DN from the directory.
    Entry userEntry;
    try
@@ -170,11 +158,6 @@
      LocalizableMessage message = ERR_SEDCM_CANNOT_GET_ENTRY.get(subjectDN, getExceptionMessage(e));
      throw new DirectoryException(ResultCode.INVALID_CREDENTIALS, message, e);
    }
    finally
    {
      LockManager.unlock(subjectDN, readLock);
    }
    if (userEntry == null)
    {
opendj-server-legacy/src/main/java/org/opends/server/types/Entry.java
@@ -29,8 +29,6 @@
import java.io.BufferedWriter;
import java.io.IOException;
import java.util.*;
import java.util.concurrent.locks.Lock;
import org.forgerock.i18n.LocalizableMessage;
import org.forgerock.i18n.LocalizableMessageBuilder;
import org.forgerock.i18n.LocalizedIllegalArgumentException;
@@ -2188,24 +2186,6 @@
        DN parentDN = dn.getParentDNInSuffix();
        if (parentDN != null)
        {
          // Get the parent entry and check its structural class.
          final Lock lock = LockManager.lockRead(parentDN);
          if (lock == null)
          {
            LocalizableMessage message = ERR_ENTRY_SCHEMA_DSR_COULD_NOT_LOCK_PARENT.get(dn, parentDN);
            if (structuralPolicy == AcceptRejectWarn.REJECT)
            {
              invalidReason.append(message);
              return false;
            }
            else if (structuralPolicy == AcceptRejectWarn.WARN)
            {
              logger.error(message);
            }
          }
          else
          {
            try
            {
              parentEntry = DirectoryServer.getEntry(parentDN);
@@ -2257,11 +2237,6 @@
                logger.error(message);
              }
            }
            finally
            {
              LockManager.unlock(parentDN, lock);
            }
          }
        }
      }
    }
@@ -2282,25 +2257,6 @@
        DN parentDN = getName().getParentDNInSuffix();
        if (parentDN != null)
        {
          // Get the parent entry and check its structural class.
          final Lock lock = LockManager.lockRead(parentDN);
          if (lock == null)
          {
            LocalizableMessage message =
                ERR_ENTRY_SCHEMA_DSR_COULD_NOT_LOCK_PARENT.get(dn, parentDN);
            if (structuralPolicy == AcceptRejectWarn.REJECT)
            {
              invalidReason.append(message);
              return false;
            }
            else if (structuralPolicy == AcceptRejectWarn.WARN)
            {
              logger.error(message);
            }
          }
          else
          {
            try
            {
              parentEntry = DirectoryServer.getEntry(parentDN);
@@ -2344,11 +2300,6 @@
                logger.error(message);
              }
            }
            finally
            {
              LockManager.unlock(parentDN, lock);
            }
          }
        }
      }
opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendBindOperation.java
@@ -27,8 +27,6 @@
package org.opends.server.workflowelement.localbackend;
import java.util.List;
import java.util.concurrent.locks.Lock;
import org.forgerock.i18n.LocalizableMessage;
import org.forgerock.i18n.LocalizableMessageDescriptor.Arg1;
import org.forgerock.i18n.LocalizableMessageDescriptor.Arg2;
@@ -67,7 +65,7 @@
  /**
   * The backend in which the bind operation should be processed.
   */
  protected Backend backend;
  protected Backend<?> backend;
  /**
   * Indicates whether the bind response should include the first warning
@@ -438,17 +436,6 @@
      bindDN = actualRootDN;
    }
    // Get the user entry based on the bind DN.  If it does not exist,
    // then fail.
    final Lock userLock = LockManager.lockRead(bindDN);
    if (userLock == null)
    {
      throw new DirectoryException(ResultCode.BUSY,
          ERR_BIND_OPERATION_CANNOT_LOCK_USER.get(bindDN));
    }
    try
    {
      Entry userEntry;
      try
      {
@@ -614,12 +601,6 @@
      return true;
    }
    finally
    {
      // No matter what, make sure to unlock the user's entry.
      LockManager.unlock(bindDN, userLock);
    }
  }
opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java
@@ -29,7 +29,6 @@
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.Lock;
import org.forgerock.i18n.LocalizableMessage;
import org.opends.server.api.Backend;
@@ -67,7 +66,7 @@
  /**
   * The backend in which the comparison is to be performed.
   */
  private Backend backend;
  private Backend<?> backend;
  /**
   * The client connection for this operation.
@@ -185,19 +184,8 @@
    // Check for a request to cancel this operation.
    checkIfCanceled(false);
    // Grab a read lock on the entry.
    final Lock readLock = LockManager.lockRead(entryDN);
    try
    {
      if (readLock == null)
      {
        setResultCode(ResultCode.BUSY);
        appendErrorMessage(ERR_COMPARE_CANNOT_LOCK_ENTRY.get(entryDN));
        return;
      }
      // Get the entry. If it does not exist, then fail.
      try
      {
@@ -318,16 +306,8 @@
    catch (DirectoryException de)
    {
      logger.traceException(de);
      setResponseData(de);
    }
    finally
    {
      if (readLock != null)
      {
        LockManager.unlock(entryDN, readLock);
      }
    }
  }
  private DirectoryException newDirectoryException(Entry entry,
opendj-server-legacy/src/test/java/org/opends/server/replication/ReplicationTestCase.java
@@ -539,10 +539,6 @@
    do
    {
      final Lock lock = LockManager.lockRead(dn);
      assertNotNull(lock, "could not lock entry " + dn);
      try
      {
        final Entry newEntry = DirectoryServer.getEntry(dn);
        if (newEntry != null)
        {
@@ -553,14 +549,11 @@
            found = tmpAttr.contains(ByteString.valueOf(valueString));
          }
        }
      }
      finally
      {
        LockManager.unlock(dn, lock);
      }
      if (found != hasAttribute)
      {
        Thread.sleep(100);
      }
    } while ((--count > 0) && (found != hasAttribute));
    return found;
  }
@@ -584,19 +577,12 @@
      count--;
    }
    final Lock lock = LockManager.lockRead(dn);
    assertNotNull(lock, "could not lock entry " + dn);
    try
    {
      Entry entry = DirectoryServer.getEntry(dn);
      if (entry != null)
        return entry.duplicate(true);
      return null;
    }
    finally
    {
      LockManager.unlock(dn, lock);
      return entry.duplicate(true);
    }
    return null;
  }
  /**
@@ -833,10 +819,6 @@
    {
      Thread.sleep(100);
      final Lock lock = LockManager.lockRead(dn);
      assertNotNull(lock, "could not lock entry " + dn);
      try
      {
        Entry newEntry = DirectoryServer.getEntry(dn);
        if (newEntry != null)
        {
@@ -847,11 +829,6 @@
            break;
          }
        }
      }
      finally
      {
        LockManager.unlock(dn, lock);
      }
      count --;
    }
    assertNotNull(found, "Entry: " + dn + " Could not be found.");