8212132: (dc) Remove DatagramChannelImpl finalize method
Reviewed-by: bpb, chegar, dfuchs, martin
--- a/src/java.base/share/classes/sun/nio/ch/DatagramChannelImpl.java Fri Oct 25 13:17:31 2019 -0700
+++ b/src/java.base/share/classes/sun/nio/ch/DatagramChannelImpl.java Sun Oct 27 12:13:51 2019 +0000
@@ -27,6 +27,8 @@
import java.io.FileDescriptor;
import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.lang.ref.Cleaner.Cleanable;
import java.net.DatagramSocket;
import java.net.Inet4Address;
import java.net.Inet6Address;
@@ -55,6 +57,7 @@
import java.util.Set;
import java.util.concurrent.locks.ReentrantLock;
+import jdk.internal.ref.CleanerFactory;
import sun.net.ResourceManager;
import sun.net.ext.ExtendedSocketOptions;
import sun.net.util.IPAddressUtil;
@@ -68,7 +71,7 @@
implements SelChImpl
{
// Used to make native read and write calls
- private static NativeDispatcher nd = new DatagramDispatcher();
+ private static final NativeDispatcher nd = new DatagramDispatcher();
// The protocol family of the socket
private final ProtocolFamily family;
@@ -76,6 +79,7 @@
// Our file descriptor
private final FileDescriptor fd;
private final int fdVal;
+ private final Cleanable cleaner;
// Cached InetAddress and port for unconnected DatagramChannels
// used by receive0
@@ -138,6 +142,7 @@
ResourceManager.afterUdpClose();
throw ioe;
}
+ this.cleaner = CleanerFactory.cleaner().register(this, closerFor(fd));
}
public DatagramChannelImpl(SelectorProvider sp, ProtocolFamily family)
@@ -164,6 +169,7 @@
ResourceManager.afterUdpClose();
throw ioe;
}
+ this.cleaner = CleanerFactory.cleaner().register(this, closerFor(fd));
}
public DatagramChannelImpl(SelectorProvider sp, FileDescriptor fd)
@@ -179,6 +185,7 @@
: StandardProtocolFamily.INET;
this.fd = fd;
this.fdVal = IOUtil.fdVal(fd);
+ this.cleaner = CleanerFactory.cleaner().register(this, closerFor(fd));
synchronized (stateLock) {
this.localAddress = Net.localAddress(fd);
}
@@ -1181,10 +1188,10 @@
if ((readerThread == 0) && (writerThread == 0) && !isRegistered()) {
state = ST_CLOSED;
try {
- nd.close(fd);
- } finally {
- // notify resource manager
- ResourceManager.afterUdpClose();
+ // close socket
+ cleaner.clean();
+ } catch (UncheckedIOException ioe) {
+ throw ioe.getCause();
}
return true;
} else {
@@ -1283,13 +1290,6 @@
}
}
- @SuppressWarnings("deprecation")
- protected void finalize() throws IOException {
- // fd is null if constructor threw exception
- if (fd != null)
- close();
- }
-
/**
* Translates native poll revent set into a ready operation set
*/
@@ -1377,6 +1377,21 @@
return fdVal;
}
+ /**
+ * Returns an action to close the given file descriptor.
+ */
+ private static Runnable closerFor(FileDescriptor fd) {
+ return () -> {
+ try {
+ nd.close(fd);
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
+ } finally {
+ // decrement
+ ResourceManager.afterUdpClose();
+ }
+ };
+ }
// -- Native methods --
--- a/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java Fri Oct 25 13:17:31 2019 -0700
+++ b/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java Sun Oct 27 12:13:51 2019 +0000
@@ -30,8 +30,7 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.io.UncheckedIOException;
-import java.lang.invoke.MethodHandles;
-import java.lang.invoke.VarHandle;
+import java.lang.ref.Cleaner.Cleanable;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.ProtocolFamily;
@@ -106,7 +105,7 @@
// set by SocketImpl.create, protected by stateLock
private boolean stream;
- private FileDescriptorCloser closer;
+ private Cleanable cleaner;
// set to true when the socket is in non-blocking mode
private volatile boolean nonBlocking;
@@ -471,9 +470,10 @@
ResourceManager.afterUdpClose();
throw ioe;
}
+ Runnable closer = closerFor(fd, stream);
this.fd = fd;
this.stream = stream;
- this.closer = FileDescriptorCloser.create(this);
+ this.cleaner = CleanerFactory.cleaner().register(this, closer);
this.state = ST_UNCONNECTED;
}
}
@@ -777,10 +777,11 @@
}
// set the fields
+ Runnable closer = closerFor(newfd, true);
synchronized (nsi.stateLock) {
nsi.fd = newfd;
nsi.stream = true;
- nsi.closer = FileDescriptorCloser.create(nsi);
+ nsi.cleaner = CleanerFactory.cleaner().register(nsi, closer);
nsi.localport = localAddress.getPort();
nsi.address = isaa[0].getAddress();
nsi.port = isaa[0].getPort();
@@ -850,7 +851,7 @@
assert Thread.holdsLock(stateLock) && state == ST_CLOSING;
if (readerThread == 0 && writerThread == 0) {
try {
- closer.run();
+ cleaner.clean();
} catch (UncheckedIOException ioe) {
throw ioe.getCause();
} finally {
@@ -1193,53 +1194,28 @@
}
/**
- * A task that closes a SocketImpl's file descriptor. The task runs when the
- * SocketImpl is explicitly closed and when the SocketImpl becomes phantom
- * reachable.
+ * Returns an action to close the given file descriptor.
*/
- private static class FileDescriptorCloser implements Runnable {
- private static final VarHandle CLOSED;
- static {
- try {
- MethodHandles.Lookup l = MethodHandles.lookup();
- CLOSED = l.findVarHandle(FileDescriptorCloser.class,
- "closed",
- boolean.class);
- } catch (Exception e) {
- throw new InternalError(e);
- }
- }
-
- private final FileDescriptor fd;
- private final boolean stream;
- private volatile boolean closed;
-
- FileDescriptorCloser(FileDescriptor fd, boolean stream) {
- this.fd = fd;
- this.stream = stream;
- }
-
- static FileDescriptorCloser create(NioSocketImpl impl) {
- assert Thread.holdsLock(impl.stateLock);
- var closer = new FileDescriptorCloser(impl.fd, impl.stream);
- CleanerFactory.cleaner().register(impl, closer);
- return closer;
- }
-
- @Override
- public void run() {
- if (CLOSED.compareAndSet(this, false, true)) {
+ private static Runnable closerFor(FileDescriptor fd, boolean stream) {
+ if (stream) {
+ return () -> {
+ try {
+ nd.close(fd);
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
+ }
+ };
+ } else {
+ return () -> {
try {
nd.close(fd);
} catch (IOException ioe) {
throw new UncheckedIOException(ioe);
} finally {
- if (!stream) {
- // decrement
- ResourceManager.afterUdpClose();
- }
+ // decrement
+ ResourceManager.afterUdpClose();
}
- }
+ };
}
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/nio/channels/DatagramChannel/Unref.java Sun Oct 27 12:13:51 2019 +0000
@@ -0,0 +1,170 @@
+/*
+ * Copyright (c) 2019, 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 8212132
+ * @summary Test that DatagramChannel does not leak file descriptors
+ * @requires os.family != "windows"
+ * @modules jdk.management
+ * @library /test/lib
+ * @run main/othervm Unref
+ */
+
+import java.io.IOException;
+import java.lang.management.ManagementFactory;
+import java.net.InetSocketAddress;
+import java.net.StandardProtocolFamily;
+import java.nio.channels.DatagramChannel;
+import java.nio.channels.SelectionKey;
+import java.nio.channels.Selector;
+
+import com.sun.management.UnixOperatingSystemMXBean;
+
+import jtreg.SkippedException;
+import jdk.test.lib.net.IPSupport;
+
+public class Unref {
+
+ interface DatagramChannelSupplier {
+ DatagramChannel get() throws IOException;
+ }
+
+ public static void main(String[] args) throws Exception {
+ if (unixOperatingSystemMXBean() == null)
+ throw new SkippedException("This test requires UnixOperatingSystemMXBean");
+
+ test(DatagramChannel::open);
+ if (IPSupport.hasIPv4())
+ test(() -> DatagramChannel.open(StandardProtocolFamily.INET));
+ if (IPSupport.hasIPv6())
+ test(() -> DatagramChannel.open(StandardProtocolFamily.INET6));
+ }
+
+ static void test(DatagramChannelSupplier supplier) throws Exception {
+ openAndClose(supplier); // warm-up
+
+ try (Selector sel = Selector.open()) {
+ long count = fileDescriptorCount();
+
+ // open+close
+ openAndClose(supplier);
+ assertEquals(fileDescriptorCount(), count);
+
+ // open+unref, file descriptor should be closed by cleaner
+ openAndUnref(supplier);
+ assertEquals(waitForFileDescriptorCount(count), count);
+
+ // open+register+close+flush
+ openRegisterAndClose(supplier, sel);
+ assertEquals(fileDescriptorCount(), count);
+
+ // open+register+flush, file descriptor should be closed by cleaner
+ openRegisterAndUnref(supplier, sel);
+ assertEquals(waitForFileDescriptorCount(count), count);
+ }
+ }
+
+ /**
+ * Create a DatagramChannel and closes it.
+ */
+ static void openAndClose(DatagramChannelSupplier supplier) throws IOException {
+ System.out.println("openAndClose ...");
+ DatagramChannel dc = supplier.get();
+ dc.close();
+ }
+
+ /**
+ * Create a DatagramChannel and exits without closing the channel.
+ */
+ static void openAndUnref(DatagramChannelSupplier supplier) throws IOException {
+ System.out.println("openAndUnref ...");
+ DatagramChannel dc = supplier.get();
+ }
+
+ /**
+ * Create a DatagramChannel, register it with a Selector, close the channel
+ * while register, and then finally flush the channel from the Selector.
+ */
+ static void openRegisterAndClose(DatagramChannelSupplier supplier, Selector sel)
+ throws IOException
+ {
+ System.out.println("openRegisterAndClose ...");
+ try (DatagramChannel dc = supplier.get()) {
+ dc.bind(new InetSocketAddress(0));
+ dc.configureBlocking(false);
+ dc.register(sel, SelectionKey.OP_READ);
+ sel.selectNow();
+ }
+
+ // flush, should close channel
+ sel.selectNow();
+ }
+
+ /**
+ * Creates a DatagramChannel, registers with a Selector, cancels the key
+ * and flushes the channel from the Selector. This method exits without
+ * closing the channel.
+ */
+ static void openRegisterAndUnref(DatagramChannelSupplier supplier, Selector sel)
+ throws IOException
+ {
+ System.out.println("openRegisterAndUnref ...");
+ DatagramChannel dc = supplier.get();
+ dc.bind(new InetSocketAddress(0));
+ dc.configureBlocking(false);
+ SelectionKey key = dc.register(sel, SelectionKey.OP_READ);
+ sel.selectNow();
+ key.cancel();
+ sel.selectNow();
+ }
+
+ /**
+ * If the file descriptor count is higher than the given count then invoke
+ * System.gc() and wait for the file descriptor count to drop.
+ */
+ static long waitForFileDescriptorCount(long target) throws InterruptedException {
+ long actual = fileDescriptorCount();
+ if (actual > target) {
+ System.gc();
+ while ((actual = fileDescriptorCount()) > target) {
+ Thread.sleep(10);
+ }
+ }
+ return actual;
+ }
+
+ static UnixOperatingSystemMXBean unixOperatingSystemMXBean() {
+ return ManagementFactory.getPlatformMXBean(UnixOperatingSystemMXBean.class);
+ }
+
+ static long fileDescriptorCount() {
+ return unixOperatingSystemMXBean().getOpenFileDescriptorCount();
+ }
+
+ static void assertEquals(long actual, long expected) {
+ if (actual != expected)
+ throw new RuntimeException("actual=" + actual + ", expected=" + expected);
+ }
+}
+