You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by li...@apache.org on 2010/11/10 10:59:28 UTC

svn commit: r1033404 - in /shindig/trunk/java: common/src/main/java/org/apache/shindig/common/cache/ehcache/ common/src/main/java/org/apache/shindig/common/servlet/ common/src/test/java/org/apache/shindig/common/cache/ehcache/ gadgets/src/main/java/org...

Author: lindner
Date: Wed Nov 10 09:59:27 2010
New Revision: 1033404

URL: http://svn.apache.org/viewvc?rev=1033404&view=rev
Log:
SHINDIG-1439, SHINDIG-1447 -- fix Cleanup routines to use Guice mechanisms instead

Modified:
    shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java
    shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheModule.java
    shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
    shindig/trunk/java/common/src/test/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProviderTest.java
    shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java

Modified: shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java?rev=1033404&r1=1033403&r2=1033404&view=diff
==============================================================================
--- shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java (original)
+++ shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProvider.java Wed Nov 10 09:59:27 2010
@@ -21,6 +21,7 @@ package org.apache.shindig.common.cache.
 import com.google.common.base.Preconditions;
 import org.apache.shindig.common.cache.Cache;
 import org.apache.shindig.common.cache.CacheProvider;
+import org.apache.shindig.common.servlet.GuiceServletContextListener;
 import org.apache.shindig.common.util.ResourceLoader;
 
 import com.google.common.collect.MapMaker;
@@ -43,7 +44,7 @@ import java.util.logging.Logger;
  * Cache interface based on ehcache
  * @see <a href="http://www.ehcache.org">http://www.ehcache.org</a>
  */
-public class EhCacheCacheProvider implements CacheProvider {
+public class EhCacheCacheProvider implements CacheProvider, GuiceServletContextListener.CleanupCapable {
   private static final Logger LOG = Logger.getLogger(EhCacheCacheProvider.class.getName());
   private final CacheManager cacheManager;
   private final ConcurrentMap<String, Cache<?, ?>> caches = new MapMaker().makeMap();
@@ -51,10 +52,12 @@ public class EhCacheCacheProvider implem
   @Inject
   public EhCacheCacheProvider(@Named("shindig.cache.ehcache.config") String configPath,
                               @Named("shindig.cache.ehcache.jmx.enabled") boolean jmxEnabled,
-                              @Named("shindig.cache.ehcache.jmx.stats") boolean withCacheStats)
+                              @Named("shindig.cache.ehcache.jmx.stats") boolean withCacheStats,
+                              GuiceServletContextListener.CleanupHandler cleanupHandler)
       throws IOException {
     cacheManager = new CacheManager(getConfiguration(configPath));
     create(jmxEnabled, withCacheStats);
+    cleanupHandler.register(this);
   }
 
   /**
@@ -71,23 +74,8 @@ public class EhCacheCacheProvider implem
   }
 
   public void create(boolean jmxEnabled, boolean withCacheStats) {
-    /*
-     * Add in a shutdown hook
-     */
-    Runtime.getRuntime().addShutdownHook(new Thread() {
-      @Override
-      public void run() {
-        try {
-          shutdown();
-        } catch (Throwable t) {
-          // I really do want to swallow this, and make the shutdown clean for
-          // others
-        }
-      }
-    });
-
-    // register the cache manager with JMX
     if (jmxEnabled) {
+      // register the cache manager with JMX
       MBeanServer mBeanServer = ManagementFactory.getPlatformMBeanServer();
       ManagementService.registerMBeans(cacheManager, mBeanServer, true, true, true, withCacheStats);
     }
@@ -96,7 +84,7 @@ public class EhCacheCacheProvider implem
   /**
    * perform a shutdown
    */
-  public void shutdown() {
+  public void cleanup() {
     cacheManager.shutdown();
   }
 

Modified: shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheModule.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheModule.java?rev=1033404&r1=1033403&r2=1033404&view=diff
==============================================================================
--- shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheModule.java (original)
+++ shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/ehcache/EhCacheModule.java Wed Nov 10 09:59:27 2010
@@ -22,6 +22,9 @@ import org.apache.shindig.common.cache.C
 
 import com.google.inject.AbstractModule;
 import com.google.inject.Scopes;
+import org.apache.shindig.common.servlet.GuiceServletContextListener;
+
+import java.util.concurrent.Callable;
 
 /**
  * Creates a module to supply a EhCache Provider

Modified: shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java?rev=1033404&r1=1033403&r2=1033404&view=diff
==============================================================================
--- shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java (original)
+++ shindig/trunk/java/common/src/main/java/org/apache/shindig/common/servlet/GuiceServletContextListener.java Wed Nov 10 09:59:27 2010
@@ -23,6 +23,7 @@ import com.google.common.collect.Lists;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
 import com.google.inject.Module;
+import com.google.inject.Singleton;
 import com.google.inject.Stage;
 import org.apache.commons.lang.StringUtils;
 
@@ -75,21 +76,28 @@ public class GuiceServletContextListener
 
   public void contextDestroyed(ServletContextEvent event) {
     ServletContext context = event.getServletContext();
+    Injector injector = (Injector) context.getAttribute(INJECTOR_ATTRIBUTE);
+    if (injector != null) {
+        CleanupHandler cleanups = injector.getInstance(CleanupHandler.class);
+        cleanups.cleanup();
+    }
+
     context.removeAttribute(INJECTOR_ATTRIBUTE);
   }
-  
+
+
   /**
    * This method sets all the (key,value) properties specified in the web.xml <contextparam> system.properties element
    * if they are not empty.
-   * @param context
+   * @param context the ServletContext
    */
   private void setSystemProperties(ServletContext context){
     String systemProperties = context.getInitParameter(SYSTEM_PROPERTIES);
     String key=null;
     String value=null;
     if(systemProperties!=null && systemProperties.trim().length()>0){
-      for (String aProperty : StringUtils.split(systemProperties, '\n')){
-      String[] keyAndvalue = StringUtils.split(aProperty.trim(), "=",2);
+      for (String prop : StringUtils.split(systemProperties, '\n')){
+      String[] keyAndvalue = StringUtils.split(prop.trim(), "=",2);
         if(keyAndvalue.length==2){
         key=keyAndvalue[0];
           value=keyAndvalue[1];
@@ -100,6 +108,42 @@ public class GuiceServletContextListener
         }
       }
     }
-  }  
+  }
+
+
+  /**
+   * Interface for classes that need to run cleanup code without
+   * using Runtime ShutdownHooks (which leaks memory on redeploys)
+   */
+  public interface CleanupCapable {
+    /** Execute the cleanup code. */
+    void cleanup();
+  }
+
+  /**
+   * Injectable handler that allows Guice classes to make themselves cleanup capable.
+   */
+  @Singleton
+  public static class CleanupHandler {
+    private List<CleanupCapable> cleanupHandlers = Lists.newArrayList();
+
+    public CleanupHandler() { }
+    /**
+     * Add a new class instance for running cleanup code.
+     *
+     * Best way
+     *
+     * @param cleanupCapable class instance implementing CleanupCapable.
+     */
+    public void register(CleanupCapable cleanupCapable) {
+      cleanupHandlers.add(cleanupCapable);
+    }
+
+    public void cleanup() {
+      for (CleanupCapable cleanupHandler : cleanupHandlers) {
+        cleanupHandler.cleanup();
+      }
+    }
+  }
 }
 

Modified: shindig/trunk/java/common/src/test/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProviderTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/common/src/test/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProviderTest.java?rev=1033404&r1=1033403&r2=1033404&view=diff
==============================================================================
--- shindig/trunk/java/common/src/test/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProviderTest.java (original)
+++ shindig/trunk/java/common/src/test/java/org/apache/shindig/common/cache/ehcache/EhCacheCacheProviderTest.java Wed Nov 10 09:59:27 2010
@@ -21,6 +21,7 @@ package org.apache.shindig.common.cache.
 import org.apache.shindig.common.cache.Cache;
 import org.apache.shindig.common.cache.CacheProvider;
 
+import org.apache.shindig.common.servlet.GuiceServletContextListener;
 import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -34,7 +35,8 @@ public class EhCacheCacheProviderTest {
   @BeforeClass
   public static void setup() throws Exception {
     defaultProvider = new EhCacheCacheProvider(
-        "res://org/apache/shindig/common/cache/ehcache/ehcacheConfig.xml", true, true);
+        "res://org/apache/shindig/common/cache/ehcache/ehcacheConfig.xml", true, true,
+        new GuiceServletContextListener.CleanupHandler());
   }
 
   @Test

Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java?rev=1033404&r1=1033403&r2=1033404&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java Wed Nov 10 09:59:27 2010
@@ -21,6 +21,7 @@ package org.apache.shindig.gadgets;
 import com.google.common.collect.ImmutableList;
 
 import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
 import com.google.inject.Provides;
 import com.google.inject.Singleton;
 import com.google.inject.multibindings.Multibinder;
@@ -29,6 +30,7 @@ import com.google.inject.name.Names;
 
 import org.apache.commons.lang.StringUtils;
 
+import org.apache.shindig.common.servlet.GuiceServletContextListener;
 import org.apache.shindig.gadgets.config.DefaultConfigContributorModule;
 import org.apache.shindig.gadgets.http.HttpResponse;
 import org.apache.shindig.gadgets.http.InvalidationHandler;
@@ -45,10 +47,7 @@ import org.apache.shindig.gadgets.variab
 
 import java.util.List;
 import java.util.Set;
-import java.util.concurrent.Executor;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.*;
 
 /**
  * Creates a module to supply all of the core gadget classes.
@@ -62,19 +61,11 @@ public class DefaultGuiceModule extends 
   @Override
   protected void configure() {
 
-    final ExecutorService service = Executors.newCachedThreadPool(DAEMON_THREAD_FACTORY);
-    bind(ExecutorService.class).toInstance(service);
-    bind(Executor.class).annotatedWith(Names.named("shindig.concat.executor")).toInstance(service);
+    bind(ExecutorService.class).to(ShindigExecutorService.class);
+    bind(Executor.class).annotatedWith(Names.named("shindig.concat.executor")).to(ShindigExecutorService.class);
 
     bindConstant().annotatedWith(Names.named("shindig.jsload.ttl-secs")).to(60 * 60); // 1 hour
 
-    Runtime.getRuntime().addShutdownHook(new Thread() {
-        @Override
-        public void run() {
-            service.shutdownNow();
-        }
-    });
-
     install(new DefaultConfigContributorModule());
     install(new ParseModule());
     install(new PreloadModule());
@@ -125,6 +116,9 @@ public class DefaultGuiceModule extends 
     return ImmutableList.<String>builder().addAll(extended).add(StringUtils.split(features, ',')).build();
   }
 
+  /**
+   * A thread factory that sets the daemon flag to allow for clean servlet shutdown.
+   */
   public static final ThreadFactory DAEMON_THREAD_FACTORY =
     new ThreadFactory() {
         private final ThreadFactory factory = Executors.defaultThreadFactory();
@@ -135,4 +129,24 @@ public class DefaultGuiceModule extends 
             return t;
         }
     };
+
+  /**
+   * An Executor service that mimics Executors.newCachedThreadPool(DAEMON_THREAD_FACTORY);
+   * Registers a cleanup handler to shutdown the thread.
+   */
+  @Singleton
+  public static class ShindigExecutorService extends ThreadPoolExecutor implements GuiceServletContextListener.CleanupCapable {
+    @Inject
+    public ShindigExecutorService(GuiceServletContextListener.CleanupHandler cleanupHandler) {
+      super(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS,
+          new SynchronousQueue<Runnable>(),
+          DAEMON_THREAD_FACTORY);
+          cleanupHandler.register(this);
+    }
+
+    public void cleanup() {
+      this.shutdown();
+    }
+  }
+
 }