Skip to content

Commit

Permalink
[FISH-1018] Out of memory redeploy leaks (#5081) (#282)
Browse files Browse the repository at this point in the history 
* [FISH-1018] Out of memory redeploy leaks (#5081)

* #4098 Reduced too broad scope of variable

* #4098 Moved field to local variable

- this could be the first cause of leak

* #4098 Refactored inner and anonymous classes to nested static classes

- anonymous and inner holds implicit reference to parent
- this was probably the second cause of leak

* #4098 Fixed code consistency

* Fix more class loader leaks by:
- making sure Server Threads and Timers do not inherit app's context class loaders
- making sure app's security contexts don't get propagated to server threads and timers

Added correct ear classes to class loader leak tests

Co-authored-by: lprimak <[email protected]>

* [FISH-1018] found more leaks and more reliable leak test (#5102)

* found more leaks and more reliable leak test

* bump jakarta.el to -p3 patch

* tyrus patched update

Co-authored-by: Lukáš Kvídera <[email protected]>
  • Loading branch information
@lprimak @sgflt
lprimak and sgflt authored Feb 2, 2021
1 parent fa786d5 commit 6115d9f
Show file tree
Hide file tree
Showing 17 changed files with 502 additions and 339 deletions.
46 changes: 34 additions & 12 deletions ...ent/concurrent-impl/src/main/java/org/glassfish/concurrent/runtime/ConcurrentRuntime.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,14 @@
* only if the new code is made subject to such option by the copyright
* holder.
*/
// Portions Copyright [2016-2018] [Payara Foundation and/or its affiliates]
// Portions Copyright [2016-2021] [Payara Foundation and/or its affiliates]

package org.glassfish.concurrent.runtime;

import com.sun.enterprise.config.serverbeans.Applications;
import com.sun.enterprise.container.common.spi.util.ComponentEnvManager;
import com.sun.enterprise.transaction.api.JavaEETransactionManager;
import com.sun.enterprise.util.Utility;
import org.glassfish.api.invocation.InvocationManager;
import org.glassfish.concurrent.LogFacade;
import org.glassfish.concurrent.runtime.deployer.ContextServiceConfig;
Expand All @@ -65,6 +66,7 @@
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.glassfish.enterprise.concurrent.spi.ContextHandle;

/**
* This class provides API to create various Concurrency Utilities objects
Expand Down Expand Up @@ -180,16 +182,16 @@ public void shutdownContextService(String jndiName) {

public synchronized ManagedExecutorServiceImpl getManagedExecutorService(ResourceInfo resource, ManagedExecutorServiceConfig config) {
String jndiName = config.getJndiName();

if (managedExecutorServiceMap != null && managedExecutorServiceMap.containsKey(jndiName)) {
return managedExecutorServiceMap.get(jndiName);
}
ManagedThreadFactoryImpl managedThreadFactory = new ManagedThreadFactoryImpl(

ManagedThreadFactoryImpl managedThreadFactory = new ThreadFactoryWrapper(
config.getJndiName() + "-managedThreadFactory",
null,
config.getThreadPriority());

ManagedExecutorServiceImpl mes = new ManagedExecutorServiceImpl(config.getJndiName(),
managedThreadFactory,
config.getHungAfterSeconds() * 1000L, // in millseconds
Expand All @@ -202,17 +204,17 @@ public synchronized ManagedExecutorServiceImpl getManagedExecutorService(Resourc
createContextService(config.getJndiName() + "-contextservice",
config.getContextInfo(), config.getContextInfoEnabled(), true),
AbstractManagedExecutorService.RejectPolicy.ABORT);

if (managedExecutorServiceMap == null) {
managedExecutorServiceMap = new HashMap();
}

managedExecutorServiceMap.put(jndiName, mes);

if (config.getHungAfterSeconds() > 0L && !config.isLongRunningTasks()) {
scheduleInternalTimer();
}

return mes;
}

Expand All @@ -234,7 +236,7 @@ public synchronized ManagedScheduledExecutorServiceImpl getManagedScheduledExecu
if (managedScheduledExecutorServiceMap != null && managedScheduledExecutorServiceMap.containsKey(jndiName)) {
return managedScheduledExecutorServiceMap.get(jndiName);
}
ManagedThreadFactoryImpl managedThreadFactory = new ManagedThreadFactoryImpl(
ManagedThreadFactoryImpl managedThreadFactory = new ThreadFactoryWrapper(
config.getJndiName() + "-managedThreadFactory",
null,
config.getThreadPriority());
Expand Down Expand Up @@ -275,7 +277,7 @@ public synchronized ManagedThreadFactoryImpl getManagedThreadFactory(ResourceInf
if (managedThreadFactoryMap != null && managedThreadFactoryMap.containsKey(jndiName)) {
return managedThreadFactoryMap.get(jndiName);
}
ManagedThreadFactoryImpl managedThreadFactory = new ManagedThreadFactoryImpl(config.getJndiName(),
ManagedThreadFactoryImpl managedThreadFactory = new ThreadFactoryWrapper(config.getJndiName(),
createContextService(config.getJndiName() + "-contextservice",
config.getContextInfo(), config.getContextInfoEnabled(), true),
config.getThreadPriority());
Expand Down Expand Up @@ -360,7 +362,7 @@ private ContextSetupProviderImpl.CONTEXT_TYPE[] parseContextInfo(String contextI
private void scheduleInternalTimer() {
if (internalScheduler == null) {
String name = "glassfish-internal";
ManagedThreadFactoryImpl managedThreadFactory = new ManagedThreadFactoryImpl(
ManagedThreadFactoryImpl managedThreadFactory = new ThreadFactoryWrapper(
name + "-managedThreadFactory",
null,
Thread.NORM_PRIORITY);
Expand All @@ -378,6 +380,26 @@ private void scheduleInternalTimer() {
}
}

/**
* context loader propagation to threads causes memory leaks on redeploy
*/
private static final class ThreadFactoryWrapper extends ManagedThreadFactoryImpl {
public ThreadFactoryWrapper(String string, ContextServiceImpl contextService, int threadPriority) {
super(string, contextService, threadPriority);
}

@Override
protected AbstractManagedThread createThread(Runnable r, ContextHandle contextHandleForSetup) {
ClassLoader appClassLoader = Utility.getClassLoader();
Utility.setContextClassLoader(null);
try {
return super.createThread(r, contextHandleForSetup);
} finally {
Utility.setContextClassLoader(appClassLoader);
}
}
}

@Override
public void postConstruct() {
}
Expand Down
20 changes: 15 additions & 5 deletions ...tors/connectors-runtime/src/main/java/com/sun/enterprise/connectors/ConnectorRuntime.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
* only if the new code is made subject to such option by the copyright
* holder.
*/
// Portions Copyright [2019] [Payara Foundation and/or its affiliates]
// Portions Copyright [2019-2021] [Payara Foundation and/or its affiliates]

package com.sun.enterprise.connectors;

Expand All @@ -63,6 +63,7 @@
import com.sun.enterprise.security.SecurityServicesUtil;
import com.sun.enterprise.security.jaspic.callback.ContainerCallbackHandler;
import com.sun.enterprise.transaction.api.JavaEETransactionManager;
import com.sun.enterprise.util.Utility;
import com.sun.logging.LogDomains;
import org.glassfish.admin.monitor.MonitoringBootstrap;
import org.glassfish.api.admin.ProcessEnvironment;
Expand Down Expand Up @@ -106,6 +107,8 @@
import javax.transaction.Transaction;
import java.io.PrintWriter;
import java.net.URI;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.sql.Connection;
import java.sql.SQLException;
import java.util.*;
Expand Down Expand Up @@ -886,7 +889,7 @@ public void postConstruct() {
}
if(isServer() || isEmbedded()){
poolMonitoringLevelListener = poolMonitoringLevelListenerProvider.get();

// Force initialization of the ResourceManager
getResourceManager();
}
Expand Down Expand Up @@ -968,7 +971,7 @@ public void createConnectorConnectionPool(ConnectorConnectionPool ccp,
throws ConnectorRuntimeException {
ccPoolAdmService.createConnectorConnectionPool(ccp, connectionDefinitionName, rarName, props, securityMaps);
}

private synchronized org.glassfish.resourcebase.resources.listener.ResourceManager getResourceManager() {
if (resourceManager == null) {
try {
Expand All @@ -978,7 +981,7 @@ private synchronized org.glassfish.resourcebase.resources.listener.ResourceManag
return null;
}
}

return resourceManager;
}

Expand Down Expand Up @@ -1027,7 +1030,14 @@ public InvocationManager getInvocationManager() {
}

public Timer getTimer() {
return ConnectorTimerProxy.getProxy();
ClassLoader appClassLoader = Utility.getClassLoader();
// prevent app class loader leaking into timer
Utility.setContextClassLoader(null);
try {
return AccessController.doPrivileged((PrivilegedAction<Timer>) ConnectorTimerProxy::getProxy);
} finally {
Utility.setContextClassLoader(appClassLoader);
}
}

/**
Expand Down
42 changes: 23 additions & 19 deletions ...nectors-runtime/src/main/java/com/sun/enterprise/connectors/util/ConnectorTimerProxy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,28 @@
* only if the new code is made subject to such option by the copyright
* holder.
*/
// Portions Copyright [2019-2021] [Payara Foundation and/or its affiliates]

package com.sun.enterprise.connectors.util;

import com.sun.enterprise.connectors.ConnectorRuntime;
import com.sun.logging.LogDomains;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Date;
import java.util.Timer;
import java.util.TimerTask;
import java.util.logging.Level;
import java.util.logging.Logger;

public class ConnectorTimerProxy extends Timer {

private volatile static ConnectorTimerProxy connectorTimer;
private Timer timer;
private boolean timerException = false;
private final Object getTimerLock = new Object();
private final static Logger _logger = LogDomains.getLogger(ConnectorTimerProxy.class,

private final static Logger _logger = LogDomains.getLogger(ConnectorTimerProxy.class,
LogDomains.RSR_LOGGER);

private ConnectorTimerProxy(boolean isDaemon) {
Expand All @@ -70,14 +73,15 @@ private Timer getTimer() {
loader = Thread.currentThread().getContextClassLoader();
Thread.currentThread().setContextClassLoader(
ConnectorRuntime.getRuntime().getConnectorClassLoader());
timer = new Timer("connector-timer-proxy", true);
timer = AccessController.doPrivileged((PrivilegedAction<Timer>) () ->
new Timer("connector-timer-proxy", true));
} finally {
Thread.currentThread().setContextClassLoader(loader);
timerException = false;
}
}
}
return timer;
return timer;
}

public static final ConnectorTimerProxy getProxy() {
Expand All @@ -90,7 +94,7 @@ public static final ConnectorTimerProxy getProxy() {
}
return connectorTimer;
}

/**
* Proxy method to schedule a timer task at fixed rate.
* The unchecked exceptions are caught here and in such cases, the timer
Expand Down Expand Up @@ -128,7 +132,7 @@ public int purge() {
status = timer.purge();
} catch(Exception ex) {
_logger.log(Level.WARNING, "exception_purging_timer", ex.getMessage());
}
}
return status;
}

Expand Down Expand Up @@ -158,7 +162,7 @@ public void schedule(TimerTask task, long delay) {
* @param task
* @param delay
* @param period
*/
*/
@Override
public void schedule(TimerTask task, Date time) {
timer = getTimer();
Expand All @@ -167,11 +171,11 @@ public void schedule(TimerTask task, Date time) {
} catch(Exception ex) {
handleTimerException(ex);
timer.schedule(task, time);
}
}
}

/**
* Proxy method to schedule a timer task for repeated fixed-delay execution,
* Proxy method to schedule a timer task for repeated fixed-delay execution,
* beginning after the specified delay.
* The unchecked exceptions are caught here and in such cases, the timer
* is recreated and task is rescheduled.
Expand All @@ -187,11 +191,11 @@ public void schedule(TimerTask task, long delay, long period) {
} catch(Exception ex) {
handleTimerException(ex);
timer.schedule(task, delay, period);
}
}
}

/**
* Proxy method to schedule a timer task for repeated fixed-delay execution,
* Proxy method to schedule a timer task for repeated fixed-delay execution,
* beginning after the specified delay.
* The unchecked exceptions are caught here and in such cases, the timer
* is recreated and task is rescheduled.
Expand All @@ -207,11 +211,11 @@ public void schedule(TimerTask task, Date firstTime, long period) {
} catch(Exception ex) {
handleTimerException(ex);
timer.schedule(task, firstTime, period);
}
}
}

/**
* Proxy method to schedule a timer task for repeated fixed-rate execution,
* Proxy method to schedule a timer task for repeated fixed-rate execution,
* beginning after the specified delay.
* The unchecked exceptions are caught here and in such cases, the timer
* is recreated and task is rescheduled.
Expand All @@ -227,19 +231,19 @@ public void scheduleAtFixedRate(TimerTask task, Date firstTime, long period) {
} catch(Exception ex) {
handleTimerException(ex);
timer.scheduleAtFixedRate(task, firstTime, period);
}
}
}

/**
* Handle any exception occured during scheduling timer.
*
* In case of unchecked exceptions, the timer is recreated to be used
* Handle any exception occured during scheduling timer.
*
* In case of unchecked exceptions, the timer is recreated to be used
* by the subsequent requests for scheduling.
* @param ex exception that was caught
*/
private void handleTimerException(Exception ex) {
_logger.log(Level.WARNING, "exception_scheduling_timer", ex.getMessage());

//In case of unchecked exceptions, timer needs to recreated.
_logger.info("Recreating Timer and scheduling at fixed rate");
timerException = true;
Expand Down
12 changes: 7 additions & 5 deletions ...oyment/javaee-full/src/main/java/org/glassfish/javaee/full/deployment/EarClassLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
* only if the new code is made subject to such option by the copyright
* holder.
*/
// Portions Copyright [2016] [Payara Foundation and/or its affiliates]
// Portions Copyright [2016-2021] [Payara Foundation and/or its affiliates]

package org.glassfish.javaee.full.deployment;

Expand All @@ -47,6 +47,7 @@

import com.sun.enterprise.loader.ASURLClassLoader;
import java.util.logging.Logger;
import org.glassfish.common.util.InstanceCounter;
import org.glassfish.internal.api.DelegatingClassLoader;
import org.glassfish.hk2.api.PreDestroy;

Expand All @@ -62,6 +63,7 @@ public class EarClassLoader extends ASURLClassLoader
boolean isPreDestroyCalled = false;
private static final Logger log = Logger.getLogger(EarClassLoader.class.getName());
private final Application application;
private final InstanceCounter instanceCounter = new InstanceCounter(this);

public EarClassLoader(ClassLoader classLoader, Application application) {
super(classLoader);
Expand Down Expand Up @@ -90,13 +92,13 @@ public void preDestroy() {
try {
for (ClassLoaderHolder clh : moduleClassLoaders) {
// destroy all the module classloaders
if ( !(clh.loader instanceof EarLibClassLoader) &&
!(clh.loader instanceof EarClassLoader) &&
if ( !(clh.loader instanceof EarLibClassLoader) &&
!(clh.loader instanceof EarClassLoader) &&
!isRARCL(clh.loader)) {
try {
PreDestroy.class.cast(clh.loader).preDestroy();
} catch (Exception e) {
// ignore, the class loader does not need to be
// ignore, the class loader does not need to be
// explicitly stopped.
}
}
Expand All @@ -111,7 +113,7 @@ public void preDestroy() {
try {
PreDestroy.class.cast(cf).preDestroy();
} catch (Exception e) {
// ignore, the class loader does not need to be
// ignore, the class loader does not need to be
// explicitly stopped.
}
}
Expand Down
Loading

0 comments on commit 6115d9f

Please sign in to comment.