bugfix: removed class loader leaks

payara · May 2, 2024 · 74f81fb · 74f81fb
1 parent 63ebd25
commit 74f81fb
Show file tree

Hide file tree

Showing 18 changed files with 98 additions and 36 deletions.
diff --git a/.gitignore b/.gitignore
@@ -13,6 +13,7 @@ target/
 .vscode/
 .idea
 .DS_Store
+.sdkmanrc
 gfbuild.log
 nb-configuration.xml
 /appserver/tests/quicklook/classes/
@@ -26,4 +27,4 @@ appserver/tests/quicklook/quicklook_summary.txt
 **/nbproject
 .flattened-pom.xml
 **/__pycache__
-appserver/packager/legal/src/main/resources/glassfish/legal/3RD-PARTY-LICENSE.txt
+appserver/packager/legal/src/main/resources/glassfish/legal/3RD-PARTY-LICENSE.txt
diff --git a/appserver/ejb/ejb-container/src/main/java/com/sun/ejb/EjbInvocation.java b/appserver/ejb/ejb-container/src/main/java/com/sun/ejb/EjbInvocation.java
@@ -269,7 +269,7 @@ public class EjbInvocation //
 
     EjbInvocation(String compEnvId, Container container) {
         super.componentId = compEnvId;
-        super.container = container;
+        setContainer(container);
         super.setComponentInvocationType(ComponentInvocation.ComponentInvocationType.EJB_INVOCATION);
 
         EjbBundleDescriptor ejbBundleDesc = container.getEjbDescriptor().getEjbBundleDescriptor();
@@ -371,6 +371,7 @@ public EjbInvocation clone() {
     @Override
     public Object getJaccEjb() {
         Object bean = null;
+        Object container = getContainer();
         if( container != null ) {
             bean = ((Container) container).getJaccEjb(this);
         }
@@ -452,7 +453,7 @@ public void setTransactionOperationsManager(TransactionOperationsManager transac
      */
     @Override
     public boolean userTransactionMethodsAllowed() {
-        return ((Container) container).userTransactionMethodsAllowed(this);
+        return ((Container) getContainer()).userTransactionMethodsAllowed(this);
     }
 
     /**
@@ -461,15 +462,15 @@ public boolean userTransactionMethodsAllowed() {
      */
     @Override
     public void userTransactionLookupAllowed() throws NameNotFoundException {
-        ((BaseContainer) container).checkUserTransactionLookup(this);
+        ((BaseContainer) getContainer()).checkUserTransactionLookup(this);
     }
 
     /**
      * Called by the UserTransaction when transaction is started.
      */
     @Override
     public void doAfterUtxBegin() {
-        ((Container) container).doAfterBegin(this);
+        ((Container) getContainer()).doAfterBegin(this);
     }
 
     public InterceptorManager.InterceptorChain getInterceptorChain() {
@@ -650,11 +651,11 @@ public Object[] getInterceptorInstances() {
 
     @Override
     public  Object invokeBeanMethod() throws Throwable {
-        return ((BaseContainer) container).invokeBeanMethod(this);
+        return ((BaseContainer) getContainer()).invokeBeanMethod(this);
     }
 
     public com.sun.enterprise.security.SecurityManager getEjbSecurityManager() {
-        return ((BaseContainer)container).getSecurityManager();
+        return ((BaseContainer) getContainer()).getSecurityManager();
     }
 
     @Override
@@ -684,7 +685,7 @@ public boolean authorizeWebService(Method m) throws Exception {
     private Exception authorizeWebServiceAndSetMethod(Method m) {
         try {
             this.method = m;
-            if (((com.sun.ejb.Container) container).authorize(this)) {
+            if (((com.sun.ejb.Container) getContainer()).authorize(this)) {
                 // Record the method on which the successful
                 // authorization check was performed.
                 setWebServiceMethod(m);

diff --git a/appserver/ejb/ejb-container/src/main/java/com/sun/ejb/EjbInvocationFactory.java b/appserver/ejb/ejb-container/src/main/java/com/sun/ejb/EjbInvocationFactory.java
@@ -58,7 +58,7 @@ public EjbInvocationFactory(String compEnvId, Container container) {
 
     public EjbInvocation create() {
         EjbInvocation ejbInv = new EjbInvocation(compEnvId, container);
-        ejbInv.jndiEnvironment = container.getEjbDescriptor();
+        ejbInv.setJNDIEnvironment(container.getEjbDescriptor());
         return ejbInv;
     }
 
@@ -67,7 +67,7 @@ public <C extends ComponentContext> EjbInvocation create(Object ejb, C ctx) {
         ejbInv.ejb = ejb;
         ejbInv.instance = ejb;
         ejbInv.context = ctx;
-        ejbInv.jndiEnvironment = container.getEjbDescriptor();
+        ejbInv.setJNDIEnvironment(container.getEjbDescriptor());
 
         return ejbInv;
     }

diff --git a/appserver/ejb/ejb-container/src/main/java/com/sun/ejb/containers/BaseContainer.java b/appserver/ejb/ejb-container/src/main/java/com/sun/ejb/containers/BaseContainer.java
@@ -2002,7 +2002,7 @@ public void preInvoke(EjbInvocation inv) {
             }
 
             inv.transactionAttribute = inv.invocationInfo.txAttr;
-            inv.container = this;
+            inv.setContainer(this);
 
             if (inv.mustInvokeAsynchronously()) {
                 return;

diff --git a/...ver/ejb/ejb-container/src/main/java/com/sun/ejb/containers/EjbAsyncInvocationManager.java b/...ver/ejb/ejb-container/src/main/java/com/sun/ejb/containers/EjbAsyncInvocationManager.java
@@ -158,7 +158,7 @@ public void cleanupContainerTasks(Container container) {
             Map.Entry<Long, EjbFutureTask> next = iterator.next();
 
             EjbAsyncTask task = next.getValue().getEjbAsyncTask();
-            if( task.getEjbInvocation().container == container ) {
+            if( task.getEjbInvocation().getContainer() == container ) {
 
                 removedTasks.add(task.getInvId());
 

diff --git a/appserver/ejb/ejb-container/src/main/java/com/sun/ejb/containers/EjbAsyncTask.java b/appserver/ejb/ejb-container/src/main/java/com/sun/ejb/containers/EjbAsyncTask.java
@@ -88,7 +88,7 @@ EjbInvocation getEjbInvocation() {
     public V call()
             throws Exception {
         V returnValue = null;
-        BaseContainer container = (BaseContainer) inv.container;
+        BaseContainer container = (BaseContainer) inv.getContainer();
         ClassLoader prevCL = Thread.currentThread().getContextClassLoader();
         try {
             Utility.setContextClassLoader(container.getClassLoader());

diff --git a/appserver/ejb/ejb-container/src/main/java/com/sun/ejb/containers/EjbEndpointFacadeImpl.java b/appserver/ejb/ejb-container/src/main/java/com/sun/ejb/containers/EjbEndpointFacadeImpl.java
@@ -89,7 +89,7 @@ public ComponentInvocation startInvocation() {
 
         // Do the portions of preInvoke that don't need a Method object.
         inv.isWebService = true;
-        inv.container = container_;
+        inv.setContainer(container_);
         inv.transactionAttribute = Container.TX_NOT_INITIALIZED;
 
         // AS per latest spec change, the MessageContext object in WebSvcCtxt

diff --git a/...rver/ejb/ejb-full-container/src/main/java/org/glassfish/ejb/mdb/MessageBeanContainer.java b/...rver/ejb/ejb-full-container/src/main/java/org/glassfish/ejb/mdb/MessageBeanContainer.java
@@ -216,7 +216,7 @@ public final class MessageBeanContainer extends BaseContainer implements Message
             messageBeanClient_ = clientFactory.createMessageBeanClient(msgBeanDesc);
 
             componentInvocation = createComponentInvocation();
-            componentInvocation.container = this;
+            componentInvocation.setContainer(this);
             invocationManager.preInvoke(componentInvocation);
             messageBeanClient_.setup(this);
 
@@ -1135,7 +1135,7 @@ public void beforeMessageDelivery(Method method, MessageDeliveryType deliveryTyp
             invocation.context = context;
             invocation.instance = context.getEJB();
             invocation.ejb = context.getEJB();
-            invocation.container = this;
+            invocation.setContainer(this);
 
             // Message Bean Container only starts a new transaction if
             // there is no imported transaction and the message listener

diff --git a/...sh/payara/microprofile/telemetry/tracing/jaxrs/OpenTelemetryApplicationEventListener.java b/...sh/payara/microprofile/telemetry/tracing/jaxrs/OpenTelemetryApplicationEventListener.java
@@ -79,6 +79,12 @@ public void postConstruct() {
 
     @Override
     public void onEvent(final ApplicationEvent event) {
+        switch (event.getType()) {
+            case DESTROY_FINISHED:
+            case RELOAD_FINISHED:
+                openTracingHelper.canTraceCache.clear(event.getResourceConfig().getClassLoader());
+                break;
+        }
         LOG.config(() -> "onEvent(event.type=" + event.getType() + ")");
     }
 

diff --git a/...try/src/main/java/fish/payara/microprofile/telemetry/tracing/jaxrs/OpenTracingHelper.java b/...try/src/main/java/fish/payara/microprofile/telemetry/tracing/jaxrs/OpenTracingHelper.java
@@ -368,7 +368,7 @@ private Traced computeTracedAnnotation(ResourceInfo resourceInfo, BeanManager be
         return result == null ? NULL_TRACED : result;
     }
 
-    private static ResourceCache<Boolean> canTraceCache = new ResourceCache<>();
+    static ResourceCache<Boolean> canTraceCache = new ResourceCache<>();
     /**
      * Helper method that checks if any specified skip patterns match this method name
      *

diff --git a/...lemetry/src/main/java/fish/payara/microprofile/telemetry/tracing/jaxrs/ResourceCache.java b/...lemetry/src/main/java/fish/payara/microprofile/telemetry/tracing/jaxrs/ResourceCache.java
@@ -47,7 +47,6 @@
 import java.util.Objects;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Supplier;
-import org.eclipse.microprofile.opentracing.Traced;
 
 class ResourceCache<T> {
 
@@ -83,4 +82,12 @@ public int hashCode() {
     T get(ResourceInfo info, Supplier<T> supplier) {
         return tracedCache.computeIfAbsent(new ResourceKey(info), k -> supplier.get());
     }
+
+    /**
+     * clear all classes belonging to this class loader
+     * @param classLoader
+     */
+    void clear(ClassLoader classLoader) {
+        tracedCache.entrySet().removeIf(entry -> entry.getKey().resourceClass.getClassLoader() == classLoader);
+    }
 }
diff --git a/...ner/src/main/java/org/glassfish/persistence/ejb/entitybean/container/EntityContainer.java b/...ner/src/main/java/org/glassfish/persistence/ejb/entitybean/container/EntityContainer.java
@@ -1284,7 +1284,7 @@ private void internalRemoveBeanUnchecked(EJBLocalRemoteObject localRemoteObj, bo
             context.incrementCalls();
 
             inv.instance = inv.ejb = context.getEJB();
-            inv.container = this;
+            inv.setContainer(this);
             invocationManager.preInvoke(inv);
 
             // call ejbLoad if necessary

diff --git a/...b/gf-web-connector/src/main/java/fish/payara/appserver/context/JavaEEContextUtilImpl.java b/...b/gf-web-connector/src/main/java/fish/payara/appserver/context/JavaEEContextUtilImpl.java
@@ -214,7 +214,7 @@ private InstanceImpl(ComponentInvocation currentInvocation) {
             } else if (currentInvocation.getJNDIEnvironment() instanceof JndiNameEnvironment) {
                 componentId = DOLUtils.toEarComponentId(
                         DOLUtils.getApplicationName((JndiNameEnvironment)
-                                currentInvocation.jndiEnvironment));
+                                currentInvocation.getJNDIEnvironment()));
                 isApplicationComponent = true;
             } else {
                 // checkState() later should error out due to this condition

diff --git a/appserver/web/war-util/src/main/java/org/glassfish/web/loader/WebappClassLoader.java b/appserver/web/war-util/src/main/java/org/glassfish/web/loader/WebappClassLoader.java
@@ -121,6 +121,7 @@
 import java.text.MessageFormat;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
@@ -2036,6 +2037,7 @@ public void stop() throws Exception {
         // START SJSAS 6258619
         ClassLoaderUtil.releaseLoader(this);
         // END SJSAS 6258619
+        clearBeanELResolverCache();
 
         synchronized(jarFilesLock) {
             started = false;
@@ -2642,6 +2644,45 @@ private void clearReferencesRmiTargets() {
         }
     }
 
+    private void clearBeanELResolverCache() {
+        try {
+            Class<?> elUtilsClass = Class.forName("com.sun.faces.el.ELUtils");
+            Class<?> elResolverClass = Class.forName("jakarta.el.BeanELResolver");
+            Object resolver = elUtilsClass.getField("BEAN_RESOLVER").get(null);
+            logger.fine(String.format("Fields: %s", Arrays.stream(elResolverClass.getDeclaredFields())
+                    .map(Field::toString).collect(Collectors.toList())));
+            try {
+                elResolverClass.getMethod("clearProperties", ClassLoader.class).invoke(resolver, this);
+            } catch (NoSuchMethodException e) {
+                clearBeanELResolverPropertiesCache(resolver, elResolverClass);
+            }
+        } catch (ClassNotFoundException e) {
+            logger.log(Level.FINE, "BeanELResolver or ELUtils not found", e);
+        } catch (Exception e) {
+            logger.log(Level.WARNING, "Error clearing BeanELResolver cache", e);
+        }
+    }
+
+    /**
+     * Workaround until clearProperties() is available in Jakarta EL
+     * @see <a href="https://github.com/jakartaee/expression-language/pull/215">Jakarta EL Pull Request</a>
+     */
+    private void clearBeanELResolverPropertiesCache(Object resolver, Class<?> elResolverClass) throws Exception {
+        Class<?> elResolverCacheClass = Class.forName("jakarta.el.BeanELResolver$SoftConcurrentHashMap");
+        var propertiesField = elResolverClass.getDeclaredField("properties");
+        propertiesField.setAccessible(true);
+        ConcurrentHashMap<Class<?>, Object> properties =
+                (ConcurrentHashMap<Class<?>, Object>) propertiesField.get(resolver);
+        properties.entrySet().removeIf(entry -> entry.getKey().getClassLoader() == this);
+        var mapField = elResolverCacheClass.getDeclaredField("map");
+        mapField.setAccessible(true);
+        ConcurrentHashMap<Class<?>, Object> map =
+                (ConcurrentHashMap<Class<?>, Object>) mapField.get(propertiesField.get(resolver));
+        map.entrySet().removeIf(entry -> entry.getKey().getClassLoader() == this);
+        var cleanupMethod = elResolverCacheClass.getDeclaredMethod("cleanup");
+        cleanupMethod.setAccessible(true);
+        cleanupMethod.invoke(propertiesField.get(resolver));
+    }
 
     /**
      * Clear the {@link ResourceBundle} cache of any bundles loaded by this

diff --git a/appserver/web/web-core/src/main/java/org/apache/catalina/core/ApplicationFilterConfig.java b/appserver/web/web-core/src/main/java/org/apache/catalina/core/ApplicationFilterConfig.java
@@ -324,8 +324,9 @@ void release() {
                     String msg = rb.getString(LogFacade.DO_AS_PRIVILEGE);
                     log.log(Level.SEVERE, msg, ex);
                 }
-            } else { 
+            } else {
                 filter.destroy();
+                filterDef = null;
             }
 
             if (context != null) {

diff --git a/appserver/web/web-glue/src/main/java/com/sun/enterprise/web/WebComponentInvocation.java b/appserver/web/web-glue/src/main/java/com/sun/enterprise/web/WebComponentInvocation.java
@@ -64,8 +64,8 @@ public WebComponentInvocation(WebModule wm, Object instance) {
         setComponentInvocationType(
                 ComponentInvocation.ComponentInvocationType.SERVLET_INVOCATION);
         componentId = wm.getComponentId();
-        jndiEnvironment = wm.getWebBundleDescriptor();
-        container = wm;
+        setJNDIEnvironment(wm.getWebBundleDescriptor());
+        setContainer(wm);
         this.instance = instance;
         setResourceTableKey(_getResourceTableKey());
 

diff --git a/core/core-parent/pom.xml b/core/core-parent/pom.xml
@@ -642,6 +642,7 @@
                                     <exclude>io.opentelemetry.extension</exclude>
                                     <exclude>io.opentelemetry.instrumentation</exclude>
                                     <exclude>fish.payara.shaded</exclude>
+                                    <exclude>org.glassfish.api.invocation</exclude>
                                 </excludes>
                             </parameter>
                         </configuration>