You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by hl...@apache.org on 2012/08/07 01:21:44 UTC

git commit: TAP5-1983: PerThreadManager does not cleanup on shutdown, can lead to memory leaks when application redeployed

Updated Branches:
  refs/heads/5.3 e070f6376 -> f9b8a1e37


TAP5-1983: PerThreadManager does not cleanup on shutdown, can lead to memory leaks when application redeployed

Conflicts:

	tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java
	tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PerthreadManagerImpl.java


Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/f9b8a1e3
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/f9b8a1e3
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/f9b8a1e3

Branch: refs/heads/5.3
Commit: f9b8a1e3743e2c562a84d313b6ae5a197b902b79
Parents: e070f63
Author: Howard M. Lewis Ship <hl...@apache.org>
Authored: Mon Aug 6 16:18:12 2012 -0700
Committer: Howard M. Lewis Ship <hl...@apache.org>
Committed: Mon Aug 6 16:21:29 2012 -0700

----------------------------------------------------------------------
 .../tapestry5/ioc/internal/RegistryImpl.java       |   31 ++++++++-------
 .../internal/services/PerthreadManagerImpl.java    |   27 ++++++++++++-
 2 files changed, 43 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/f9b8a1e3/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java
----------------------------------------------------------------------
diff --git a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java
index b61d017..ea1d6e5 100644
--- a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java
+++ b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java
@@ -1,4 +1,4 @@
-// Copyright 2006, 2007, 2008, 2009, 2010, 2011 The Apache Software Foundation
+// Copyright 2006-2012 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -139,7 +139,9 @@ public class RegistryImpl implements Registry, InternalRegistry, ServiceProxyPro
 
         Logger logger = loggerForBuiltinService(PERTHREAD_MANAGER_SERVICE_ID);
 
-        perthreadManager = new PerthreadManagerImpl(logger);
+        PerthreadManagerImpl ptmImpl = new PerthreadManagerImpl(logger);
+
+        perthreadManager = ptmImpl;
 
         final ServiceActivityTrackerImpl scoreboardAndTracker = new ServiceActivityTrackerImpl(perthreadManager);
 
@@ -148,6 +150,7 @@ public class RegistryImpl implements Registry, InternalRegistry, ServiceProxyPro
         logger = loggerForBuiltinService(REGISTRY_SHUTDOWN_HUB_SERVICE_ID);
 
         registryShutdownHub = new RegistryShutdownHubImpl(logger);
+        ptmImpl.registerForShutdown(registryShutdownHub);
 
         lifecycles.put("singleton", new SingletonServiceLifecycle());
 
@@ -267,18 +270,18 @@ public class RegistryImpl implements Registry, InternalRegistry, ServiceProxyPro
         // Match services with the correct interface AND having as markers *all* the marker annotations
 
         Flow<ServiceDef2> filtered = serviceDefs.filter(F.and(new Predicate<ServiceDef2>()
-                {
-                    public boolean accept(ServiceDef2 object)
-                    {
-                        return object.getServiceInterface().equals(cd.getServiceInterface());
-                    }
-                }, new Predicate<ServiceDef2>()
-        {
-            public boolean accept(ServiceDef2 serviceDef)
-            {
-                return serviceDef.getMarkers().containsAll(contributionMarkers);
-            }
-        }
+                                                              {
+                                                                  public boolean accept(ServiceDef2 object)
+                                                                  {
+                                                                      return object.getServiceInterface().equals(cd.getServiceInterface());
+                                                                  }
+                                                              }, new Predicate<ServiceDef2>()
+                                                              {
+                                                                  public boolean accept(ServiceDef2 serviceDef)
+                                                                  {
+                                                                      return serviceDef.getMarkers().containsAll(contributionMarkers);
+                                                                  }
+                                                              }
         ));
 
         // That's a lot of logic; the good news is it will short-circuit as soon as it finds a single match,

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/f9b8a1e3/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PerthreadManagerImpl.java
----------------------------------------------------------------------
diff --git a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PerthreadManagerImpl.java b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PerthreadManagerImpl.java
index 54ef8f6..984edda 100644
--- a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PerthreadManagerImpl.java
+++ b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/PerthreadManagerImpl.java
@@ -1,4 +1,4 @@
-// Copyright 2006, 2007, 2008, 2010, 2011 The Apache Software Foundation
+// Copyright 2006-2012 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -19,11 +19,13 @@ import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
 import org.apache.tapestry5.ioc.internal.util.JDKUtils;
 import org.apache.tapestry5.ioc.services.PerThreadValue;
 import org.apache.tapestry5.ioc.services.PerthreadManager;
+import org.apache.tapestry5.ioc.services.RegistryShutdownHub;
 import org.apache.tapestry5.ioc.services.ThreadCleanupListener;
 import org.slf4j.Logger;
 
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.locks.Lock;
 
@@ -49,6 +51,8 @@ public class PerthreadManagerImpl implements PerthreadManager
 
     private final AtomicInteger uuidGenerator = new AtomicInteger();
 
+    private final AtomicBoolean shutdown = new AtomicBoolean();
+
     public PerthreadManagerImpl(Logger logger)
     {
         this.logger = logger;
@@ -56,8 +60,29 @@ public class PerthreadManagerImpl implements PerthreadManager
         listenersValue = createValue();
     }
 
+    public void registerForShutdown(RegistryShutdownHub hub)
+    {
+        hub.addRegistryShutdownListener(new Runnable()
+        {
+            @Override
+            public void run()
+            {
+                cleanup();
+                shutdown.set(true);
+            }
+        });
+    }
+
     private Map getPerthreadMap()
     {
+        // This is a degenerate case; it may not even exist; but if during registry shutdown somehow code executes
+        // that attempts to create new values or add new listeners, those go into a new map instance that is
+        // not referenced (and so immediately GCed).
+        if (shutdown.get())
+        {
+            return CollectionFactory.newMap();
+        }
+
         lock.lock();
 
         try