From bee45a61482b784454ea880adb95e03d3b6fccb9 Mon Sep 17 00:00:00 2001
From: matthew_swift <matthew_swift@localhost>
Date: Wed, 06 May 2009 20:08:02 +0000
Subject: [PATCH] Fix issue 3962: Memory leaks after Bind <op> Unbind
---
opendj-sdk/opends/src/server/org/opends/server/monitors/ClientConnectionMonitorProvider.java | 164 ++++++++++++++++++--------------
opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPStatistics.java | 26 ----
opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPConnectionHandler.java | 47 +++++++--
opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPClientConnection.java | 5
4 files changed, 136 insertions(+), 106 deletions(-)
diff --git a/opendj-sdk/opends/src/server/org/opends/server/monitors/ClientConnectionMonitorProvider.java b/opendj-sdk/opends/src/server/org/opends/server/monitors/ClientConnectionMonitorProvider.java
index 4155e1a..d34b5a7 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/monitors/ClientConnectionMonitorProvider.java
+++ b/opendj-sdk/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.
+
+ this.handler = handler;
}
@@ -79,9 +94,10 @@
/**
* {@inheritDoc}
*/
+ @Override
public void initializeMonitorProvider(
- ClientConnectionMonitorProviderCfg configuration)
- throws ConfigException, InitializationException
+ ClientConnectionMonitorProviderCfg configuration)
+ throws ConfigException, InitializationException
{
// No initialization is required.
}
@@ -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.
+ * @return The name of this monitor provider.
*/
+ @Override
public String getMonitorInstanceName()
{
- if (this.handler==null) {
- return "Client Connections";
+ if (handler == null)
+ {
+ return "Client Connections";
}
- else {
- // Client connections of a connection handler
- return "Client Connections"+",cn="+
- this.handler.getConnectionHandlerName();
+ else
+ {
+ // Client connections of a connection handler
+ 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.
+ * @return A set of attributes containing monitor data that should be
+ * 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>();
+ TreeMap<Long, ClientConnection> connMap =
+ new TreeMap<Long, ClientConnection>();
- if (this.handler==null) {
- // Get information about all the available connections.
- ArrayList<Collection<ClientConnection>> connCollections =
- new ArrayList<Collection<ClientConnection>>();
- for (ConnectionHandler<?> hdl : DirectoryServer.getConnectionHandlers())
+ if (handler == null)
+ {
+ // Get information about all the available connections.
+ for (ConnectionHandler<?> hdl : DirectoryServer
+ .getConnectionHandlers())
+ {
+ // FIXME: connections from different handlers could have the
+ // same ID.
+ for (ClientConnection conn : hdl.getClientConnections())
{
- ConnectionHandler<? extends ConnectionHandlerCfg> connHandler =
- (ConnectionHandler<? extends ConnectionHandlerCfg>) hdl;
- connCollections.add(connHandler.getClientConnections());
+ connMap.put(conn.getConnectionID(), conn);
}
- for (Collection<ClientConnection> collection : connCollections)
- {
- 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);
- }
+ else
+ {
+ Collection<ClientConnection> collection =
+ 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;
}
}
-
diff --git a/opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPClientConnection.java b/opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPClientConnection.java
index d0cd61f..1360862 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPClientConnection.java
+++ b/opendj-sdk/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)
diff --git a/opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPConnectionHandler.java b/opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPConnectionHandler.java
index 52bf912..8d68346 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPConnectionHandler.java
+++ b/opendj-sdk/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,15 +307,10 @@
// * 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();
- }
+ statTracker.clearStatistics();
}
}
@@ -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);
}
diff --git a/opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPStatistics.java b/opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPStatistics.java
index 2a81285..74e28f0 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/protocols/ldap/LDAPStatistics.java
+++ b/opendj-sdk/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);
}
--
Gitblit v1.10.0