Fix potential deadlock which may occur while performing simultaneous large ...

matthew_swift

18.16.2010 bf7e97e3cb610d18d4fbe62e614cdb55c2d24930

Fix potential deadlock which may occur while performing simultaneous large searches and modify operations on the same connection. Use single channel lock per connection and share it across thread local ASN1 writers.

 opends/src/server/org/opends/server/protocols/asn1/ASN1.java

@@ -22,7 +22,7 @@
 * CDDL HEADER END
 *
 *
 *      Copyright 2006-2008 Sun Microsystems, Inc.
 *      Copyright 2006-2010 Sun Microsystems, Inc.
 */
package org.opends.server.protocols.asn1;

@@ -32,6 +32,7 @@
import java.io.OutputStream;
import java.nio.channels.ReadableByteChannel;
import java.nio.channels.WritableByteChannel;
import java.util.concurrent.locks.ReentrantLock;

import org.opends.server.types.ByteSequence;
import org.opends.server.types.ByteString;
@@ -263,11 +264,14 @@
   *
   * @param channel
   *          The writable byte channel.
   * @param writeLock
   *          The write lock to use when flushing to the destination.
   * @return The new ASN.1 writer.
   */
  public static ASN1Writer getWriter(WritableByteChannel channel)
  public static ASN1Writer getWriter(WritableByteChannel channel,
                                     ReentrantLock writeLock)
  {
    return new ASN1ByteChannelWriter(channel, 4096);
    return new ASN1ByteChannelWriter(channel, writeLock, 4096);
  }


@@ -282,14 +286,17 @@
   *
   * @param channel
   *          The writable byte channel.
   * @param writeLock
   *          The write lock to use when flushing to the destination.
   * @param bufferSize
   *          The buffer size to use when writing to the channel.
   * @return The new ASN.1 writer.
   */
  public static ASN1Writer getWriter(WritableByteChannel channel,
                                     ReentrantLock writeLock,
                                     int bufferSize)
  {
    return new ASN1ByteChannelWriter(channel, bufferSize);
    return new ASN1ByteChannelWriter(channel, writeLock, bufferSize);
  }



 opends/src/server/org/opends/server/protocols/asn1/ASN1ByteChannelWriter.java

@@ -22,7 +22,7 @@
 * CDDL HEADER END
 *
 *
 *      Copyright 2006-2009 Sun Microsystems, Inc.
 *      Copyright 2006-2010 Sun Microsystems, Inc.
 */
package org.opends.server.protocols.asn1;

@@ -115,14 +115,16 @@
   * Constructs a new ASN1ByteChannelWriter.
   *
   * @param byteChannel The WritableByteChannel to write to.
   * @param writeLock The write lock to use when flushing to the destination.
   * @param writeBufferSize The NIO ByteBuffer size.
   */
  ASN1ByteChannelWriter(WritableByteChannel byteChannel,
                               int writeBufferSize)
                        ReentrantLock writeLock,
                        int writeBufferSize)
  {
    this.byteChannel = byteChannel;
    this.byteBuffer = ByteBuffer.allocate(writeBufferSize);
    this.flushLock = new ReentrantLock(false);
    this.flushLock = writeLock;

    ByteBufferOutputStream bufferStream = new ByteBufferOutputStream();
    this.writer = new ASN1OutputStreamWriter(bufferStream);
@@ -319,12 +321,12 @@
  public void flush() throws IOException
  {
    byteBuffer.flip();
    if (!flushLock.isHeldByCurrentThread())
    {
      flushLock.lock();
    }
    try
    {
      if (!flushLock.isHeldByCurrentThread())
      {
        flushLock.lock();
      }
      byteChannel.write(byteBuffer);
    }
    finally
@@ -347,9 +349,17 @@
    {
      flushLock.lock();
    }
    while(byteBuffer.hasRemaining())
    try
    {
      byteChannel.write(byteBuffer);
      while (byteBuffer.hasRemaining())
      {
        byteChannel.write(byteBuffer);
      }
    }
    catch (IOException e)
    {
      flushLock.unlock();
      throw e;
    }
    byteBuffer.clear();
  }

 opends/src/server/org/opends/server/protocols/ldap/LDAPClientConnection.java

@@ -51,6 +51,7 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.ReentrantLock;

import org.opends.messages.Message;
import org.opends.messages.MessageBuilder;
@@ -362,6 +363,7 @@

  private final RedirectingByteChannel saslChannel;
  private final RedirectingByteChannel tlsChannel;
  private final ReentrantLock writeLock;
  private volatile ConnectionSecurityProvider activeProvider = null;
  private volatile ConnectionSecurityProvider tlsPendingProvider = null;
  private volatile ConnectionSecurityProvider saslPendingProvider = null;
@@ -427,6 +429,7 @@
    this.asn1Reader =
        ASN1.getReader(saslChannel, APPLICATION_BUFFER_SIZE, connectionHandler
            .getMaxRequestSize());
    writeLock = new ReentrantLock();

    asn1WriterMap = new ConcurrentHashMap<Thread,ASN1Writer>();

@@ -933,11 +936,11 @@
        if (isSecure())
        {
          int appBufSize = activeProvider.getAppBufSize();
          asn1Writer = ASN1.getWriter(saslChannel, appBufSize);
          asn1Writer = ASN1.getWriter(saslChannel, writeLock, appBufSize);
        }
        else
        {
          asn1Writer = ASN1.getWriter(saslChannel,
          asn1Writer = ASN1.getWriter(saslChannel, writeLock,
                  APPLICATION_BUFFER_SIZE);
        }
        asn1WriterMap.put(currentThread, asn1Writer);

 opends/tests/unit-tests-testng/src/server/org/opends/server/protocols/asn1/ASN1ByteChannelWriterTestCase.java

@@ -22,7 +22,7 @@
 * CDDL HEADER END
 *
 *
 *      Copyright 2006-2009 Sun Microsystems, Inc.
 *      Copyright 2006-2010 Sun Microsystems, Inc.
 */
package org.opends.server.protocols.asn1;

@@ -32,6 +32,7 @@
import java.nio.channels.Channels;
import java.nio.channels.ReadableByteChannel;
import java.nio.channels.WritableByteChannel;
import java.util.concurrent.locks.ReentrantLock;

/**
 * Test class for ASN1ByteChannelWriter
@@ -40,7 +41,8 @@
{
  private ByteArrayOutputStream outStream = new ByteArrayOutputStream();
  private WritableByteChannel outChannel = Channels.newChannel(outStream);
  private ASN1Writer writer = new ASN1ByteChannelWriter(outChannel, 500);
  private ASN1Writer writer = new ASN1ByteChannelWriter(outChannel,
      new ReentrantLock(), 500);

  @Override
  ASN1Writer getWriter() throws IOException