# HG changeset patch # User alanb # Date 1572178431 0 # Node ID 9261ad32cba9b6202debf87c6c1f82428fb43959 # Parent d3382812b7881e025c7676397f7284e392103fa4 8212132: (dc) Remove DatagramChannelImpl finalize method Reviewed-by: bpb, chegar, dfuchs, martin diff -r d3382812b788 -r 9261ad32cba9 src/java.base/share/classes/sun/nio/ch/DatagramChannelImpl.java --- 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 -- diff -r d3382812b788 -r 9261ad32cba9 src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java --- 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(); } - } + }; } } diff -r d3382812b788 -r 9261ad32cba9 test/jdk/java/nio/channels/DatagramChannel/Unref.java --- /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); + } +} +