You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tomee.apache.org by da...@apache.org on 2008/01/17 03:12:09 UTC

svn commit: r612681 - in /openejb/trunk/openejb3/container/openejb-core/src: main/java/org/apache/openejb/ main/java/org/apache/openejb/assembler/classic/ main/java/org/apache/openejb/persistence/ test/java/org/apache/openejb/core/cmp/jpa/

Author: dain
Date: Wed Jan 16 18:12:06 2008
New Revision: 612681

URL: http://svn.apache.org/viewvc?rev=612681&view=rev
Log:
OPENEJB-746 Command line tool Deploy/Undeploy asymmetry
Fixed memory leak in OpenJPA which caused class loaders to not be garbage collected.

Modified:
    openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/ClassLoaderUtil.java
    openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java
    openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/PersistenceBuilder.java
    openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/persistence/PersistenceClassLoaderHandler.java
    openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/persistence/PersistenceUnitInfoImpl.java
    openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/cmp/jpa/JpaTest.java
    openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/cmp/jpa/UnenhancedTest.java

Modified: openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/ClassLoaderUtil.java
URL: http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/ClassLoaderUtil.java?rev=612681&r1=612680&r2=612681&view=diff
==============================================================================
--- openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/ClassLoaderUtil.java (original)
+++ openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/ClassLoaderUtil.java Wed Jan 16 18:12:06 2008
@@ -22,6 +22,7 @@
 import java.io.ObjectOutputStream;
 import java.io.ObjectStreamClass;
 import java.lang.reflect.Field;
+import java.lang.reflect.Method;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.Map;
@@ -74,6 +75,16 @@
             synchronized (cache) {
                 cache.clear();
             }
+        }
+    }
+
+    public static void cleanOpenJPACache(ClassLoader classLoader) {
+        try {
+            Class<?> pcRegistryClass = classLoader.loadClass("org.apache.openjpa.enhance.PCRegistry");
+            Method deRegisterMethod = pcRegistryClass.getMethod("deRegister", ClassLoader.class);
+            deRegisterMethod.invoke(null, classLoader);
+        } catch (Throwable ignored) {
+            // there is nothing a user could do about this anyway
         }
     }
 }

Modified: openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java
URL: http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java?rev=612681&r1=612680&r2=612681&view=diff
==============================================================================
--- openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java (original)
+++ openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java Wed Jan 16 18:12:06 2008
@@ -16,6 +16,45 @@
  */
 package org.apache.openejb.assembler.classic;
 
+import java.io.File;
+import java.io.IOException;
+import java.lang.instrument.ClassFileTransformer;
+import java.lang.instrument.Instrumentation;
+import java.lang.reflect.Method;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.TreeMap;
+import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.atomic.AtomicInteger;
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+import javax.persistence.EntityManagerFactory;
+import javax.resource.spi.BootstrapContext;
+import javax.resource.spi.ConnectionManager;
+import javax.resource.spi.ManagedConnectionFactory;
+import javax.resource.spi.ResourceAdapter;
+import javax.resource.spi.ResourceAdapterInternalException;
+import javax.resource.spi.work.WorkManager;
+import javax.transaction.TransactionManager;
+import javax.transaction.TransactionSynchronizationRegistry;
+
+import org.apache.geronimo.connector.GeronimoBootstrapContext;
+import org.apache.geronimo.connector.work.GeronimoWorkManager;
+import org.apache.geronimo.transaction.manager.GeronimoTransactionManager;
+import org.apache.openejb.BeanType;
+import org.apache.openejb.ClassLoaderUtil;
 import org.apache.openejb.Container;
 import org.apache.openejb.DeploymentInfo;
 import org.apache.openejb.DuplicateDeploymentIdException;
@@ -25,21 +64,19 @@
 import org.apache.openejb.OpenEJB;
 import org.apache.openejb.OpenEJBException;
 import org.apache.openejb.UndeployException;
-import org.apache.openejb.BeanType;
-import org.apache.openejb.ClassLoaderUtil;
-import org.apache.openejb.resource.GeronimoConnectionManagerFactory;
 import org.apache.openejb.core.ConnectorReference;
 import org.apache.openejb.core.CoreContainerSystem;
 import org.apache.openejb.core.CoreDeploymentInfo;
 import org.apache.openejb.core.SimpleTransactionSynchronizationRegistry;
 import org.apache.openejb.core.TemporaryClassLoader;
+import org.apache.openejb.core.ivm.naming.IvmContext;
 import org.apache.openejb.core.timer.EjbTimerServiceImpl;
 import org.apache.openejb.core.timer.NullEjbTimerServiceImpl;
-import org.apache.openejb.core.ivm.naming.IvmContext;
 import org.apache.openejb.javaagent.Agent;
 import org.apache.openejb.loader.SystemInstance;
 import org.apache.openejb.persistence.JtaEntityManagerRegistry;
 import org.apache.openejb.persistence.PersistenceClassLoaderHandler;
+import org.apache.openejb.resource.GeronimoConnectionManagerFactory;
 import org.apache.openejb.spi.ApplicationServer;
 import org.apache.openejb.spi.ContainerSystem;
 import org.apache.openejb.spi.SecurityService;
@@ -50,45 +87,9 @@
 import org.apache.openejb.util.proxy.ProxyFactory;
 import org.apache.openejb.util.proxy.ProxyManager;
 import org.apache.xbean.recipe.ObjectRecipe;
-import org.apache.xbean.recipe.StaticRecipe;
 import org.apache.xbean.recipe.Option;
+import org.apache.xbean.recipe.StaticRecipe;
 import org.apache.xbean.recipe.UnsetPropertiesRecipe;
-import org.apache.geronimo.connector.work.GeronimoWorkManager;
-import org.apache.geronimo.connector.GeronimoBootstrapContext;
-import org.apache.geronimo.transaction.manager.GeronimoTransactionManager;
-
-import javax.naming.Context;
-import javax.naming.InitialContext;
-import javax.naming.NamingException;
-import javax.persistence.EntityManagerFactory;
-import javax.resource.spi.ConnectionManager;
-import javax.resource.spi.ManagedConnectionFactory;
-import javax.resource.spi.BootstrapContext;
-import javax.resource.spi.ResourceAdapterInternalException;
-import javax.resource.spi.ResourceAdapter;
-import javax.resource.spi.work.WorkManager;
-import javax.transaction.TransactionManager;
-import javax.transaction.TransactionSynchronizationRegistry;
-import java.io.File;
-import java.io.IOException;
-import java.lang.instrument.ClassFileTransformer;
-import java.lang.instrument.Instrumentation;
-import java.lang.reflect.Method;
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.net.URLClassLoader;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Properties;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.concurrent.atomic.AtomicInteger;
-
-import java.util.concurrent.Executor;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ThreadFactory;
 
 public class Assembler extends AssemblerTool implements org.apache.openejb.spi.Assembler {
 
@@ -96,6 +97,7 @@
 
     private final CoreContainerSystem containerSystem;
     private final PersistenceClassLoaderHandler persistenceClassLoaderHandler;
+    private final ClassLoaderRegistry classLoaderRegistry;;
     private final JndiBuilder jndiBuilder;
     private TransactionManager transactionManager;
     private SecurityService securityService;
@@ -162,6 +164,8 @@
     public Assembler() {
         persistenceClassLoaderHandler = new PersistenceClassLoaderHandlerImpl();
 
+        classLoaderRegistry = new ClassLoaderRegistry();
+
         installNaming();
 
         SystemInstance system = SystemInstance.get();
@@ -436,11 +440,14 @@
         }
 
         try {
+            classLoaderRegistry.registerClassLoader(appInfo.jarPath, classLoader);
+
             // Generate the cmp2 concrete subclasses
             CmpJarBuilder cmpJarBuilder = new CmpJarBuilder(appInfo, classLoader);
             File generatedJar = cmpJarBuilder.getJarFile();
             if (generatedJar != null) {
                 classLoader = new URLClassLoader(new URL []{generatedJar.toURL()}, classLoader);
+                classLoaderRegistry.registerClassLoader(appInfo.jarPath, classLoader);
             }
 
             // JPA - Persistence Units MUST be processed first since they will add ClassFileTransformers
@@ -648,7 +655,12 @@
 
         for (PersistenceUnitInfo unitInfo : appInfo.persistenceUnits) {
             try {
+                Object object = globalContext.lookup("openejb/PersistenceUnit/" + unitInfo.id);
                 globalContext.unbind("openejb/PersistenceUnit/"+unitInfo.id);
+
+                // close EMF so all resources are released
+                ((EntityManagerFactory) object).close();
+                persistenceClassLoaderHandler.destroy(unitInfo.id);
             } catch (Throwable t) {
                 undeployException.getCauses().add(new Exception("persistence-unit: " + unitInfo.id + ": " + t.getMessage(), t));
             }
@@ -684,6 +696,7 @@
             }
         }
 
+        classLoaderRegistry.unregisterClassLoaders(appInfo.jarPath);
         ClassLoaderUtil.clearClassLoaderCaches();
 
         if (undeployException.getCauses().size() > 0) {
@@ -1113,17 +1126,84 @@
     }
 
     private static class PersistenceClassLoaderHandlerImpl implements PersistenceClassLoaderHandler {
-        public void addTransformer(ClassLoader classLoader, ClassFileTransformer classFileTransformer) {
+        private final Map<String,List<ClassFileTransformer>> transformers = new TreeMap<String,List<ClassFileTransformer>>();
+
+        public void addTransformer(String unitId, ClassLoader classLoader, ClassFileTransformer classFileTransformer) {
             Instrumentation instrumentation = Agent.getInstrumentation();
             if (instrumentation != null) {
                 instrumentation.addTransformer(classFileTransformer);
+
+                if (unitId != null) {
+                    List<ClassFileTransformer> transformers = this.transformers.get(unitId);
+                    if (transformers == null) {
+                        transformers = new ArrayList<ClassFileTransformer>(1);
+                        this.transformers.put(unitId, transformers);
+                    }
+                    transformers.add(classFileTransformer);
+                }
             } else {
                 logger.error("assembler.noAgent");
             }
         }
 
+        public void destroy(String unitId) {
+            List<ClassFileTransformer> transformers = this.transformers.remove(unitId);
+            if (transformers != null) {
+                Instrumentation instrumentation = Agent.getInstrumentation();
+                if (instrumentation != null) {
+                    for (ClassFileTransformer transformer : transformers) {
+                        instrumentation.removeTransformer(transformer);
+                    }
+                } else {
+                    logger.error("assembler.noAgent");
+                }
+            }
+        }
+
         public ClassLoader getNewTempClassLoader(ClassLoader classLoader) {
             return new TemporaryClassLoader(classLoader);
+        }
+    }
+
+    // todo remove this when OpenJPA fixes it ClassLoader cache
+    private static class ClassLoaderRegistry {
+        private final Map<String,List<ClassLoader>> classLoadersByApp = new HashMap<String,List<ClassLoader>>();
+        private final Map<ClassLoader,List<String>> appsByClassLoader = new HashMap<ClassLoader,List<String>>();
+
+        public void registerClassLoader(String appId, ClassLoader classLoader) {
+            List<ClassLoader> classLoaders = classLoadersByApp.get(appId);
+            if (classLoaders == null) {
+                classLoaders = new ArrayList<ClassLoader>(2);
+                classLoadersByApp.put(appId, classLoaders);
+            }
+            classLoaders.add(classLoader);
+
+            List<String> apps = appsByClassLoader.get(classLoader);
+            if (apps == null) {
+                apps = new ArrayList<String>(1);
+                appsByClassLoader.put(classLoader, apps);
+            }
+            apps.add(appId);
+        }
+
+        public void unregisterClassLoaders(String appId) {
+            List<ClassLoader> classLoaders = classLoadersByApp.remove(appId);
+            if (classLoaders != null) {
+                for (ClassLoader classLoader : classLoaders) {
+                    // get the apps using the class loader
+                    List<String> apps = appsByClassLoader.get(classLoader);
+                    if (apps == null) apps = Collections.emptyList();
+
+                    // this app is no longer using the class loader
+                    apps.remove(appId);
+
+                    // if no apps are using the class loader, clear the OpenJPA cache
+                    if (apps.isEmpty()) {
+                        appsByClassLoader.remove(classLoader);
+                        ClassLoaderUtil.cleanOpenJPACache(classLoader);
+                    }
+                }
+            }
         }
     }
 

Modified: openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/PersistenceBuilder.java
URL: http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/PersistenceBuilder.java?rev=612681&r1=612680&r2=612681&view=diff
==============================================================================
--- openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/PersistenceBuilder.java (original)
+++ openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/PersistenceBuilder.java Wed Jan 16 18:12:06 2008
@@ -85,6 +85,9 @@
     public EntityManagerFactory createEntityManagerFactory(PersistenceUnitInfo info, ClassLoader classLoader) throws Exception {
         PersistenceUnitInfoImpl unitInfo = new PersistenceUnitInfoImpl(persistenceClassLoaderHandler);
 
+        // Persistence Unit Id
+        unitInfo.setId(info.id);
+
         // Persistence Unit Name
         unitInfo.setPersistenceUnitName(info.name);
 

Modified: openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/persistence/PersistenceClassLoaderHandler.java
URL: http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/persistence/PersistenceClassLoaderHandler.java?rev=612681&r1=612680&r2=612681&view=diff
==============================================================================
--- openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/persistence/PersistenceClassLoaderHandler.java (original)
+++ openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/persistence/PersistenceClassLoaderHandler.java Wed Jan 16 18:12:06 2008
@@ -20,6 +20,7 @@
 import java.lang.instrument.ClassFileTransformer;
 
 public interface PersistenceClassLoaderHandler {
-    void addTransformer(ClassLoader classLoader, ClassFileTransformer classFileTransformer);
+    void addTransformer(String unitId, ClassLoader classLoader, ClassFileTransformer classFileTransformer);
+    void destroy(String unitId);
     ClassLoader getNewTempClassLoader(ClassLoader classLoader);
 }

Modified: openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/persistence/PersistenceUnitInfoImpl.java
URL: http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/persistence/PersistenceUnitInfoImpl.java?rev=612681&r1=612680&r2=612681&view=diff
==============================================================================
--- openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/persistence/PersistenceUnitInfoImpl.java (original)
+++ openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/persistence/PersistenceUnitInfoImpl.java Wed Jan 16 18:12:06 2008
@@ -16,7 +16,6 @@
  */
 package org.apache.openejb.persistence;
 
-import java.io.File;
 import java.lang.instrument.ClassFileTransformer;
 import java.lang.instrument.IllegalClassFormatException;
 import java.net.MalformedURLException;
@@ -39,6 +38,11 @@
     private final PersistenceClassLoaderHandler persistenceClassLoaderHandler;
 
     /**
+     * The unique id of this persistence unit.
+     */
+    private String id;
+
+    /**
      * Name of this persistence unit.  The JPA specification has restrictions on the
      * uniqueness of this name.
      */
@@ -110,6 +114,14 @@
         this.persistenceClassLoaderHandler = persistenceClassLoaderHandler;
     }
 
+    public String getId() {
+        return id;
+    }
+
+    public void setId(String id) {
+        this.id = id;
+    }
+
     public String getPersistenceUnitName() {
         return persistenceUnitName;
     }
@@ -233,7 +245,7 @@
     public void addTransformer(ClassTransformer classTransformer) {
         if (persistenceClassLoaderHandler != null) {
             PersistenceClassFileTransformer classFileTransformer = new PersistenceClassFileTransformer(classTransformer);
-            persistenceClassLoaderHandler.addTransformer(classLoader, classFileTransformer);
+            persistenceClassLoaderHandler.addTransformer(id, classLoader, classFileTransformer);
         }
     }
 

Modified: openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/cmp/jpa/JpaTest.java
URL: http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/cmp/jpa/JpaTest.java?rev=612681&r1=612680&r2=612681&view=diff
==============================================================================
--- openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/cmp/jpa/JpaTest.java (original)
+++ openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/cmp/jpa/JpaTest.java Wed Jan 16 18:12:06 2008
@@ -159,9 +159,13 @@
 
     private EntityManagerFactory createEntityManagerFactory() throws Exception {
         PersistenceClassLoaderHandler persistenceClassLoaderHandler = new PersistenceClassLoaderHandler() {
-            public void addTransformer(ClassLoader classLoader, ClassFileTransformer classFileTransformer) {
+
+            public void addTransformer(String unitId, ClassLoader classLoader, ClassFileTransformer classFileTransformer) {
                 Instrumentation instrumentation = Agent.getInstrumentation();
                 instrumentation.addTransformer(classFileTransformer);
+            }
+
+            public void destroy(String unitId) {
             }
 
             public ClassLoader getNewTempClassLoader(ClassLoader classLoader) {

Modified: openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/cmp/jpa/UnenhancedTest.java
URL: http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/cmp/jpa/UnenhancedTest.java?rev=612681&r1=612680&r2=612681&view=diff
==============================================================================
--- openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/cmp/jpa/UnenhancedTest.java (original)
+++ openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/cmp/jpa/UnenhancedTest.java Wed Jan 16 18:12:06 2008
@@ -190,11 +190,15 @@
         ClassLoader loader = new FilteredChildFirstClassLoader(getClass().getClassLoader(), "org.apache.openejb.core.cmp.jpa");
 
         PersistenceClassLoaderHandler persistenceClassLoaderHandler = new PersistenceClassLoaderHandler() {
-            public void addTransformer(ClassLoader classLoader, ClassFileTransformer classFileTransformer) {
+
+            public void addTransformer(String unitId, ClassLoader classLoader, ClassFileTransformer classFileTransformer) {
                 Instrumentation instrumentation = Agent.getInstrumentation();
                 if (instrumentation != null) {
                     instrumentation.addTransformer(new ControllableTransformer(classFileTransformer));
                 }
+            }
+
+            public void destroy(String unitId) {
             }
 
             public ClassLoader getNewTempClassLoader(ClassLoader classLoader) {