pool: Fix memory leak in xrootd and httpd movers

Addresses a slow memory leak that occurs whenever more than one httpd or more than one xrootd transfer is started on a pool. Also reduces http/xrootd server component startup overhead. Target: trunk Request: 2.6 Require-notes: yes Require-book: no Acked-by: Paul Millar <paul.millar@desy.de> Patch: http://rb.dcache.org/r/5695/
dCache · Jul 4, 2013 · 7679d8d · 7679d8d
1 parent 59c9af5
commit 7679d8d
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 28 deletions.
diff --git a/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/pool/XrootdPoolNettyServer.java b/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/pool/XrootdPoolNettyServer.java
@@ -10,7 +10,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
@@ -42,7 +41,7 @@ public class XrootdPoolNettyServer
     /**
      * Used to generate channel-idle events for the pool handler
      */
-    private Timer _timer;
+    private final Timer _timer;
 
     private final long _clientIdleTimeout;
     private final int _maxFrameSize;
@@ -76,6 +75,7 @@ public XrootdPoolNettyServer(int threadPoolSize,
         _clientIdleTimeout = clientIdleTimeout;
         _maxFrameSize = maxFrameSize;
         _plugins = plugins;
+        _timer = new HashedWheelTimer();
 
         String range = System.getProperty("org.globus.tcp.port.range");
         PortRange portRange =
@@ -88,19 +88,10 @@ public int getMaxFrameSize()
         return _maxFrameSize;
     }
 
-    @Override
-    protected synchronized void startServer() throws IOException
-    {
-        _timer = new HashedWheelTimer();
-        super.startServer();
-    }
-
-    @Override
-    protected synchronized void stopServer()
+    public void shutdown()
     {
-        super.stopServer();
+        stopServer();
         _timer.stop();
-        _timer = null;
     }
 
     @Override

diff --git a/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/pool/XrootdTransferService.java b/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/pool/XrootdTransferService.java
@@ -23,6 +23,7 @@
 import org.springframework.beans.factory.annotation.Required;
 
 import javax.annotation.PostConstruct;
+import javax.annotation.PreDestroy;
 
 import java.io.IOException;
 import java.net.InetAddress;
@@ -196,7 +197,7 @@ public List<ChannelHandlerFactory> getPlugins()
     }
 
     @PostConstruct
-    private synchronized void init() throws Exception
+    public synchronized void init()
     {
         if (socketThreads == null) {
             server = new XrootdPoolNettyServer(
@@ -218,6 +219,14 @@ private synchronized void init() throws Exception
         }
     }
 
+    @PreDestroy
+    public synchronized void shutdown()
+    {
+        if (server != null) {
+            server.shutdown();
+        }
+    }
+
     @Override
     public Mover<?> createMover(ReplicaDescriptor handle, PoolIoFileMessage message,
                              CellPath pathToDoor) throws CacheException

diff --git a/modules/dcache/src/main/java/org/dcache/http/HttpPoolNettyServer.java b/modules/dcache/src/main/java/org/dcache/http/HttpPoolNettyServer.java
@@ -13,7 +13,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
 import java.util.concurrent.TimeUnit;
 
 import diskCacheV111.vehicles.HttpProtocolInfo;
@@ -39,7 +38,7 @@ public class HttpPoolNettyServer
     private final static PortRange DEFAULT_PORTRANGE =
         new PortRange(20000, 25000);
 
-    private Timer _timer;
+    private final Timer _timer;
 
     private final long _clientIdleTimeout;
 
@@ -68,26 +67,18 @@ public HttpPoolNettyServer(int threadPoolSize,
 
         _clientIdleTimeout = clientIdleTimeout;
         _chunkSize = chunkSize;
+        _timer = new HashedWheelTimer();
 
         String range = System.getProperty("org.globus.tcp.port.range");
         PortRange portRange =
             (range != null) ? PortRange.valueOf(range) : DEFAULT_PORTRANGE;
         setPortRange(portRange);
     }
 
-    @Override
-    protected synchronized void startServer() throws IOException
-    {
-        _timer = new HashedWheelTimer();
-        super.startServer();
-    }
-
-    @Override
-    protected synchronized void stopServer()
+    public void shutdown()
     {
-        super.stopServer();
+        stopServer();
         _timer.stop();
-        _timer = null;
     }
 
     @Override

diff --git a/modules/dcache/src/main/java/org/dcache/http/HttpTransferService.java b/modules/dcache/src/main/java/org/dcache/http/HttpTransferService.java
@@ -23,6 +23,7 @@
 import org.springframework.beans.factory.annotation.Required;
 
 import javax.annotation.PostConstruct;
+import javax.annotation.PreDestroy;
 
 import java.io.IOException;
 import java.net.InetAddress;
@@ -189,7 +190,7 @@ public void setSocketThreads(String socketThreads)
     }
 
     @PostConstruct
-    public void init()
+    public synchronized void init()
     {
         if (socketThreads == null) {
             server = new HttpPoolNettyServer(diskThreads,
@@ -207,6 +208,14 @@ public void init()
         }
     }
 
+    @PreDestroy
+    public synchronized void shutdown()
+    {
+        if (server != null) {
+            server.shutdown();
+        }
+    }
+
     @Override
     public Mover<?> createMover(ReplicaDescriptor handle, PoolIoFileMessage message,
                                 CellPath pathToDoor) throws CacheException