Fix issue 3962: Memory leaks after Bind <op> Unbind

matthew_swift

06.08.2009 1050ea9c70d1028b93f3d0784de1b30dd8444226

Fix issue 3962: Memory leaks after Bind <op> Unbind

* avoid creating ClientConnectionMonitorProviders for each new connection

* separate construction of ClientConnectionMonitorProviders and LDAPStatistics

* push registration of ClientConnectionMonitorProviders and LDAPStatistics in LDAPConnectionHandler initialization

* ensure that ClientConnectionMonitorProviders and LDAPStatistics are deregistered when the LDAPConnectionHandler is finalized.

 opends/src/server/org/opends/server/monitors/ClientConnectionMonitorProvider.java

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

@@ -32,27 +32,36 @@
import java.util.Collection;
import java.util.TreeMap;

import org.opends.server.admin.std.server.ConnectionHandlerCfg;
import org.opends.server.admin.std.server.ClientConnectionMonitorProviderCfg;
import org.opends.server.api.ClientConnection;
import org.opends.server.api.ConnectionHandler;
import org.opends.server.api.MonitorProvider;
import org.opends.server.config.ConfigException;
import org.opends.server.core.DirectoryServer;
import org.opends.server.types.*;
import org.opends.server.types.Attribute;
import org.opends.server.types.AttributeBuilder;
import org.opends.server.types.AttributeType;
import org.opends.server.types.AttributeValues;
import org.opends.server.types.InitializationException;



/**
 * This class defines a Directory Server monitor provider that can be used to
 * obtain information about the client connections established to the server.
 * Note that the information reported is obtained with little or no locking, so
 * it may not be entirely consistent, especially for active connections.
 * This class defines a Directory Server monitor provider that can be
 * used to obtain information about the client connections established
 * to the server. Note that the information reported is obtained with
 * little or no locking, so it may not be entirely consistent,
 * especially for active connections.
 */
public class ClientConnectionMonitorProvider
       extends MonitorProvider<ClientConnectionMonitorProviderCfg>
public class ClientConnectionMonitorProvider extends
    MonitorProvider<ClientConnectionMonitorProviderCfg>
{

  private ConnectionHandler<?> handler;
  // The connection handler associated with this monitor, or null if all
  // connection handlers should be monitored.
  private final ConnectionHandler<?> handler;



  /**
   * Creates an instance of this monitor provider.
@@ -60,18 +69,24 @@
  public ClientConnectionMonitorProvider()
  {
    super("Client Connection Monitor Provider");
    // No initialization should be performed here.

    // This will monitor all connection handlers.
    this.handler = null;
  }



  /**
   * Creates an instance of this monitor provider.
   * @param handler to which the monitor provider is associated.
   *
   * @param handler
   *          to which the monitor provider is associated.
   */
  public ClientConnectionMonitorProvider(ConnectionHandler handler)
  {
    super("Client Connection Monitor Provider");

    this.handler=handler;
    // No initialization should be performed here.
  }


@@ -79,6 +94,7 @@
  /**
   * {@inheritDoc}
   */
  @Override
  public void initializeMonitorProvider(
                   ClientConnectionMonitorProviderCfg configuration)
         throws ConfigException, InitializationException
@@ -89,34 +105,41 @@


  /**
   * Retrieves the name of this monitor provider.  It should be unique among all
   * monitor providers, including all instances of the same monitor provider.
   * Retrieves the name of this monitor provider. It should be unique
   * among all monitor providers, including all instances of the same
   * monitor provider.
   *
   * @return  The name of this monitor provider.
   */
  @Override
  public String getMonitorInstanceName()
  {
    if (this.handler==null) {
    if (handler == null)
    {
       return "Client Connections";
    }
    else {
    else
    {
       // Client connections of a connection handler
       return "Client Connections"+",cn="+
               this.handler.getConnectionHandlerName();
      return "Client Connections" + ",cn="
          + handler.getConnectionHandlerName();
    }
  }



  /**
   * Retrieves the length of time in milliseconds that should elapse between
   * calls to the <CODE>updateMonitorData()</CODE> method.  A negative or zero
   * return value indicates that the <CODE>updateMonitorData()</CODE> method
   * should not be periodically invoked.
   * Retrieves the length of time in milliseconds that should elapse
   * between calls to the <CODE>updateMonitorData()</CODE> method. A
   * negative or zero return value indicates that the
   * <CODE>updateMonitorData()</CODE> method should not be periodically
   * invoked.
   *
   * @return  The length of time in milliseconds that should elapse between
   *          calls to the <CODE>updateMonitorData()</CODE> method.
   * @return The length of time in milliseconds that should elapse
   *         between calls to the <CODE>updateMonitorData()</CODE>
   *         method.
   */
  @Override
  public long getUpdateInterval()
  {
    // This monitor does not need to run periodically.
@@ -126,12 +149,13 @@


  /**
   * Performs any processing periodic processing that may be desired to update
   * the information associated with this monitor.  Note that best-effort
   * attempts will be made to ensure that calls to this method come
   * <CODE>getUpdateInterval()</CODE> milliseconds apart, but no guarantees will
   * be made.
   * Performs any processing periodic processing that may be desired to
   * update the information associated with this monitor. Note that
   * best-effort attempts will be made to ensure that calls to this
   * method come <CODE>getUpdateInterval()</CODE> milliseconds apart,
   * but no guarantees will be made.
   */
  @Override
  public void updateMonitorData()
  {
    // This monitor does not need to run periodically.
@@ -141,58 +165,56 @@


  /**
   * Retrieves a set of attributes containing monitor data that should be
   * returned to the client if the corresponding monitor entry is requested.
   * Retrieves a set of attributes containing monitor data that should
   * be returned to the client if the corresponding monitor entry is
   * requested.
   *
   * @return  A set of attributes containing monitor data that should be
   *          returned to the client if the corresponding monitor entry is
   *          requested.
   *         returned to the client if the corresponding monitor entry
   *         is requested.
   */
  @Override
  public ArrayList<Attribute> getMonitorData()
  {
    // Re-order the connections by connection ID.
    TreeMap<Long,ClientConnection> connMap =
             new TreeMap<Long,ClientConnection>();

    if (this.handler==null) {
    if (handler == null)
    {
        // Get information about all the available connections.
        ArrayList<Collection<ClientConnection>> connCollections =
             new ArrayList<Collection<ClientConnection>>();
        for (ConnectionHandler<?> hdl : DirectoryServer.getConnectionHandlers())
      for (ConnectionHandler<?> hdl : DirectoryServer
          .getConnectionHandlers())
        {
          ConnectionHandler<? extends ConnectionHandlerCfg> connHandler =
               (ConnectionHandler<? extends ConnectionHandlerCfg>) hdl;
          connCollections.add(connHandler.getClientConnections());
        // FIXME: connections from different handlers could have the
        // same ID.
        for (ClientConnection conn : hdl.getClientConnections())
        {
          connMap.put(conn.getConnectionID(), conn);
        }
        for (Collection<ClientConnection> collection : connCollections)
      }
    }
    else
        {
      Collection<ClientConnection> collection =
          handler.getClientConnections();
          for (ClientConnection conn : collection)
          {
            connMap.put(conn.getConnectionID(), conn);
          }
        }

    }
    else {
       Collection<ClientConnection> collection =
               this.handler.getClientConnections();
       for (ClientConnection conn : collection) {
            connMap.put(conn.getConnectionID(), conn);
       }
    }


    AttributeType attrType = DirectoryServer
        .getDefaultAttributeType("connection");
    AttributeType attrType =
        DirectoryServer.getDefaultAttributeType("connection");
    AttributeBuilder builder = new AttributeBuilder(attrType);
    for (ClientConnection conn : connMap.values())
    {
      builder.add(AttributeValues.create(attrType, conn.getMonitorSummary()));
      builder.add(AttributeValues.create(attrType, conn
          .getMonitorSummary()));
    }

    ArrayList<Attribute> attrs = new ArrayList<Attribute>(2);
    ArrayList<Attribute> attrs = new ArrayList<Attribute>(1);
    attrs.add(builder.toAttribute());
    return attrs;
  }
}


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

@@ -261,9 +261,7 @@
    String instanceName =
        parentTracker.getMonitorInstanceName() + " for " + toString();
    this.initializeOperationMonitors();
    statTracker =
        new LDAPStatistics(connectionHandler, instanceName,
            parentTracker);
    statTracker = new LDAPStatistics(instanceName, parentTracker);

    if (keepStats)
    {
@@ -2657,6 +2655,7 @@
  /**
   * {@inheritDoc}
   */
  @Override
  public void finishBindOrStartTLS()
  {
    if(this.tlsPendingProvider != null)

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

@@ -33,6 +33,8 @@
import static org.opends.server.loggers.ErrorLogger.logError;
import static org.opends.server.loggers.debug.DebugLogger.*;
import org.opends.server.loggers.debug.DebugTracer;
import org.opends.server.monitors.ClientConnectionMonitorProvider;

import static org.opends.messages.ProtocolMessages.*;

import static org.opends.server.util.ServerConstants.*;
@@ -187,6 +189,10 @@
  // The set of statistics collected for this connection handler.
  private LDAPStatistics statTracker;

  // The client connection monitor provider associated with this
  // connection handler.
  private ClientConnectionMonitorProvider connMonitor;

  // The selector that will be used to multiplex connection acceptance
  // across multiple sockets by a single thread.
  private Selector selector;
@@ -301,17 +307,12 @@
    // * tcp reuse address
    // * num request handler

    // Start/clear the stat tracker if LDAPv2 is being enabled.
    // Clear the stat tracker if LDAPv2 is being enabled.
    if (currentConfig.isAllowLDAPV2() != config.isAllowLDAPV2()) {
      if (config.isAllowLDAPV2()) {
        if (statTracker == null) {
          statTracker = new LDAPStatistics(this,handlerName
              + " Statistics");
        } else {
          statTracker.clearStatistics();
        }
      }
    }

    // Apply the changes.
    currentConfig = config;
@@ -358,11 +359,25 @@
   *          associated with the connection handler should also be
   *          closed.
   */
  @Override
  public void finalizeConnectionHandler(Message finalizeReason,
      boolean closeConnections) {
    shutdownRequested = true;
    currentConfig.removeLDAPChangeListener(this);

    if (connMonitor != null)
    {
      String lowerName =
          toLowerCase(connMonitor.getMonitorInstanceName());
      DirectoryServer.deregisterMonitorProvider(lowerName);
    }

    if (statTracker != null) {
      String lowerName =
        toLowerCase(statTracker.getMonitorInstanceName());
      DirectoryServer.deregisterMonitorProvider(lowerName);
    }

    DirectoryServer.deregisterSupportedLDAPVersion(2, this);
    DirectoryServer.deregisterSupportedLDAPVersion(3, this);

@@ -433,6 +448,7 @@
   * @return The set of active client connections that have been
   *         established through this connection handler.
   */
  @Override
  public Collection<ClientConnection> getClientConnections() {
    LinkedList<ClientConnection> connectionList =
      new LinkedList<ClientConnection>();
@@ -452,6 +468,7 @@
   * @return The DN of the configuration entry with which this alert
   *         generator is associated.
   */
  @Override
  public DN getComponentEntryDN() {
    return currentConfig.dn();
  }
@@ -461,6 +478,7 @@
  /**
   * {@inheritDoc}
   */
  @Override
  public String getConnectionHandlerName() {
    return handlerName;
  }
@@ -498,6 +516,7 @@
  /**
   * {@inheritDoc}
   */
  @Override
  public Collection<HostPort> getListeners() {
    return listeners;
  }
@@ -547,6 +566,7 @@
  /**
   * {@inheritDoc}
   */
  @Override
  public String getProtocol() {
    return protocol;
  }
@@ -595,6 +615,7 @@
  /**
   * {@inheritDoc}
   */
  @Override
  public void initializeConnectionHandler(LDAPConnectionHandlerCfg config)
         throws ConfigException, InitializationException
  {
@@ -648,9 +669,6 @@
    nameBuffer.append(listenPort);
    handlerName = nameBuffer.toString();

    // Perform any additional initialization that might be required.
    statTracker = new LDAPStatistics(this, handlerName + " Statistics");

    // Attempt to bind to the listen port on all configured addresses to
    // verify whether the connection handler will be able to start.
    for (InetAddress a : listenAddresses) {
@@ -693,6 +711,13 @@
      DirectoryServer.registerSupportedLDAPVersion(2, this);
    }

    // Create and register monitors.
    statTracker = new LDAPStatistics(handlerName + " Statistics");
    DirectoryServer.registerMonitorProvider(statTracker);

    connMonitor = new ClientConnectionMonitorProvider(this);
    DirectoryServer.registerMonitorProvider(connMonitor);

    // Register this as a change listener.
    config.addLDAPChangeListener(this);
  }
@@ -812,6 +837,7 @@
   * Operates in a loop, accepting new connections and ensuring that
   * requests on those connections are handled properly.
   */
  @Override
  public void run() {
    setName(handlerName);
    boolean listening = false;
@@ -1124,6 +1150,7 @@
   * @param buffer
   *          The buffer to which the information should be appended.
   */
  @Override
  public void toString(StringBuilder buffer) {
    buffer.append(handlerName);
  }

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

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

@@ -36,11 +36,9 @@

import org.opends.messages.Message;
import org.opends.server.admin.std.server.MonitorProviderCfg;
import org.opends.server.api.ConnectionHandler;
import org.opends.server.api.MonitorProvider;
import org.opends.server.config.ConfigException;
import org.opends.server.core.DirectoryServer;
import org.opends.server.monitors.ClientConnectionMonitorProvider;
import org.opends.server.monitors.OperationMonitor;
import org.opends.server.types.Attribute;
import org.opends.server.types.AttributeBuilder;
@@ -122,9 +120,6 @@
  // The instance name for this monitor provider instance.
  private final String instanceName;

  // Connection Handler to which the statistics belong to.
  private final ConnectionHandler handler;

  // Monitor Objects : for Operations.
  private final OperationMonitor addRequestsMonitor =
      OperationMonitor.getOperationMonitor(ADD);
@@ -152,16 +147,12 @@
  /**
   * Creates a new instance of this class with no parent.
   *
   * @param handler
   *          to which the stats belong to.
   * @param instanceName
   *          The name for this monitor provider instance.
   */
  public LDAPStatistics(ConnectionHandler handler, String instanceName)
  public LDAPStatistics(String instanceName)
  {
    this(handler, instanceName, null);

    DirectoryServer.registerMonitorProvider(this);
    this(instanceName, null);
  }


@@ -169,8 +160,6 @@
  /**
   * Creates a new instance of this class with the specified parent.
   *
   * @param handler
   *          the handler to which the stats belong to.
   * @param instanceName
   *          The name for this monitor provider instance.
   * @param parent
@@ -178,14 +167,12 @@
   *          this class is updated. It may be null if there should not
   *          be a parent.
   */
  public LDAPStatistics(ConnectionHandler handler, String instanceName,
      LDAPStatistics parent)
  public LDAPStatistics(String instanceName, LDAPStatistics parent)
  {
    super("LDAP Statistics Monitor Provider");

    this.instanceName = instanceName;
    this.parent = parent;
    this.handler = handler;

    abandonLock = new Object();
    connectLock = new Object();
@@ -222,11 +209,6 @@
    searchResultReferences = 0;
    searchResultsDone = 0;
    unbindRequests = 0;

    ClientConnectionMonitorProvider connections =
        new ClientConnectionMonitorProvider(this.handler);

    DirectoryServer.registerMonitorProvider(connections);
  }