Skip to content

Fixing com.sun.enterprise.loader.ASURLClassLoader memory leak #7919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
svendiedrichsen wants to merge 2 commits into
base: main
Choose a base branch
from
+83 −97
Open

Fixing com.sun.enterprise.loader.ASURLClassLoader memory leak #7919

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
42e5aa6
using internal static classes to wrap SentinelInputStream and URLEntr…
Jan 29, 2026
b513597
adding usage of cleanable to allow to unregister the runnable in case…
Jan 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 83 additions & 97 deletions nucleus/common/common-util/src/main/java/com/sun/enterprise/loader/ASURLClassLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,15 @@
import java.lang.instrument.ClassFileTransformer;
import java.lang.instrument.IllegalClassFormatException;
import org.glassfish.hk2.utilities.CleanerFactory;

import java.lang.ref.Cleaner;
import java.lang.ref.WeakReference;
import java.net.*;
import java.nio.file.Path;
import java.security.*;
import java.security.cert.Certificate;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Predicate;
import java.util.jar.Attributes;
import java.util.jar.JarEntry;
Expand All @@ -70,11 +71,9 @@
import java.util.logging.Logger;
import java.util.stream.Collectors;
import java.util.zip.ZipEntry;
import org.glassfish.common.util.InstanceCounter;

/**
* Class loader used by the ejbs of an application or stand alone module.
*
* Class loader used by the EJBs of an application or standalone module.
* This class loader also keeps cache of not found classes and resources.
* </xmp>
*
Expand Down Expand Up @@ -126,16 +125,12 @@ list of url entries of this class loader. Using LinkedHashSet instead of origina

private boolean mustLoadInThisStackFrame;

/** streams opened by this loader */
private final List<SentinelInputStream> streams = new CopyOnWriteArrayList<>();

private final ArrayList<ClassFileTransformer> transformers = new ArrayList<>(1);

private static final StringManager sm = StringManager.getManager(ASURLClassLoader.class);

//holder for declared and ee permissions
private final PermsHolder permissionsHolder;
private final InstanceCounter instanceCounter = new InstanceCounter(this);

/**
* Constructor.
Expand Down Expand Up @@ -215,8 +210,6 @@ public void done() {
u.close();
}

closeOpenStreams();

// clears out the tables
// Clear all values. Because fields are 'final' (for thread safety), cannot null them
this.urlSet.clear();
Expand Down Expand Up @@ -906,7 +899,6 @@ private static final class ProtectedJarFile extends JarFile {
*/
public ProtectedJarFile(File file) throws IOException {
super(file, true, OPEN_READ, RUNTIME_VERSION);
registerCloseEvent();
}

/**
Expand Down Expand Up @@ -942,15 +934,6 @@ public void reallyClose() throws IOException {
super.close();
}

public final void registerCloseEvent() {
CleanerFactory.create().register(this, () -> {
try {
reallyClose();
} catch (IOException ex) {
_logger.log(Level.WARNING, null, ex);
}
});
}
}

/**
Expand Down Expand Up @@ -978,6 +961,33 @@ protected static final class URLEntry {
ensure thread visibility by making it 'volatile' */
volatile ProtectionDomain pd = null;

private Cleaner.Cleanable cleanable;

static class CleanableURLEntryState implements Runnable {

private final ProtectedJarFile zip;
private final URL source;
private final Logger logger;

CleanableURLEntryState(ProtectedJarFile zip, URL source, Logger logger) {
this.zip = zip;
this.source = source;
this.logger = logger;
}

@Override
public void run() {
try {
zip.reallyClose();
} catch (IOException ioe) {
logger.log(Level.INFO,
CULoggerInfo.getString(CULoggerInfo.exceptionClosingURLEntry, source),
ioe);
}
}

}

URLEntry(URL url) throws IOException {
source = url;
init();
Expand All @@ -990,6 +1000,7 @@ void init() throws IOException {

if (isJar) {
zip = new ProtectedJarFile(file);
this.cleanable = CleanerFactory.create().register(this, new CleanableURLEntryState(zip, source, _logger));
// negative lookups in jar are cheap
presenceCheck = (any) -> true;
} else {
Expand Down Expand Up @@ -1034,8 +1045,7 @@ public boolean equals(Object obj) {

boolean tf = false;

if (obj instanceof URLEntry) {
URLEntry e = (URLEntry) obj;
if (obj instanceof URLEntry e) {
try {
//try comparing URIs
if (source.toURI().equals(e.source.toURI())) {
Expand Down Expand Up @@ -1068,36 +1078,15 @@ public int hashCode() {
}

public void close() {
if (zip != null) {
try {
zip.reallyClose();
} catch (IOException ioe) {
_logger.log(Level.INFO,
CULoggerInfo.getString(CULoggerInfo.exceptionClosingURLEntry, source),
ioe);
}
if (this.cleanable != null) {
this.cleanable.clean();
}
if (!isJar) {
DirWatcher.unregister(file.toPath());
}
}
}

/**
*Closes any streams that remain open, logging a warning for each.
*<p>
*This method should be invoked when the loader will no longer be used
*and the app will no longer explicitly close any streams it may have opened.
* Must be synchnronized to (a) avoid race condition checking 'streams'.
*/
private synchronized void closeOpenStreams() {
SentinelInputStream[] toClose = streams.toArray(new SentinelInputStream[0]);
for (SentinelInputStream s : toClose) {
s.closeWithWarning();
}
streams.clear();
}

/**
* Wraps all InputStreams returned by this class loader to report when
* a finalizer is run before the stream has been closed. This helps
Expand All @@ -1106,10 +1095,51 @@ private synchronized void closeOpenStreams() {
* @author tjquinn
*/
protected static final class SentinelInputStream extends FilterInputStream {
private volatile boolean closed = false;
private volatile Throwable throwable;
private final WeakReference<ASURLClassLoader> loader;

private final CleanableSentinelInputStreamState state;
private final Cleaner.Cleanable cleanable;

static class CleanableSentinelInputStreamState implements Runnable {

private boolean closed = false;
private final Throwable throwable;
private final InputStream in;

CleanableSentinelInputStreamState(final InputStream in) {
this.in = in;
this.throwable = new Throwable();
}

@Override
public void run() {
if ( closed ) {
return;
}
try {
in.close();
} catch (IOException ioe) {
_logger.log(Level.WARNING, CULoggerInfo.exceptionClosingStream, ioe);
}

report(this.throwable);
}

/**
* Report "left-overs"!
*/
private void report(Throwable localThrowable) {
_logger.log(Level.WARNING, CULoggerInfo.inputStreamFinalized, localThrowable);
}

public boolean isClosed() {
return closed;
}

public void setClosed(boolean closed) {
this.closed = closed;
}

}
/**
* Constructs new FilteredInputStream which reports InputStreams not closed properly.
* When the garbage collector runs the cleaner. If the stream is still open this class will
Expand All @@ -1119,68 +1149,24 @@ protected static final class SentinelInputStream extends FilterInputStream {
*/
protected SentinelInputStream(ASURLClassLoader loader, final InputStream in) {
super(in);
throwable = new Throwable();
this.loader = new WeakReference<>(loader);
loader.streams.add(this);
registerStopEvent();
this.state = new CleanableSentinelInputStreamState(in);
this.cleanable = CleanerFactory.create().register(this, state);
}

/**
* Closes underlying input stream.
*/
@Override
public void close() throws IOException {
_close();
}

/**
* Callback invoked by Garbage Collector. If underlying InputStream was not closed properly,
* the stack trace of the constructor will be logged!
*
* 'closed' is 'volatile', but it's a race condition to check it and how this code
* relates to _close() is unclear.
*/
public final void registerStopEvent() {
CleanerFactory.create().register(this, () -> closeWithWarning());
}

private synchronized void _close() throws IOException {
if ( closed ) {
if ( state.isClosed() ) {
return;
}
// race condition with above check, but should have no harmful effects

closed = true;
throwable = null;
if (loader.get() != null) {
loader.get().streams.remove(this);
}
state.setClosed(true);
this.cleanable.clean(); // just to unregister runnable
super.close();
}

private void closeWithWarning() {
if ( closed ) {
return;
}

// stores the internal Throwable as it will be set to null in the _close method
Throwable localThrowable = this.throwable;

try {
_close();
} catch (IOException ioe) {
_logger.log(Level.WARNING, CULoggerInfo.exceptionClosingStream, ioe);
}

report(localThrowable);
}

/**
* Report "left-overs"!
*/
private void report(Throwable localThrowable) {
_logger.log(Level.WARNING, CULoggerInfo.inputStreamFinalized, localThrowable);
}
}
/**
* To properly close streams obtained through URL.getResource().getStream():
Expand Down