diff --git a/make/mapfiles/libjava/mapfile-vers b/make/mapfiles/libjava/mapfile-vers index a2f7303f06e..ce5100c539e 100644 --- a/make/mapfiles/libjava/mapfile-vers +++ b/make/mapfiles/libjava/mapfile-vers @@ -74,7 +74,7 @@ SUNWprivate_1.1 { JNU_ThrowStringIndexOutOfBoundsException; JNU_ToString; - Java_java_io_FileDescriptor_close; + Java_java_io_FileDescriptor_close0; Java_java_io_FileDescriptor_initIDs; Java_java_io_FileDescriptor_sync; Java_java_io_FileDescriptor_getAppend; diff --git a/src/java.base/share/classes/java/io/FileInputStream.java b/src/java.base/share/classes/java/io/FileInputStream.java index 5dfe71907fe..72a12ff76de 100644 --- a/src/java.base/share/classes/java/io/FileInputStream.java +++ b/src/java.base/share/classes/java/io/FileInputStream.java @@ -25,6 +25,7 @@ package java.io; +import java.lang.reflect.Method; import java.nio.channels.FileChannel; import sun.nio.ch.FileChannelImpl; @@ -37,6 +38,22 @@ import sun.nio.ch.FileChannelImpl; *

FileInputStream is meant for reading streams of raw bytes * such as image data. For reading streams of characters, consider using * FileReader. + * + * @apiNote + * To release resources used by this stream {@link #close} should be called + * directly or by try-with-resources. Subclasses are responsible for the cleanup + * of resources acquired by the subclass. + * Subclasses that override {@link #finalize} in order to perform cleanup + * should be modified to use alternative cleanup mechanisms such as + * {@link java.lang.ref.Cleaner} and remove the overriding {@code finalize} method. + * + * @implSpec + * If this FileInputStream has been subclassed and the {@link #close} + * method has been overridden, the {@link #close} method will be + * called when the FileInputStream is unreachable. + * Otherwise, it is implementation specific how the resource cleanup described in + * {@link #close} is performed. + * * @author Arthur van Hoff * @see java.io.File @@ -63,6 +80,8 @@ class FileInputStream extends InputStream private volatile boolean closed; + private final AltFinalizer altFinalizer; + /** * Creates a FileInputStream by * opening a connection to an actual file, @@ -137,6 +156,10 @@ class FileInputStream extends InputStream fd.attach(this); path = name; open(name); + altFinalizer = AltFinalizer.get(this); + if (altFinalizer == null) { + fd.registerCleanup(); // open set the fd, register the cleanup + } } /** @@ -173,6 +196,7 @@ class FileInputStream extends InputStream } fd = fdObj; path = null; + altFinalizer = null; /* * FileDescriptor is being shared by streams. @@ -316,6 +340,14 @@ class FileInputStream extends InputStream *

If this stream has an associated channel then the channel is closed * as well. * + * @apiNote + * Overriding {@link #close} to perform cleanup actions is reliable + * only when called directly or when called by try-with-resources. + * Do not depend on finalization to invoke {@code close}; + * finalization is not reliable and is deprecated. + * If cleanup of native resources is needed, other mechanisms such as + * {@linkplain java.lang.ref.Cleaner} should be used. + * * @exception IOException if an I/O error occurs. * * @revised 1.4 @@ -404,16 +436,27 @@ class FileInputStream extends InputStream private static native void initIDs(); - static { initIDs(); } /** - * Ensures that the close method of this file input stream is + * Ensures that the {@link #close} method of this file input stream is * called when there are no more references to it. + * The {@link #finalize} method does not call {@link #close} directly. * - * @deprecated The {@code finalize} method has been deprecated. + * @apiNote + * To release resources used by this stream {@link #close} should be called + * directly or by try-with-resources. + * + * @implSpec + * If this FileInputStream has been subclassed and the {@link #close} + * method has been overridden, the {@link #close} method will be + * called when the FileInputStream is unreachable. + * Otherwise, it is implementation specific how the resource cleanup described in + * {@link #close} is performed. + * + * @deprecated The {@code finalize} method has been deprecated and will be removed. * Subclasses that override {@code finalize} in order to perform cleanup * should be modified to use alternative cleanup mechanisms and * to remove the overriding {@code finalize} method. @@ -425,15 +468,57 @@ class FileInputStream extends InputStream * @exception IOException if an I/O error occurs. * @see java.io.FileInputStream#close() */ - @Deprecated(since="9") + @Deprecated(since="9", forRemoval = true) protected void finalize() throws IOException { - if ((fd != null) && (fd != FileDescriptor.in)) { - /* if fd is shared, the references in FileDescriptor - * will ensure that finalizer is only called when - * safe to do so. All references using the fd have - * become unreachable. We can call close() - */ - close(); + } + + /** + * Class to call {@code FileInputStream.close} when finalized. + * If finalization of the stream is needed, an instance is created + * in its constructor(s). When the set of instances + * related to the stream is unreachable, the AltFinalizer performs + * the needed call to the stream's {@code close} method. + */ + static class AltFinalizer { + private final FileInputStream fis; + + /* + * Returns a finalizer object if the FIS needs a finalizer; otherwise null. + * If the FIS has a close method; it needs an AltFinalizer. + */ + static AltFinalizer get(FileInputStream fis) { + Class clazz = fis.getClass(); + while (clazz != FileInputStream.class) { + try { + clazz.getDeclaredMethod("close"); + return new AltFinalizer(fis); + } catch (NoSuchMethodException nsme) { + // ignore + } + clazz = clazz.getSuperclass(); + } + return null; + } + + private AltFinalizer(FileInputStream fis) { + this.fis = fis; + } + + @Override + @SuppressWarnings("deprecation") + protected final void finalize() { + try { + if ((fis.fd != null) && (fis.fd != FileDescriptor.in)) { + /* if fd is shared, the references in FileDescriptor + * will ensure that finalizer is only called when + * safe to do so. All references using the fd have + * become unreachable. We can call close() + */ + fis.close(); + } + } catch (IOException ioe) { + // ignore + } } } } diff --git a/src/java.base/share/classes/java/io/FileOutputStream.java b/src/java.base/share/classes/java/io/FileOutputStream.java index f9c1baa3518..cb9c8cda0f3 100644 --- a/src/java.base/share/classes/java/io/FileOutputStream.java +++ b/src/java.base/share/classes/java/io/FileOutputStream.java @@ -25,6 +25,7 @@ package java.io; +import java.lang.reflect.Method; import java.nio.channels.FileChannel; import jdk.internal.misc.SharedSecrets; import jdk.internal.misc.JavaIOFileDescriptorAccess; @@ -44,6 +45,21 @@ import sun.nio.ch.FileChannelImpl; * such as image data. For writing streams of characters, consider using * FileWriter. * + * @apiNote + * To release resources used by this stream {@link #close} should be called + * directly or by try-with-resources. Subclasses are responsible for the cleanup + * of resources acquired by the subclass. + * Subclasses that override {@link #finalize} in order to perform cleanup + * should be modified to use alternative cleanup mechanisms such as + * {@link java.lang.ref.Cleaner} and remove the overriding {@code finalize} method. + * + * @implSpec + * If this FileOutputStream has been subclassed and the {@link #close} + * method has been overridden, the {@link #close} method will be + * called when the FileInputStream is unreachable. + * Otherwise, it is implementation specific how the resource cleanup described in + * {@link #close} is performed. + * * @author Arthur van Hoff * @see java.io.File * @see java.io.FileDescriptor @@ -80,6 +96,8 @@ class FileOutputStream extends OutputStream private volatile boolean closed; + private final AltFinalizer altFinalizer; + /** * Creates a file output stream to write to the file with the * specified name. A new FileDescriptor object is @@ -218,6 +236,10 @@ class FileOutputStream extends OutputStream this.path = name; open(name, append); + altFinalizer = AltFinalizer.get(this); + if (altFinalizer == null) { + fd.registerCleanup(); // open set the fd, register the cleanup + } } /** @@ -253,6 +275,7 @@ class FileOutputStream extends OutputStream } this.fd = fdObj; this.path = null; + this.altFinalizer = null; fd.attach(this); } @@ -340,6 +363,14 @@ class FileOutputStream extends OutputStream *

If this stream has an associated channel then the channel is closed * as well. * + * @apiNote + * Overriding {@link #close} to perform cleanup actions is reliable + * only when called directly or when called by try-with-resources. + * Do not depend on finalization to invoke {@code close}; + * finalization is not reliable and is deprecated. + * If cleanup of native resources is needed, other mechanisms such as + * {@linkplain java.lang.ref.Cleaner} should be used. + * * @exception IOException if an I/O error occurs. * * @revised 1.4 @@ -429,34 +460,35 @@ class FileOutputStream extends OutputStream /** * Cleans up the connection to the file, and ensures that the - * close method of this file output stream is + * {@link #close} method of this file output stream is * called when there are no more references to this stream. + * The {@link #finalize} method does not call {@link #close} directly. + * + * @apiNote + * To release resources used by this stream {@link #close} should be called + * directly or by try-with-resources. + * + * @implSpec + * If this FileOutputStream has been subclassed and the {@link #close} + * method has been overridden, the {@link #close} method will be + * called when the FileOutputStream is unreachable. + * Otherwise, it is implementation specific how the resource cleanup described in + * {@link #close} is performed. + * + * @deprecated The {@code finalize} method has been deprecated and will be removed. + * Subclasses that override {@code finalize} in order to perform cleanup + * should be modified to use alternative cleanup mechanisms and + * to remove the overriding {@code finalize} method. + * When overriding the {@code finalize} method, its implementation must explicitly + * ensure that {@code super.finalize()} is invoked as described in {@link Object#finalize}. + * See the specification for {@link Object#finalize()} for further + * information about migration options. * - * @deprecated The {@code finalize} method has been deprecated. - * Subclasses that override {@code finalize} in order to perform cleanup - * should be modified to use alternative cleanup mechanisms and - * to remove the overriding {@code finalize} method. - * When overriding the {@code finalize} method, its implementation must explicitly - * ensure that {@code super.finalize()} is invoked as described in {@link Object#finalize}. - * See the specification for {@link Object#finalize()} for further - * information about migration options. * @exception IOException if an I/O error occurs. * @see java.io.FileInputStream#close() */ - @Deprecated(since="9") + @Deprecated(since="9", forRemoval = true) protected void finalize() throws IOException { - if (fd != null) { - if (fd == FileDescriptor.out || fd == FileDescriptor.err) { - flush(); - } else { - /* if fd is shared, the references in FileDescriptor - * will ensure that finalizer is only called when - * safe to do so. All references using the fd have - * become unreachable. We can call close() - */ - close(); - } - } } private static native void initIDs(); @@ -465,4 +497,59 @@ class FileOutputStream extends OutputStream initIDs(); } + /** + * Class to call {@code FileOutputStream.close} when finalized. + * If finalization of the stream is needed, an instance is created + * in its constructor(s). When the set of instances + * related to the stream is unreachable, the AltFinalizer performs + * the needed call to the stream's {@code close} method. + */ + static class AltFinalizer { + private final FileOutputStream fos; + + /* + * Returns a finalizer object if the FOS needs a finalizer; otherwise null. + * If the FOS has a close method; it needs an AltFinalizer. + */ + static AltFinalizer get(FileOutputStream fos) { + Class clazz = fos.getClass(); + while (clazz != FileOutputStream.class) { + try { + clazz.getDeclaredMethod("close"); + return new AltFinalizer(fos); + } catch (NoSuchMethodException nsme) { + // ignore + } + clazz = clazz.getSuperclass(); + } + return null; + } + + private AltFinalizer(FileOutputStream fos) { + this.fos = fos; + } + + @Override + @SuppressWarnings("deprecation") + protected final void finalize() { + try { + if (fos.fd != null) { + if (fos.fd == FileDescriptor.out || fos.fd == FileDescriptor.err) { + // Subclass may override flush; otherwise it is no-op + fos.flush(); + } else { + /* if fd is shared, the references in FileDescriptor + * will ensure that finalizer is only called when + * safe to do so. All references using the fd have + * become unreachable. We can call close() + */ + fos.close(); + } + } + } catch (IOException ioe) { + // ignore + } + } + } + } diff --git a/src/java.base/share/classes/java/io/RandomAccessFile.java b/src/java.base/share/classes/java/io/RandomAccessFile.java index 5c946d1d715..4b6e66ff140 100644 --- a/src/java.base/share/classes/java/io/RandomAccessFile.java +++ b/src/java.base/share/classes/java/io/RandomAccessFile.java @@ -257,6 +257,7 @@ public class RandomAccessFile implements DataOutput, DataInput, Closeable { fd.attach(this); path = name; open(name, imode); + fd.registerCleanup(); // open sets the fd, register the cleanup } /** diff --git a/src/java.base/share/classes/java/net/SocketInputStream.java b/src/java.base/share/classes/java/net/SocketInputStream.java index 04ee3378eaa..bb3481b7164 100644 --- a/src/java.base/share/classes/java/net/SocketInputStream.java +++ b/src/java.base/share/classes/java/net/SocketInputStream.java @@ -283,7 +283,7 @@ class SocketInputStream extends FileInputStream /** * Overrides finalize, the fd is closed by the Socket. */ - @SuppressWarnings("deprecation") + @SuppressWarnings({"deprecation", "removal"}) protected void finalize() {} /** diff --git a/src/java.base/share/classes/java/net/SocketOutputStream.java b/src/java.base/share/classes/java/net/SocketOutputStream.java index 0ba877bf56e..ddf3dcf9bb5 100644 --- a/src/java.base/share/classes/java/net/SocketOutputStream.java +++ b/src/java.base/share/classes/java/net/SocketOutputStream.java @@ -175,7 +175,7 @@ class SocketOutputStream extends FileOutputStream /** * Overrides finalize, the fd is closed by the Socket. */ - @SuppressWarnings("deprecation") + @SuppressWarnings({"deprecation", "removal"}) protected void finalize() {} /** diff --git a/src/java.base/share/classes/jdk/internal/misc/JavaIOFileDescriptorAccess.java b/src/java.base/share/classes/jdk/internal/misc/JavaIOFileDescriptorAccess.java index 017aefb16af..a82d04d6eed 100644 --- a/src/java.base/share/classes/jdk/internal/misc/JavaIOFileDescriptorAccess.java +++ b/src/java.base/share/classes/jdk/internal/misc/JavaIOFileDescriptorAccess.java @@ -25,6 +25,7 @@ package jdk.internal.misc; import java.io.FileDescriptor; +import java.io.IOException; /* * @author Chris Hegarty @@ -35,7 +36,8 @@ public interface JavaIOFileDescriptorAccess { public int get(FileDescriptor fdo); public void setAppend(FileDescriptor fdo, boolean append); public boolean getAppend(FileDescriptor fdo); - public void close(FileDescriptor fdo); + public void close(FileDescriptor fdo) throws IOException; + public void registerCleanup(FileDescriptor fdo); // Only valid on Windows public void setHandle(FileDescriptor fdo, long handle); diff --git a/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java b/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java index 5e48758bac0..274a17851e2 100644 --- a/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java +++ b/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java @@ -27,6 +27,7 @@ package sun.nio.ch; import java.io.FileDescriptor; import java.io.IOException; +import java.io.UncheckedIOException; import java.lang.ref.Cleaner.Cleanable; import java.nio.ByteBuffer; import java.nio.MappedByteBuffer; @@ -109,7 +110,12 @@ public class FileChannelImpl } public void run() { - fdAccess.close(fd); + try { + fdAccess.close(fd); + } catch (IOException ioe) { + // Rethrow as unchecked so the exception can be propagated as needed + throw new UncheckedIOException("close", ioe); + } } } @@ -188,7 +194,11 @@ public class FileChannelImpl } else if (closer != null) { // Perform the cleaning action so it is not redone when // this channel becomes phantom reachable. - closer.clean(); + try { + closer.clean(); + } catch (UncheckedIOException uioe) { + throw uioe.getCause(); + } } else { fdAccess.close(fd); } diff --git a/src/java.base/unix/classes/java/io/FileDescriptor.java b/src/java.base/unix/classes/java/io/FileDescriptor.java index 6c20e9d5763..5e0b80eb625 100644 --- a/src/java.base/unix/classes/java/io/FileDescriptor.java +++ b/src/java.base/unix/classes/java/io/FileDescriptor.java @@ -25,10 +25,14 @@ package java.io; +import java.lang.ref.Cleaner; import java.util.ArrayList; import java.util.List; + import jdk.internal.misc.JavaIOFileDescriptorAccess; import jdk.internal.misc.SharedSecrets; +import jdk.internal.ref.CleanerFactory; +import jdk.internal.ref.PhantomCleanable; /** * Instances of the file descriptor class serve as an opaque handle @@ -64,7 +68,7 @@ public final class FileDescriptor { SharedSecrets.setJavaIOFileDescriptorAccess( new JavaIOFileDescriptorAccess() { public void set(FileDescriptor fdo, int fd) { - fdo.fd = fd; + fdo.set(fd); } public int get(FileDescriptor fdo) { @@ -79,10 +83,14 @@ public final class FileDescriptor { return fdo.append; } - public void close(FileDescriptor fdo) { + public void close(FileDescriptor fdo) throws IOException { fdo.close(); } + public void registerCleanup(FileDescriptor fdo) { + fdo.registerCleanup(); + } + public void setHandle(FileDescriptor fdo, long handle) { throw new UnsupportedOperationException(); } @@ -94,6 +102,11 @@ public final class FileDescriptor { ); } + /** + * Cleanup in case FileDescriptor is not explicitly closed. + */ + private FDCleanup cleanup; + /** * Constructs an (invalid) FileDescriptor * object. @@ -177,6 +190,34 @@ public final class FileDescriptor { /* This routine initializes JNI field offsets for the class */ private static native void initIDs(); + /** + * Set the fd. + * If setting to -1, clear the cleaner. + * The {@link #registerCleanup()} method should be called for new fds. + * @param fd the fd or -1 to indicate closed + */ + @SuppressWarnings("unchecked") + synchronized void set(int fd) { + if (fd == -1 && cleanup != null) { + cleanup.clear(); + cleanup = null; + } + this.fd = fd; + } + + /** + * Register a cleanup for the current raw fd. + * Used directly in java.io and indirectly via fdAccess. + * The cleanup should be registered after the fd is set in the FileDescriptor. + */ + @SuppressWarnings("unchecked") + synchronized void registerCleanup() { + if (cleanup != null) { + cleanup.clear(); + } + cleanup = FDCleanup.create(this); + } + /** * Returns true, if the file was opened for appending. */ @@ -185,9 +226,30 @@ public final class FileDescriptor { /** * Close the raw file descriptor or handle, if it has not already been closed * and set the fd and handle to -1. + * Clear the cleaner so the close does not happen twice. * Package private to allow it to be used in java.io. + * @throws IOException if close fails */ - native void close(); + @SuppressWarnings("unchecked") + synchronized void close() throws IOException { + if (cleanup != null) { + cleanup.clear(); + cleanup = null; + } + close0(); + } + + /* + * Close the raw file descriptor or handle, if it has not already been closed + * and set the fd and handle to -1. + */ + private native void close0() throws IOException; + + /* + * Raw close of the file descriptor. + * Used only for last chance cleanup. + */ + private static native void cleanupClose0(int fd) throws IOException; /* * Package private methods to track referents. @@ -252,4 +314,45 @@ public final class FileDescriptor { } } } + + /** + * Cleanup for a FileDescriptor when it becomes phantom reachable. + * Create a cleanup if fd != -1. + * Subclassed from {@code PhantomCleanable} so that {@code clear} can be + * called to disable the cleanup when the fd is closed by any means other + * than calling {@link FileDescriptor#close}. + * Otherwise, it may close the native fd after it has been reused. + */ + static final class FDCleanup extends PhantomCleanable { + private final int fd; + + static FDCleanup create(FileDescriptor fdo) { + return fdo.fd == -1 + ? null + : new FDCleanup(fdo, CleanerFactory.cleaner(), fdo.fd); + } + + /** + * Constructor for a phantom cleanable reference. + * @param obj the object to monitor + * @param cleaner the cleaner + * @param fd file descriptor to close + */ + private FDCleanup(Object obj, Cleaner cleaner, int fd) { + super(obj, cleaner); + this.fd = fd; + } + + /** + * Close the native fd. + */ + @Override + protected void performCleanup() { + try { + cleanupClose0(fd); + } catch (IOException ioe) { + throw new UncheckedIOException("close", ioe); + } + } + } } diff --git a/src/java.base/unix/native/libjava/FileDescriptor_md.c b/src/java.base/unix/native/libjava/FileDescriptor_md.c index fcde1ef7f93..49669384489 100644 --- a/src/java.base/unix/native/libjava/FileDescriptor_md.c +++ b/src/java.base/unix/native/libjava/FileDescriptor_md.c @@ -71,8 +71,17 @@ Java_java_io_FileDescriptor_getAppend(JNIEnv *env, jclass fdClass, jint fd) { return ((flags & O_APPEND) == 0) ? JNI_FALSE : JNI_TRUE; } +JNIEXPORT void JNICALL +Java_java_io_FileDescriptor_cleanupClose0(JNIEnv *env, jclass fdClass, jint fd) { + if (fd != -1) { + if (close(fd) == -1) { + JNU_ThrowIOExceptionWithLastError(env, "close failed"); + } + } +} + // instance method close0 for FileDescriptor JNIEXPORT void JNICALL -Java_java_io_FileDescriptor_close(JNIEnv *env, jobject this) { +Java_java_io_FileDescriptor_close0(JNIEnv *env, jobject this) { fileDescriptorClose(env, this); } diff --git a/src/java.base/unix/native/libjava/io_util_md.c b/src/java.base/unix/native/libjava/io_util_md.c index 64374b50196..1ec21237856 100644 --- a/src/java.base/unix/native/libjava/io_util_md.c +++ b/src/java.base/unix/native/libjava/io_util_md.c @@ -139,6 +139,11 @@ fileDescriptorClose(JNIEnv *env, jobject this) if ((*env)->ExceptionOccurred(env)) { return; } + + if (fd == -1) { + return; // already closed and set to -1 + } + /* Set the fd to -1 before closing it so that the timing window * of other threads using the wrong fd (closed but recycled fd, * that gets re-opened with some other filename) is reduced. diff --git a/src/java.base/windows/classes/java/io/FileDescriptor.java b/src/java.base/windows/classes/java/io/FileDescriptor.java index 21fbd951fa5..ecc1934795d 100644 --- a/src/java.base/windows/classes/java/io/FileDescriptor.java +++ b/src/java.base/windows/classes/java/io/FileDescriptor.java @@ -25,10 +25,14 @@ package java.io; +import java.lang.ref.Cleaner; import java.util.ArrayList; import java.util.List; + import jdk.internal.misc.JavaIOFileDescriptorAccess; import jdk.internal.misc.SharedSecrets; +import jdk.internal.ref.CleanerFactory; +import jdk.internal.ref.PhantomCleanable; /** * Instances of the file descriptor class serve as an opaque handle @@ -81,12 +85,16 @@ public final class FileDescriptor { return fdo.append; } - public void close(FileDescriptor fdo) { + public void close(FileDescriptor fdo) throws IOException { fdo.close(); } + public void registerCleanup(FileDescriptor fdo) { + fdo.registerCleanup(); + } + public void setHandle(FileDescriptor fdo, long handle) { - fdo.handle = handle; + fdo.setHandle(handle); } public long getHandle(FileDescriptor fdo) { @@ -96,6 +104,11 @@ public final class FileDescriptor { ); } + /** + * Cleanup in case FileDescriptor is not explicitly closed. + */ + private FDCleanup cleanup; + /** * Constructs an (invalid) FileDescriptor * object. @@ -149,7 +162,7 @@ public final class FileDescriptor { * relevant device(s). In particular, if this FileDescriptor * refers to a physical storage medium, such as a file in a file * system, sync will not return until all in-memory modified copies - * of buffers associated with this FileDesecriptor have been + * of buffers associated with this FileDescriptor have been * written to the physical medium. * * sync is meant to be used by code that requires physical @@ -175,20 +188,69 @@ public final class FileDescriptor { /* This routine initializes JNI field offsets for the class */ private static native void initIDs(); - private static native long set(int d); - private static FileDescriptor standardStream(int fd) { FileDescriptor desc = new FileDescriptor(); - desc.handle = set(fd); + desc.handle = getHandle(fd); return desc; } + private static native long getHandle(int d); + + /** + * Set the handle. + * If setting to -1, clear the cleaner. + * The {@link #registerCleanup()} method should be called for new handles. + * @param handle the handle or -1 to indicate closed + */ + @SuppressWarnings("unchecked") + void setHandle(long handle) { + if (handle == -1 && cleanup != null) { + cleanup.clear(); + cleanup = null; + } + this.handle = handle; + } + + /** + * Register a cleanup for the current handle. + * Used directly in java.io and indirectly via fdAccess. + * The cleanup should be registered after the handle is set in the FileDescriptor. + */ + @SuppressWarnings("unchecked") + synchronized void registerCleanup() { + if (cleanup != null) { + cleanup.clear(); + } + cleanup = FDCleanup.create(this); + } + /** * Close the raw file descriptor or handle, if it has not already been closed * and set the fd and handle to -1. + * Clear the cleaner so the close does not happen twice. * Package private to allow it to be used in java.io. + * @throws IOException if close fails */ - native void close(); + @SuppressWarnings("unchecked") + synchronized void close() throws IOException { + if (cleanup != null) { + cleanup.clear(); + cleanup = null; + } + close0(); + } + + /* + * Close the raw file descriptor or handle, if it has not already been closed + * and set the fd and handle to -1. + */ + private native void close0() throws IOException; + + /* + * Raw close of the file handle. + * Used only for last chance cleanup. + */ + private static native void cleanupClose0(long handle) throws IOException; /* * Package private methods to track referents. @@ -253,4 +315,45 @@ public final class FileDescriptor { } } } + + /** + * Cleanup for a FileDescriptor when it becomes phantom reachable. + * Create a cleanup if handle != -1. + * Subclassed from {@code PhantomCleanable} so that {@code clear} can be + * called to disable the cleanup when the fd is closed by any means other + * than calling {@link FileDescriptor#close}. + * Otherwise, it may close the handle after it has been reused. + */ + static final class FDCleanup extends PhantomCleanable { + private final long handle; + + static FDCleanup create(FileDescriptor fdo) { + return fdo.handle == -1 + ? null + : new FDCleanup(fdo, CleanerFactory.cleaner(), fdo.handle); + } + + /** + * Constructor for a phantom cleanable reference. + * @param obj the object to monitor + * @param cleaner the cleaner + * @param handle file handle to close + */ + private FDCleanup(Object obj, Cleaner cleaner, long handle) { + super(obj, cleaner); + this.handle = handle; + } + + /** + * Close the native handle. + */ + @Override + protected void performCleanup() { + try { + cleanupClose0(handle); + } catch (IOException ioe) { + throw new UncheckedIOException("close", ioe); + } + } + } } diff --git a/src/java.base/windows/classes/sun/nio/ch/FileDispatcherImpl.java b/src/java.base/windows/classes/sun/nio/ch/FileDispatcherImpl.java index 23adf004e6d..9424d36f892 100644 --- a/src/java.base/windows/classes/sun/nio/ch/FileDispatcherImpl.java +++ b/src/java.base/windows/classes/sun/nio/ch/FileDispatcherImpl.java @@ -114,6 +114,7 @@ class FileDispatcherImpl extends FileDispatcher { FileDescriptor result = new FileDescriptor(); long handle = duplicateHandle(fdAccess.getHandle(fd)); fdAccess.setHandle(result, handle); + fdAccess.registerCleanup(result); return result; } diff --git a/src/java.base/windows/classes/sun/nio/ch/WindowsAsynchronousFileChannelImpl.java b/src/java.base/windows/classes/sun/nio/ch/WindowsAsynchronousFileChannelImpl.java index 30f62f22f71..2a4fda647d8 100644 --- a/src/java.base/windows/classes/sun/nio/ch/WindowsAsynchronousFileChannelImpl.java +++ b/src/java.base/windows/classes/sun/nio/ch/WindowsAsynchronousFileChannelImpl.java @@ -139,7 +139,7 @@ public class WindowsAsynchronousFileChannelImpl invalidateAllLocks(); // close the file - close0(handle); + nd.close(fdObj); // waits until all I/O operations have completed ioCache.close(); @@ -728,8 +728,6 @@ public class WindowsAsynchronousFileChannelImpl private static native int lockFile(long handle, long position, long size, boolean shared, long overlapped) throws IOException; - private static native void close0(long handle); - static { IOUtil.load(); } diff --git a/src/java.base/windows/classes/sun/nio/fs/WindowsChannelFactory.java b/src/java.base/windows/classes/sun/nio/fs/WindowsChannelFactory.java index 16d65c78a30..3e219c98f24 100644 --- a/src/java.base/windows/classes/sun/nio/fs/WindowsChannelFactory.java +++ b/src/java.base/windows/classes/sun/nio/fs/WindowsChannelFactory.java @@ -216,8 +216,7 @@ class WindowsChannelFactory { } catch (IOException x) { // IOException is thrown if the file handle cannot be associated // with the completion port. All we can do is close the file. - long handle = fdAccess.getHandle(fdObj); - CloseHandle(handle); + fdAccess.close(fdObj); throw x; } } @@ -347,6 +346,7 @@ class WindowsChannelFactory { FileDescriptor fdObj = new FileDescriptor(); fdAccess.setHandle(fdObj, handle); fdAccess.setAppend(fdObj, flags.append); + fdAccess.registerCleanup(fdObj); return fdObj; } } diff --git a/src/java.base/windows/native/libjava/FileDescriptor_md.c b/src/java.base/windows/native/libjava/FileDescriptor_md.c index 0cd3a7321a7..8b64741b08a 100644 --- a/src/java.base/windows/native/libjava/FileDescriptor_md.c +++ b/src/java.base/windows/native/libjava/FileDescriptor_md.c @@ -57,7 +57,7 @@ Java_java_io_FileDescriptor_initIDs(JNIEnv *env, jclass fdClass) { } JNIEXPORT jlong JNICALL -Java_java_io_FileDescriptor_set(JNIEnv *env, jclass fdClass, jint fd) { +Java_java_io_FileDescriptor_getHandle(JNIEnv *env, jclass fdClass, jint fd) { SET_HANDLE(fd); } @@ -73,8 +73,17 @@ Java_java_io_FileDescriptor_sync(JNIEnv *env, jobject this) { } } +JNIEXPORT void JNICALL +Java_java_io_FileDescriptor_cleanupClose0(JNIEnv *env, jclass fdClass, jlong handle) { + if (handle != -1) { + if (CloseHandle((HANDLE)handle) == -1) { + JNU_ThrowIOExceptionWithLastError(env, "close failed"); + } + } +} + // instance method close0 for FileDescriptor JNIEXPORT void JNICALL -Java_java_io_FileDescriptor_close(JNIEnv *env, jobject this) { +Java_java_io_FileDescriptor_close0(JNIEnv *env, jobject this) { fileDescriptorClose(env, this); } diff --git a/src/java.base/windows/native/libnio/ch/WindowsAsynchronousFileChannelImpl.c b/src/java.base/windows/native/libnio/ch/WindowsAsynchronousFileChannelImpl.c index e24ca6e52b2..2269231f410 100644 --- a/src/java.base/windows/native/libnio/ch/WindowsAsynchronousFileChannelImpl.c +++ b/src/java.base/windows/native/libnio/ch/WindowsAsynchronousFileChannelImpl.c @@ -121,13 +121,3 @@ Java_sun_nio_ch_WindowsAsynchronousFileChannelImpl_lockFile(JNIEnv *env, jobject return 0; } -JNIEXPORT void JNICALL -Java_sun_nio_ch_WindowsAsynchronousFileChannelImpl_close0(JNIEnv* env, jclass this, - jlong handle) -{ - HANDLE h = (HANDLE)jlong_to_ptr(handle); - BOOL result = CloseHandle(h); - if (result == 0) { - JNU_ThrowIOExceptionWithLastError(env, "Close failed"); - } -} diff --git a/test/jdk/java/io/FileInputStream/FinalizeShdCallClose.java b/test/jdk/java/io/FileInputStream/FinalizeShdCallClose.java deleted file mode 100644 index 6ebcbf8c8ec..00000000000 --- a/test/jdk/java/io/FileInputStream/FinalizeShdCallClose.java +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Copyright (c) 2007, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -/** - * - * @test - * @bug 6524062 - * @summary Test to ensure that FIS.finalize() invokes the close() method as per - * the specification. - */ -import java.io.File; -import java.io.FileInputStream; -import java.io.FileNotFoundException; -import java.io.IOException; - -public class FinalizeShdCallClose { - - static final String FILE_NAME = "empty.txt"; - - public static class MyStream extends FileInputStream { - private boolean closed = false; - - public MyStream(String name) throws FileNotFoundException { - super(name); - } - - public void finalize() { - try { - super.finalize(); - } catch (IOException ioe) { - ioe.printStackTrace(); - } - } - - public void close() { - try { - super.close(); - } catch (IOException ioe) { - ioe.printStackTrace(); - } - closed = true; - } - - public boolean isClosed() { - return closed; - } - } - - /* standalone interface */ - public static void main(String argv[]) throws Exception { - - File inFile= new File(System.getProperty("test.dir", "."), FILE_NAME); - inFile.createNewFile(); - inFile.deleteOnExit(); - - String name = inFile.getPath(); - MyStream ms = null; - try { - ms = new MyStream(name); - } catch (FileNotFoundException e) { - System.out.println("Unexpected exception " + e); - throw(e); - } - ms.finalize(); - if (!ms.isClosed()) { - throw new Exception("MyStream.close() method is not called"); - } - System.out.println("OK"); - } -} diff --git a/test/jdk/java/io/FileInputStream/UnreferencedFISClosesFd.java b/test/jdk/java/io/FileInputStream/UnreferencedFISClosesFd.java new file mode 100644 index 00000000000..7664348f329 --- /dev/null +++ b/test/jdk/java/io/FileInputStream/UnreferencedFISClosesFd.java @@ -0,0 +1,247 @@ +/* + * Copyright (c) 2007, 2017, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * + * @test + * @modules java.base/java.io:open + * @bug 6524062 + * @summary Test to ensure that FIS.finalize() invokes the close() method as per + * the specification. + * @run main/othervm UnreferencedFISClosesFd + */ +import java.io.File; +import java.io.FileDescriptor; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; +import java.lang.reflect.Field; +import java.util.HashSet; +import java.util.concurrent.atomic.AtomicInteger; + + +/** + * Tests for FIS unreferenced. + * - Not subclassed - cleaner cleanup + * - Subclassed no finalize or close - cleaner cleanup + * - Subclassed close overridden - AltFinalizer cleanup + * - Subclasses finalize overridden - cleaner cleanup + * - Subclasses finalize and close overridden - AltFinalizer cleanup + */ +public class UnreferencedFISClosesFd { + + enum CleanupType { + CLOSE, // Cleanup is handled via calling close + CLEANER} // Cleanup is handled via Cleaner + + static final String FILE_NAME = "empty.txt"; + + /** + * Subclass w/ no overrides; not finalize or close. + * Cleanup should be via the Cleaner (not close). + */ + public static class StreamOverrides extends FileInputStream { + + protected final AtomicInteger closeCounter; + + public StreamOverrides(String name) throws FileNotFoundException { + super(name); + closeCounter = new AtomicInteger(0); + } + + final AtomicInteger closeCounter() { + return closeCounter; + } + } + + /** + * Subclass overrides close. + * Cleanup should be via AltFinalizer calling close(). + */ + public static class StreamOverridesClose extends StreamOverrides { + + public StreamOverridesClose(String name) throws FileNotFoundException { + super(name); + } + + public void close() throws IOException { + closeCounter.incrementAndGet(); + super.close(); + } + } + + /** + * Subclass overrides finalize. + * Cleanup should be via the Cleaner (not close). + */ + public static class StreamOverridesFinalize extends StreamOverrides { + + public StreamOverridesFinalize(String name) throws FileNotFoundException { + super(name); + } + + @SuppressWarnings({"deprecation","removal"}) + protected void finalize() throws IOException { + super.finalize(); + } + } + + /** + * Subclass overrides finalize and close. + * Cleanup should be via AltFinalizer calling close(). + */ + public static class StreamOverridesFinalizeClose extends StreamOverridesClose { + + public StreamOverridesFinalizeClose(String name) throws FileNotFoundException { + super(name); + } + + @SuppressWarnings({"deprecation","removal"}) + protected void finalize() throws IOException { + super.finalize(); + } + } + + /** + * Main runs each test case and reports number of failures. + */ + public static void main(String argv[]) throws Exception { + + File inFile = new File(System.getProperty("test.dir", "."), FILE_NAME); + inFile.createNewFile(); + inFile.deleteOnExit(); + + String name = inFile.getPath(); + + int failCount = 0; + failCount += test(new FileInputStream(name), CleanupType.CLEANER); + + failCount += test(new StreamOverrides(name), CleanupType.CLEANER); + + failCount += test(new StreamOverridesClose(name), CleanupType.CLOSE); + + failCount += test(new StreamOverridesFinalize(name), CleanupType.CLEANER); + + failCount += test(new StreamOverridesFinalizeClose(name), CleanupType.CLOSE); + + if (failCount > 0) { + throw new AssertionError("Failed test count: " + failCount); + } + } + + private static int test(FileInputStream fis, CleanupType cleanType) throws Exception { + + try { + System.out.printf("%nTesting %s%n", fis.getClass().getName()); + + // Prepare to wait for FIS to be reclaimed + ReferenceQueue queue = new ReferenceQueue<>(); + HashSet> pending = new HashSet<>(); + WeakReference msWeak = new WeakReference<>(fis, queue); + pending.add(msWeak); + + FileDescriptor fd = fis.getFD(); + WeakReference fdWeak = new WeakReference<>(fd, queue); + pending.add(fdWeak); + + Field fdField = FileDescriptor.class.getDeclaredField("fd"); + fdField.setAccessible(true); + int ffd = fdField.getInt(fd); + + Field altFinalizerField = FileInputStream.class.getDeclaredField("altFinalizer"); + altFinalizerField.setAccessible(true); + Object altFinalizer = altFinalizerField.get(fis); + if ((altFinalizer != null) ^ (cleanType == CleanupType.CLOSE)) { + throw new RuntimeException("Unexpected AltFinalizer: " + altFinalizer + + ", for " + cleanType); + } + + Field cleanupField = FileDescriptor.class.getDeclaredField("cleanup"); + cleanupField.setAccessible(true); + Object cleanup = cleanupField.get(fd); + System.out.printf(" cleanup: %s, alt: %s, ffd: %d, cf: %s%n", + cleanup, altFinalizer, ffd, cleanupField); + if ((cleanup != null) ^ (cleanType == CleanupType.CLEANER)) { + throw new Exception("unexpected cleanup: " + + cleanup + ", for " + cleanType); + } + if (cleanup != null) { + WeakReference cleanupWeak = new WeakReference<>(cleanup, queue); + pending.add(cleanupWeak); + System.out.printf(" fdWeak: %s%n msWeak: %s%n cleanupWeak: %s%n", + fdWeak, msWeak, cleanupWeak); + } + if (altFinalizer != null) { + WeakReference altFinalizerWeak = new WeakReference<>(altFinalizer, queue); + pending.add(altFinalizerWeak); + System.out.printf(" fdWeak: %s%n msWeak: %s%n altFinalizerWeak: %s%n", + fdWeak, msWeak, altFinalizerWeak); + } + + AtomicInteger closeCounter = fis instanceof StreamOverrides + ? ((StreamOverrides)fis).closeCounter() : null; + + Reference r; + while (((r = queue.remove(1000L)) != null) + || !pending.isEmpty()) { + System.out.printf(" r: %s, pending: %d%n", + r, pending.size()); + if (r != null) { + pending.remove(r); + } else { + fis = null; + fd = null; + cleanup = null; + altFinalizer = null; + System.gc(); // attempt to reclaim them + } + } + Reference.reachabilityFence(fd); + Reference.reachabilityFence(fis); + Reference.reachabilityFence(cleanup); + Reference.reachabilityFence(altFinalizer); + + // Confirm the correct number of calls to close depending on the cleanup type + switch (cleanType) { + case CLEANER: + if (closeCounter != null && closeCounter.get() > 0) { + throw new RuntimeException("Close should not have been called: count: " + + closeCounter); + } + break; + case CLOSE: + if (closeCounter == null || closeCounter.get() == 0) { + throw new RuntimeException("Close should have been called: count: 0"); + } + break; + } + } catch (Exception ex) { + ex.printStackTrace(System.out); + return 1; + } + return 0; + } +} diff --git a/test/jdk/java/io/FileOutputStream/FinalizeShdCallClose.java b/test/jdk/java/io/FileOutputStream/FinalizeShdCallClose.java deleted file mode 100644 index a981a5e4154..00000000000 --- a/test/jdk/java/io/FileOutputStream/FinalizeShdCallClose.java +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Copyright (c) 2007, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -/** - * - * @test - * @bug 6524062 - * @summary Test to ensure that FOS.finalize() invokes the close() method as per - * the specification. - */ -import java.io.File; -import java.io.FileOutputStream; -import java.io.FileNotFoundException; -import java.io.IOException; - -public class FinalizeShdCallClose { - - static final String FILE_NAME = "empty.txt"; - - public static class MyStream extends FileOutputStream { - private boolean closed = false; - - public MyStream(String name) throws FileNotFoundException { - super(name); - } - - public void finalize() { - try { - super.finalize(); - } catch (IOException ioe) { - ioe.printStackTrace(); - } - } - - public void close() { - try { - super.close(); - } catch (IOException ioe) { - ioe.printStackTrace(); - } - closed = true; - } - - public boolean isClosed() { - return closed; - } - } - - /* standalone interface */ - public static void main(String argv[]) throws Exception { - - File inFile= new File(System.getProperty("test.dir", "."), FILE_NAME); - inFile.createNewFile(); - inFile.deleteOnExit(); - - String name = inFile.getPath(); - MyStream ms = null; - try { - ms = new MyStream(name); - } catch (FileNotFoundException e) { - System.out.println("Unexpected exception " + e); - throw(e); - } - ms.finalize(); - if (!ms.isClosed()) { - throw new Exception("MyStream.close() method is not called"); - } - System.out.println("OK"); - } -} diff --git a/test/jdk/java/io/FileOutputStream/UnreferencedFOSClosesFd.java b/test/jdk/java/io/FileOutputStream/UnreferencedFOSClosesFd.java new file mode 100644 index 00000000000..71d0e0008f3 --- /dev/null +++ b/test/jdk/java/io/FileOutputStream/UnreferencedFOSClosesFd.java @@ -0,0 +1,236 @@ +/* + * Copyright (c) 2007, 2017, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * + * @test + * @modules java.base/java.io:open + * @bug 6524062 + * @summary Test to ensure that FOS.finalize() invokes the close() method as per + * the specification. + * @run main/othervm UnreferencedFOSClosesFd + */ +import java.io.*; +import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; +import java.lang.reflect.Field; +import java.util.HashSet; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; + +public class UnreferencedFOSClosesFd { + + enum CleanupType { + CLOSE, // Cleanup is handled via calling close + CLEANER} // Cleanup is handled via Cleaner + + static final String FILE_NAME = "empty.txt"; + + /** + * Subclass w/ no overrides; not finalize or close. + * Cleanup should be via the Cleaner (not close). + */ + public static class StreamOverrides extends FileOutputStream { + + protected final AtomicInteger closeCounter; + + public StreamOverrides(String name) throws FileNotFoundException { + super(name); + closeCounter = new AtomicInteger(0); + } + + final AtomicInteger closeCounter() { + return closeCounter; + } + } + + /** + * Subclass overrides close. + * Cleanup should be via AltFinalizer calling close(). + */ + public static class StreamOverridesClose extends StreamOverrides { + + public StreamOverridesClose(String name) throws FileNotFoundException { + super(name); + } + + public void close() throws IOException { + closeCounter.incrementAndGet(); + super.close(); + } + } + + /** + * Subclass overrides finalize and close. + * Cleanup should be via the Cleaner (not close). + */ + public static class StreamOverridesFinalize extends StreamOverrides { + + public StreamOverridesFinalize(String name) throws FileNotFoundException { + super(name); + } + + @SuppressWarnings({"deprecation","removal"}) + protected void finalize() throws IOException { + super.finalize(); + } + } + + /** + * Subclass overrides finalize and close. + * Cleanup should be via AltFinalizer calling close(). + */ + public static class StreamOverridesFinalizeClose extends StreamOverridesClose { + + public StreamOverridesFinalizeClose(String name) throws FileNotFoundException { + super(name); + } + + @SuppressWarnings({"deprecation","removal"}) + protected void finalize() throws IOException { + super.finalize(); + } + } + + /** + * Main runs each test case and reports number of failures. + */ + public static void main(String argv[]) throws Exception { + + File inFile = new File(System.getProperty("test.dir", "."), FILE_NAME); + inFile.createNewFile(); + inFile.deleteOnExit(); + + String name = inFile.getPath(); + + int failCount = 0; + failCount += test(new FileOutputStream(name), CleanupType.CLEANER); + + failCount += test(new StreamOverrides(name), CleanupType.CLEANER); + + failCount += test(new StreamOverridesClose(name), CleanupType.CLOSE); + + failCount += test(new StreamOverridesFinalize(name), CleanupType.CLEANER); + + failCount += test(new StreamOverridesFinalizeClose(name), CleanupType.CLOSE); + + if (failCount > 0) { + throw new AssertionError("Failed test count: " + failCount); + } + } + + + private static int test(FileOutputStream fos, CleanupType cleanType) throws Exception { + + try { + System.out.printf("%nTesting %s%n", fos.getClass().getName()); + + // Prepare to wait for FOS to be reclaimed + ReferenceQueue queue = new ReferenceQueue<>(); + HashSet> pending = new HashSet<>(); + WeakReference msWeak = new WeakReference<>(fos, queue); + pending.add(msWeak); + + FileDescriptor fd = fos.getFD(); + WeakReference fdWeak = new WeakReference<>(fd, queue); + pending.add(fdWeak); + + Field fdField = FileDescriptor.class.getDeclaredField("fd"); + fdField.setAccessible(true); + int ffd = fdField.getInt(fd); + + Field altFinalizerField = FileOutputStream.class.getDeclaredField("altFinalizer"); + altFinalizerField.setAccessible(true); + Object altFinalizer = altFinalizerField.get(fos); + if ((altFinalizer != null) ^ (cleanType == CleanupType.CLOSE)) { + throw new RuntimeException("Unexpected AltFinalizer: " + altFinalizer + + ", for " + cleanType); + } + + Field cleanupField = FileDescriptor.class.getDeclaredField("cleanup"); + cleanupField.setAccessible(true); + Object cleanup = cleanupField.get(fd); + System.out.printf(" cleanup: %s, alt: %s, ffd: %d, cf: %s%n", + cleanup, altFinalizer, ffd, cleanupField); + if ((cleanup != null) ^ (cleanType == CleanupType.CLEANER)) { + throw new Exception("unexpected cleanup: " + + cleanup + ", for " + cleanType); + } + if (cleanup != null) { + WeakReference cleanupWeak = new WeakReference<>(cleanup, queue); + pending.add(cleanupWeak); + System.out.printf(" fdWeak: %s%n msWeak: %s%n cleanupWeak: %s%n", + fdWeak, msWeak, cleanupWeak); + } + if (altFinalizer != null) { + WeakReference altFinalizerWeak = new WeakReference<>(altFinalizer, queue); + pending.add(altFinalizerWeak); + System.out.printf(" fdWeak: %s%n msWeak: %s%n altFinalizerWeak: %s%n", + fdWeak, msWeak, altFinalizerWeak); + } + + AtomicInteger closeCounter = fos instanceof StreamOverrides + ? ((StreamOverrides) fos).closeCounter() : null; + + Reference r; + while (((r = queue.remove(1000L)) != null) + || !pending.isEmpty()) { + System.out.printf(" r: %s, pending: %d%n", + r, pending.size()); + if (r != null) { + pending.remove(r); + } else { + fos = null; + fd = null; + cleanup = null; + altFinalizer = null; + System.gc(); // attempt to reclaim them + } + } + Reference.reachabilityFence(fd); + Reference.reachabilityFence(fos); + Reference.reachabilityFence(cleanup); + Reference.reachabilityFence(altFinalizer); + + // Confirm the correct number of calls to close depending on the cleanup type + switch (cleanType) { + case CLEANER: + if (closeCounter != null && closeCounter.get() > 0) { + throw new RuntimeException("Close should not have been called: count: " + + closeCounter); + } + break; + case CLOSE: + if (closeCounter == null || closeCounter.get() == 0) { + throw new RuntimeException("Close should have been called: count: 0"); + } + break; + } + } catch (Exception ex) { + ex.printStackTrace(System.out); + return 1; + } + return 0; + } +} diff --git a/test/jdk/java/io/RandomAccessFile/UnreferencedRAFClosesFd.java b/test/jdk/java/io/RandomAccessFile/UnreferencedRAFClosesFd.java new file mode 100644 index 00000000000..b6ebff6a14b --- /dev/null +++ b/test/jdk/java/io/RandomAccessFile/UnreferencedRAFClosesFd.java @@ -0,0 +1,96 @@ +/* + * Copyright (c) 2007, 2017, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import java.io.File; +import java.io.FileDescriptor; +import java.io.FileOutputStream; +import java.io.FileNotFoundException; +import java.io.RandomAccessFile; +import java.lang.ref.Cleaner; +import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; +import java.lang.reflect.Field; +import java.util.HashSet; + +/** + * @test + * @bug 8080225 + * @modules java.base/java.io:open + * @summary Test to ensure that an unclosed and unreferenced RandomAccessFile closes the fd + * @run main/othervm UnreferencedRAFClosesFd + */ +public class UnreferencedRAFClosesFd { + + static final String FILE_NAME = "empty.txt"; + + /* standalone interface */ + public static void main(String argv[]) throws Exception { + + File inFile= new File(System.getProperty("test.dir", "."), FILE_NAME); + inFile.createNewFile(); + inFile.deleteOnExit(); + + String name = inFile.getPath(); + RandomAccessFile raf; + try { + // raf is explicitly *not* closed to allow cleaner to work + raf = new RandomAccessFile(name, "rw"); + } catch (FileNotFoundException e) { + System.out.println("Unexpected exception " + e); + throw(e); + } + FileDescriptor fd = raf.getFD(); + + Field fdField = FileDescriptor.class.getDeclaredField("cleanup"); + fdField.setAccessible(true); + Cleaner.Cleanable cleanup = (Cleaner.Cleanable)fdField.get(fd); + + // Prepare to wait for FOS, FD, Cleanup to be reclaimed + ReferenceQueue queue = new ReferenceQueue<>(); + HashSet> pending = new HashSet<>(3); + pending.add(new WeakReference<>(cleanup, queue)); + pending.add(new WeakReference<>(raf, queue)); + pending.add(new WeakReference<>(fd, queue)); + + Reference r; + while (((r = queue.remove(10L)) != null) + || !pending.isEmpty()) { + System.out.printf("r: %s, pending: %d%n", r, pending.size()); + if (r != null) { + pending.remove(r); + } else { + cleanup = null; + raf = null; + fd = null; + System.gc(); // attempt to reclaim the RAF, cleanup, and fd + } + } + + // Keep these variables in scope as gc roots + Reference.reachabilityFence(cleanup); + Reference.reachabilityFence(fd); + Reference.reachabilityFence(raf); + Reference.reachabilityFence(pending); + } +} diff --git a/test/jdk/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java b/test/jdk/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java index e7fafc10fb5..860b2d99910 100644 --- a/test/jdk/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java +++ b/test/jdk/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java @@ -24,26 +24,34 @@ /** * @test * @bug 8047769 - * @modules java.base/java.lang.ref:open + * @modules java.base/java.io:open + * java.base/java.lang.ref:open * java.base/sun.security.provider:open * @summary SecureRandom should be more frugal with file descriptors */ import java.io.File; +import java.io.FileDescriptor; +import java.io.FileInputStream; import java.io.FileOutputStream; +import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; +import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.UndeclaredThrowableException; import java.util.Arrays; +import java.util.HashSet; public class FileInputStreamPoolTest { static final byte[] bytes = new byte[]{1, 2, 3, 4, 5, 6, 7, 8}; - static void testCaching(File file) throws IOException { + static FilterInputStream testCaching(File file) throws IOException { InputStream in1 = TestProxy.FileInputStreamPool_getInputStream(file); InputStream in2 = TestProxy.FileInputStreamPool_getInputStream(file); assertTrue(in1 == in2, @@ -58,6 +66,8 @@ public class FileInputStreamPoolTest { assertTrue(Arrays.equals(readBytes, bytes), "readBytes: " + Arrays.toString(readBytes) + " not equal to expected: " + Arrays.toString(bytes)); + + return (FilterInputStream)in1; } static void assertTrue(boolean test, String message) { @@ -66,13 +76,34 @@ public class FileInputStreamPoolTest { } } - static void processReferences() { - // make JVM process References - System.gc(); - // help ReferenceHandler thread enqueue References - while (TestProxy.Reference_waitForReferenceProcessing()) { } - // help run Finalizers - System.runFinalization(); + static void processReferences(FilterInputStream in1) throws InterruptedException, IOException { + FileInputStream fis = TestProxy.FilterInputStream_getInField(in1); + FileDescriptor fd = fis.getFD(); + System.out.printf("fis: %s, fd: %s%n", fis, fd); + // Prepare to wait for FD to be reclaimed + ReferenceQueue queue = new ReferenceQueue<>(); + HashSet> pending = new HashSet<>(); + pending.add(new WeakReference<>(in1, queue)); + pending.add(new WeakReference<>(fis, queue)); + pending.add(new WeakReference<>(fd, queue)); + + Reference r; + while (((r = queue.remove(10L)) != null) + || !pending.isEmpty()) { + System.out.printf("r: %s, pending: %d%n", r, pending.size()); + if (r != null) { + pending.remove(r); + } else { + fd = null; + fis = null; + in1 = null; + System.gc(); // attempt to reclaim the FD + } + Thread.sleep(10L); + } + Reference.reachabilityFence(fd); + Reference.reachabilityFence(fis); + Reference.reachabilityFence(in1); } public static void main(String[] args) throws Exception { @@ -88,16 +119,14 @@ public class FileInputStreamPoolTest { out.write(bytes); } - // test caching 1t time - testCaching(file); + // test caching 1st time - processReferences(); + processReferences(testCaching(file)); // test caching 2nd time - this should only succeed if the stream // is re-opened as a consequence of cleared WeakReference - testCaching(file); - processReferences(); + processReferences(testCaching(file)); } } @@ -109,6 +138,7 @@ public class FileInputStreamPoolTest { static class TestProxy { private static final Method getInputStreamMethod; private static final Method waitForReferenceProcessingMethod; + private static final Field inField; static { try { @@ -122,6 +152,9 @@ public class FileInputStreamPoolTest { waitForReferenceProcessingMethod = Reference.class.getDeclaredMethod("waitForReferenceProcessing"); waitForReferenceProcessingMethod.setAccessible(true); + + inField = FilterInputStream.class.getDeclaredField("in"); + inField.setAccessible(true); } catch (Exception e) { throw new Error(e); } @@ -165,5 +198,13 @@ public class FileInputStreamPoolTest { throw new RuntimeException(e); } } + + static FileInputStream FilterInputStream_getInField(FilterInputStream fis) { + try { + return (FileInputStream) inField.get(fis); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } + } } }