8080225: FileInput/OutputStream/FileChannel cleanup should be improved
Reviewed-by: mchung, plevart, bpb
--- a/make/mapfiles/libjava/mapfile-vers Fri Dec 08 20:46:40 2017 +0530
+++ b/make/mapfiles/libjava/mapfile-vers Fri Dec 01 16:40:08 2017 -0500
@@ -74,7 +74,7 @@
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;
--- a/src/java.base/share/classes/java/io/FileInputStream.java Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/share/classes/java/io/FileInputStream.java Fri Dec 01 16:40:08 2017 -0500
@@ -25,6 +25,7 @@
package java.io;
+import java.lang.reflect.Method;
import java.nio.channels.FileChannel;
import sun.nio.ch.FileChannelImpl;
@@ -38,6 +39,22 @@
* such as image data. For reading streams of characters, consider using
* <code>FileReader</code>.
*
+ * @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
* @see java.io.FileDescriptor
@@ -63,6 +80,8 @@
private volatile boolean closed;
+ private final AltFinalizer altFinalizer;
+
/**
* Creates a <code>FileInputStream</code> by
* opening a connection to an actual file,
@@ -137,6 +156,10 @@
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 @@
}
fd = fdObj;
path = null;
+ altFinalizer = null;
/*
* FileDescriptor is being shared by streams.
@@ -316,6 +340,14 @@
* <p> 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 @@
private static native void initIDs();
-
static {
initIDs();
}
/**
- * Ensures that the <code>close</code> 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 @@
* @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
+ }
}
}
}
--- a/src/java.base/share/classes/java/io/FileOutputStream.java Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/share/classes/java/io/FileOutputStream.java Fri Dec 01 16:40:08 2017 -0500
@@ -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 @@
* such as image data. For writing streams of characters, consider using
* <code>FileWriter</code>.
*
+ * @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 @@
private volatile boolean closed;
+ private final AltFinalizer altFinalizer;
+
/**
* Creates a file output stream to write to the file with the
* specified name. A new <code>FileDescriptor</code> object is
@@ -218,6 +236,10 @@
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 @@
}
this.fd = fdObj;
this.path = null;
+ this.altFinalizer = null;
fd.attach(this);
}
@@ -340,6 +363,14 @@
* <p> 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 @@
/**
* Cleans up the connection to the file, and ensures that the
- * <code>close</code> 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.
*
- * @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.
+ * @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.
+ *
* @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 @@
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
+ }
+ }
+ }
+
}
--- a/src/java.base/share/classes/java/io/RandomAccessFile.java Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/share/classes/java/io/RandomAccessFile.java Fri Dec 01 16:40:08 2017 -0500
@@ -257,6 +257,7 @@
fd.attach(this);
path = name;
open(name, imode);
+ fd.registerCleanup(); // open sets the fd, register the cleanup
}
/**
--- a/src/java.base/share/classes/java/net/SocketInputStream.java Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/share/classes/java/net/SocketInputStream.java Fri Dec 01 16:40:08 2017 -0500
@@ -283,7 +283,7 @@
/**
* Overrides finalize, the fd is closed by the Socket.
*/
- @SuppressWarnings("deprecation")
+ @SuppressWarnings({"deprecation", "removal"})
protected void finalize() {}
/**
--- a/src/java.base/share/classes/java/net/SocketOutputStream.java Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/share/classes/java/net/SocketOutputStream.java Fri Dec 01 16:40:08 2017 -0500
@@ -175,7 +175,7 @@
/**
* Overrides finalize, the fd is closed by the Socket.
*/
- @SuppressWarnings("deprecation")
+ @SuppressWarnings({"deprecation", "removal"})
protected void finalize() {}
/**
--- a/src/java.base/share/classes/jdk/internal/misc/JavaIOFileDescriptorAccess.java Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/share/classes/jdk/internal/misc/JavaIOFileDescriptorAccess.java Fri Dec 01 16:40:08 2017 -0500
@@ -25,6 +25,7 @@
package jdk.internal.misc;
import java.io.FileDescriptor;
+import java.io.IOException;
/*
* @author Chris Hegarty
@@ -35,7 +36,8 @@
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);
--- a/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java Fri Dec 01 16:40:08 2017 -0500
@@ -27,6 +27,7 @@
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 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 @@
} 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);
}
--- a/src/java.base/unix/classes/java/io/FileDescriptor.java Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/unix/classes/java/io/FileDescriptor.java Fri Dec 01 16:40:08 2017 -0500
@@ -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 @@
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 @@
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();
}
@@ -95,6 +103,11 @@
}
/**
+ * Cleanup in case FileDescriptor is not explicitly closed.
+ */
+ private FDCleanup cleanup;
+
+ /**
* Constructs an (invalid) FileDescriptor
* object.
*/
@@ -178,6 +191,34 @@
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.
*/
private static native boolean getAppend(int fd);
@@ -185,9 +226,30 @@
/**
* 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 @@
}
}
}
+
+ /**
+ * 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<Object> {
+ 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);
+ }
+ }
+ }
}
--- a/src/java.base/unix/native/libjava/FileDescriptor_md.c Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/unix/native/libjava/FileDescriptor_md.c Fri Dec 01 16:40:08 2017 -0500
@@ -71,8 +71,17 @@
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);
}
--- a/src/java.base/unix/native/libjava/io_util_md.c Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/unix/native/libjava/io_util_md.c Fri Dec 01 16:40:08 2017 -0500
@@ -139,6 +139,11 @@
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.
--- a/src/java.base/windows/classes/java/io/FileDescriptor.java Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/windows/classes/java/io/FileDescriptor.java Fri Dec 01 16:40:08 2017 -0500
@@ -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 @@
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) {
@@ -97,6 +105,11 @@
}
/**
+ * Cleanup in case FileDescriptor is not explicitly closed.
+ */
+ private FDCleanup cleanup;
+
+ /**
* Constructs an (invalid) FileDescriptor
* object.
*/
@@ -149,7 +162,7 @@
* 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 @@
/* 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 @@
}
}
}
+
+ /**
+ * 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<Object> {
+ 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);
+ }
+ }
+ }
}
--- a/src/java.base/windows/classes/sun/nio/ch/FileDispatcherImpl.java Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/windows/classes/sun/nio/ch/FileDispatcherImpl.java Fri Dec 01 16:40:08 2017 -0500
@@ -114,6 +114,7 @@
FileDescriptor result = new FileDescriptor();
long handle = duplicateHandle(fdAccess.getHandle(fd));
fdAccess.setHandle(result, handle);
+ fdAccess.registerCleanup(result);
return result;
}
--- a/src/java.base/windows/classes/sun/nio/ch/WindowsAsynchronousFileChannelImpl.java Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/windows/classes/sun/nio/ch/WindowsAsynchronousFileChannelImpl.java Fri Dec 01 16:40:08 2017 -0500
@@ -139,7 +139,7 @@
invalidateAllLocks();
// close the file
- close0(handle);
+ nd.close(fdObj);
// waits until all I/O operations have completed
ioCache.close();
@@ -728,8 +728,6 @@
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();
}
--- a/src/java.base/windows/classes/sun/nio/fs/WindowsChannelFactory.java Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/windows/classes/sun/nio/fs/WindowsChannelFactory.java Fri Dec 01 16:40:08 2017 -0500
@@ -216,8 +216,7 @@
} 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 @@
FileDescriptor fdObj = new FileDescriptor();
fdAccess.setHandle(fdObj, handle);
fdAccess.setAppend(fdObj, flags.append);
+ fdAccess.registerCleanup(fdObj);
return fdObj;
}
}
--- a/src/java.base/windows/native/libjava/FileDescriptor_md.c Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/windows/native/libjava/FileDescriptor_md.c Fri Dec 01 16:40:08 2017 -0500
@@ -57,7 +57,7 @@
}
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 @@
}
}
+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);
}
--- a/src/java.base/windows/native/libnio/ch/WindowsAsynchronousFileChannelImpl.c Fri Dec 08 20:46:40 2017 +0530
+++ b/src/java.base/windows/native/libnio/ch/WindowsAsynchronousFileChannelImpl.c Fri Dec 01 16:40:08 2017 -0500
@@ -121,13 +121,3 @@
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");
- }
-}
--- a/test/jdk/java/io/FileInputStream/FinalizeShdCallClose.java Fri Dec 08 20:46:40 2017 +0530
+++ /dev/null Thu Jan 01 00:00:00 1970 +0000
@@ -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");
- }
-}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/io/FileInputStream/UnreferencedFISClosesFd.java Fri Dec 01 16:40:08 2017 -0500
@@ -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<Object> queue = new ReferenceQueue<>();
+ HashSet<Reference<?>> pending = new HashSet<>();
+ WeakReference<FileInputStream> msWeak = new WeakReference<>(fis, queue);
+ pending.add(msWeak);
+
+ FileDescriptor fd = fis.getFD();
+ WeakReference<FileDescriptor> 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<Object> 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<Object> 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;
+ }
+}
--- a/test/jdk/java/io/FileOutputStream/FinalizeShdCallClose.java Fri Dec 08 20:46:40 2017 +0530
+++ /dev/null Thu Jan 01 00:00:00 1970 +0000
@@ -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");
- }
-}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/io/FileOutputStream/UnreferencedFOSClosesFd.java Fri Dec 01 16:40:08 2017 -0500
@@ -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<Object> queue = new ReferenceQueue<>();
+ HashSet<Reference<?>> pending = new HashSet<>();
+ WeakReference<FileOutputStream> msWeak = new WeakReference<>(fos, queue);
+ pending.add(msWeak);
+
+ FileDescriptor fd = fos.getFD();
+ WeakReference<FileDescriptor> 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<Object> 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<Object> 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;
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/io/RandomAccessFile/UnreferencedRAFClosesFd.java Fri Dec 01 16:40:08 2017 -0500
@@ -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<Object> queue = new ReferenceQueue<>();
+ HashSet<Reference<?>> 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);
+ }
+}
--- a/test/jdk/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java Fri Dec 08 20:46:40 2017 +0530
+++ b/test/jdk/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java Fri Dec 01 16:40:08 2017 -0500
@@ -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 @@
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 @@
}
}
- 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<Object> queue = new ReferenceQueue<>();
+ HashSet<Reference<?>> 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 @@
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 @@
static class TestProxy {
private static final Method getInputStreamMethod;
private static final Method waitForReferenceProcessingMethod;
+ private static final Field inField;
static {
try {
@@ -122,6 +152,9 @@
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 @@
throw new RuntimeException(e);
}
}
+
+ static FileInputStream FilterInputStream_getInField(FilterInputStream fis) {
+ try {
+ return (FileInputStream) inField.get(fis);
+ } catch (IllegalAccessException e) {
+ throw new RuntimeException(e);
+ }
+ }
}
}