diff --git a/nucleus/common/common-util/src/main/java/com/sun/enterprise/loader/ASURLClassLoader.java b/nucleus/common/common-util/src/main/java/com/sun/enterprise/loader/ASURLClassLoader.java index ba07e558d8c..2198507a7db 100644 --- a/nucleus/common/common-util/src/main/java/com/sun/enterprise/loader/ASURLClassLoader.java +++ b/nucleus/common/common-util/src/main/java/com/sun/enterprise/loader/ASURLClassLoader.java @@ -53,6 +53,8 @@ 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; @@ -60,7 +62,6 @@ 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; @@ -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. * * @@ -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 streams = new CopyOnWriteArrayList<>(); - private final ArrayList 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. @@ -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(); @@ -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(); } /** @@ -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); - } - }); - } } /** @@ -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(); @@ -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 { @@ -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())) { @@ -1068,14 +1078,8 @@ 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()); @@ -1083,21 +1087,6 @@ public void close() { } } - /** - *Closes any streams that remain open, logging a warning for each. - *

- *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 @@ -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 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 @@ -1119,10 +1149,8 @@ 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); } /** @@ -1130,57 +1158,15 @@ protected SentinelInputStream(ASURLClassLoader loader, final InputStream in) { */ @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():