--- a/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java Sat Mar 09 12:54:20 2019 +0000
+++ b/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java Mon Mar 11 19:53:08 2019 +0000
@@ -727,42 +727,6 @@
socketClose0(false);
}
- @Override
- public void copyTo(SocketImpl si) {
- // this SocketImpl should be connected
- assert fd.valid() && localport != 0 && address != null && port != 0;
-
- if (si instanceof AbstractPlainSocketImpl) {
- AbstractPlainSocketImpl psi = (AbstractPlainSocketImpl) si;
- try {
- psi.close();
- } catch (IOException ignore) { }
-
- // copy fields
- psi.stream = this.stream;
- psi.fd = this.fd;
- psi.localport = this.localport;
- psi.address = this.address;
- psi.port = this.port;
-
- // reset fields; do not reset timeout
- psi.closePending = false;
- psi.connectionReset = false;
- psi.shut_rd = false;
- psi.shut_wr = false;
- } else {
- // copy fields
- si.fd = this.fd;
- si.localport = this.localport;
- si.address = this.address;
- si.port = this.port;
- }
-
- // this SocketImpl is now closed and should be discarded
- this.closePending = true;
- this.fd = null;
- }
-
abstract void socketCreate(boolean isServer) throws IOException;
abstract void socketConnect(InetAddress address, int port, int timeout)
throws IOException;
--- a/src/java.base/share/classes/java/net/ServerSocket.java Sat Mar 09 12:54:20 2019 +0000
+++ b/src/java.base/share/classes/java/net/ServerSocket.java Mon Mar 11 19:53:08 2019 +0000
@@ -546,15 +546,7 @@
// Socket has no SocketImpl
if (si == null) {
- // create a platform or custom SocketImpl and accept the connection
- SocketImplFactory factory = Socket.socketImplFactory();
- if (factory == null) {
- si = SocketImpl.createPlatformSocketImpl(false);
- } else {
- si = factory.createSocketImpl();
- }
- implAccept(si);
- // bind Socket to the SocketImpl and update socket state
+ si = implAccept();
s.setImpl(si);
s.postAccept();
return;
@@ -566,18 +558,61 @@
assert si instanceof PlatformSocketImpl;
}
- // ServerSocket or Socket (or both) have, or delegate to, a platform SocketImpl
- if (impl instanceof PlatformSocketImpl || si instanceof PlatformSocketImpl) {
- // create a new platform SocketImpl and accept the connection
- var psi = SocketImpl.createPlatformSocketImpl(false);
- implAccept(psi);
- // copy connection/state to the existing SocketImpl and update socket state
- psi.copyTo(si);
- s.postAccept();
- return;
+ // accept connection with a platform or custom SocketImpl
+ ensureCompatible(si);
+ if (impl instanceof PlatformSocketImpl) {
+ SocketImpl psi = platformImplAccept();
+ tryCopyOptions(si, psi);
+ s.setImpl(psi);
+ close(si);
+ } else {
+ customImplAccept(s, si);
+ assert s.impl == si;
}
+ s.postAccept();
+ }
- // ServerSocket and Socket bound to custom SocketImpls
+ /**
+ * Accepts a connection with a new SocketImpl.
+ * @return the new SocketImpl
+ */
+ private SocketImpl implAccept() throws IOException {
+ if (impl instanceof PlatformSocketImpl) {
+ return platformImplAccept();
+ } else {
+ // custom server SocketImpl, client socket SocketImpl must be set
+ SocketImplFactory factory = Socket.socketImplFactory();
+ if (factory == null) {
+ throw new IOException("An instance of " + impl.getClass() +
+ " cannot accept connection with 'null' SocketImpl:" +
+ " client socket implementation factory not set");
+ }
+ SocketImpl si = factory.createSocketImpl();
+ implAccept(si);
+ return si;
+ }
+ }
+
+ /**
+ * Accepts a connection with a new platform SocketImpl.
+ * @return the new platform SocketImpl
+ */
+ private SocketImpl platformImplAccept() throws IOException {
+ assert impl instanceof PlatformSocketImpl;
+
+ // create a new platform SocketImpl and accept the connection
+ SocketImpl psi = SocketImpl.createPlatformSocketImpl(false);
+ implAccept(psi);
+ return psi;
+ }
+
+ /**
+ * Accepts a new connection with a custom SocketImpl.
+ */
+ private void customImplAccept(Socket s, SocketImpl si) throws IOException {
+ assert !(impl instanceof PlatformSocketImpl)
+ && !(si instanceof PlatformSocketImpl);
+
s.impl = null; // break connection to impl
si.reset();
try {
@@ -588,7 +623,6 @@
} finally {
s.impl = si; // restore connection to impl
}
- s.postAccept();
}
/**
@@ -601,12 +635,6 @@
private void implAccept(SocketImpl si) throws IOException {
assert !(si instanceof DelegatingSocketImpl);
- // A non-platform SocketImpl cannot accept a connection with a platform SocketImpl
- if (!(impl instanceof PlatformSocketImpl) && (si instanceof PlatformSocketImpl)) {
- throw new IOException("An instance of " + impl.getClass() +
- " cannot accept a connection with an instance of " + si.getClass());
- }
-
// custom SocketImpl may expect fd/address objects to be created
if (!(si instanceof PlatformSocketImpl)) {
si.fd = new FileDescriptor();
@@ -616,15 +644,6 @@
// accept a connection
impl.accept(si);
- // sanity check that the fields defined by SocketImpl have been set
- if (si instanceof PlatformSocketImpl) {
- var fd = si.fd;
- if (fd == null || !fd.valid() || si.localport <= 0
- || si.address == null || si.port <= 0) {
- throw new IOException("Invalid accepted state:" + si);
- }
- }
-
// check permission, close SocketImpl/connection if denied
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
@@ -638,6 +657,39 @@
}
/**
+ * Attempts to copy socket options from an old/existing SocketImpl to a new
+ * SocketImpl. At this time, only the SO_TIMEOUT make sense to copy.
+ */
+ private void tryCopyOptions(SocketImpl oldImpl, SocketImpl newImpl) {
+ try {
+ Object timeout = oldImpl.getOption(SocketOptions.SO_TIMEOUT);
+ if (timeout instanceof Integer) {
+ newImpl.setOption(SocketOptions.SO_TIMEOUT, timeout);
+ }
+ } catch (IOException ignore) { }
+ }
+
+ /**
+ * Close the given SocketImpl.
+ */
+ private void close(SocketImpl si) {
+ try {
+ si.close();
+ } catch (IOException ignore) { }
+ }
+
+ /**
+ * Throws IOException if the server SocketImpl and the given client
+ * SocketImpl are not both platform or custom SocketImpls.
+ */
+ private void ensureCompatible(SocketImpl si) throws IOException {
+ if ((impl instanceof PlatformSocketImpl) != (si instanceof PlatformSocketImpl)) {
+ throw new IOException("An instance of " + impl.getClass() +
+ " cannot accept a connection with an instance of " + si.getClass());
+ }
+ }
+
+ /**
* Closes this socket.
*
* Any thread currently blocked in {@link #accept()} will throw
--- a/src/java.base/share/classes/sun/net/PlatformSocketImpl.java Sat Mar 09 12:54:20 2019 +0000
+++ b/src/java.base/share/classes/sun/net/PlatformSocketImpl.java Mon Mar 11 19:53:08 2019 +0000
@@ -24,20 +24,9 @@
*/
package sun.net;
-import java.net.SocketImpl;
-
/**
* Implemented by the platform's SocketImpl implementations.
*/
public interface PlatformSocketImpl {
-
- /**
- * Copy the state from this connected SocketImpl to a target SocketImpl. If
- * the target SocketImpl is not a newly created SocketImpl then it is first
- * closed to release any resources. The target SocketImpl becomes the owner
- * of the file descriptor, this SocketImpl is marked as closed and should
- * be discarded.
- */
- void copyTo(SocketImpl si);
}
--- a/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java Sat Mar 09 12:54:20 2019 +0000
+++ b/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java Mon Mar 11 19:53:08 2019 +0000
@@ -54,7 +54,6 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.ReentrantLock;
-import jdk.internal.access.SharedSecrets;
import jdk.internal.ref.CleanerFactory;
import sun.net.ConnectionResetException;
import sun.net.NetHooks;
@@ -470,77 +469,6 @@
}
/**
- * For use by ServerSocket to copy the state from this connected SocketImpl
- * to a target SocketImpl. If the target SocketImpl is not a newly created
- * SocketImpl then it is first closed to release any resources. The target
- * SocketImpl becomes the owner of the file descriptor, this SocketImpl
- * is marked as closed and should be discarded.
- */
- @Override
- public void copyTo(SocketImpl si) {
- if (si instanceof NioSocketImpl) {
- NioSocketImpl nsi = (NioSocketImpl) si;
- if (nsi.state != ST_NEW) {
- try {
- nsi.close();
- } catch (IOException ignore) { }
- }
- // copy/reset fields protected by stateLock
- synchronized (nsi.stateLock) {
- guarantee(nsi.state == ST_NEW || nsi.state == ST_CLOSED);
- synchronized (this.stateLock) {
- // this SocketImpl should be connected
- guarantee(state == ST_CONNECTED && fd != null && fd.valid()
- && localport > 0 && address != null && port > 0);
-
- // copy fields
- nsi.stream = this.stream;
- nsi.fd = this.fd;
- nsi.localport = this.localport;
- nsi.address = this.address;
- nsi.port = this.port;
-
- // reset fields; do not reset timeout
- nsi.nonBlocking = false;
- nsi.isReuseAddress = false;
- nsi.isInputClosed = false;
- nsi.isOutputClosed = false;
- nsi.state = ST_CONNECTED;
-
- // GC'ing of this impl should not close the file descriptor
- this.closer.disable();
- this.state = ST_CLOSED;
-
- // create new closer to execute when nsi is GC'ed
- nsi.closer = FileDescriptorCloser.create(nsi);
- }
- }
- // reset fields protected by readLock
- nsi.readLock.lock();
- try {
- nsi.readEOF = false;
- nsi.connectionReset = false;
- } finally {
- nsi.readLock.unlock();
- }
- } else {
- synchronized (this.stateLock) {
- // this SocketImpl should be connected
- guarantee(state == ST_CONNECTED && fd != null && fd.valid()
- && localport > 0 && address != null && port > 0);
-
- // set fields in foreign impl
- setSocketImplFields(si, fd, localport, address, port);
-
- // disable closer to prevent GC'ing of this impl from
- // closing the file descriptor
- this.closer.disable();
- this.state = ST_CLOSED;
- }
- }
- }
-
- /**
* Marks the beginning of a connect operation that might block.
* @throws SocketException if the socket is closed or already connected
*/
@@ -1267,10 +1195,6 @@
}
}
}
-
- boolean disable() {
- return CLOSED.compareAndSet(this, false, true);
- }
}
/**
@@ -1314,11 +1238,4 @@
field.setAccessible(true);
field.set(si, value);
}
-
- /**
- * Throws InternalError if the given expression is not true.
- */
- private static void guarantee(boolean expr) {
- if (!expr) throw new InternalError();
- }
}
--- a/test/jdk/java/net/SocketImpl/BadSocketImpls.java Sat Mar 09 12:54:20 2019 +0000
+++ /dev/null Thu Jan 01 00:00:00 1970 +0000
@@ -1,147 +0,0 @@
-/*
- * 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
- * @summary Combinations of bad SocketImpls
- * @run testng/othervm BadSocketImpls
- * @run testng/othervm -Djdk.net.usePlainSocketImpl BadSocketImpls
- */
-
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
-import java.net.InetAddress;
-import java.net.InetSocketAddress;
-import java.net.ServerSocket;
-import java.net.Socket;
-import java.net.SocketAddress;
-import java.net.SocketImpl;
-import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.Test;
-import static java.lang.String.format;
-import static java.lang.System.out;
-import static org.testng.Assert.fail;
-
-public class BadSocketImpls {
-
- Class<IOException> IOE = IOException.class;
-
- /**
- * Tests that a server-side custom socket impl, whose accept is a no-op,
- * does not have any adverse negative effects. Default accept impl.
- */
- @Test
- public void testNoOpAccept() throws IOException {
- ServerSocket ss = new ServerSocket(new NoOpSocketImpl()) { };
- try (ss) {
- ss.bind(new InetSocketAddress(0));
- assertThrows(IOE, ss::accept);
- }
- }
-
- /**
- * Tests that a server-side custom socket impl, whose accept is a no-op,
- * does not have any adverse negative effects. Custom accept, client has
- * an explicit null impl.
- */
- @Test
- public void testNoOpAcceptWithNullClientImpl() throws IOException {
- ServerSocket ss = new ServerSocket(new NoOpSocketImpl()) {
- @Override
- public Socket accept() throws IOException {
- Socket s = new Socket((SocketImpl)null) { };
- implAccept(s);
- return s;
- }
- };
- try (ss) {
- ss.bind(new InetSocketAddress(0));
- assertThrows(IOE, ss::accept);
- }
- }
-
- /**
- * Tests that a server-side custom socket impl, whose accept is a no-op,
- * does not have any adverse negative effects. Custom accept, client has
- * a platform impl.
- */
- @Test
- public void testNoOpAcceptWithPlatformClientImpl() throws IOException {
- ServerSocket ss = new ServerSocket(new NoOpSocketImpl()) {
- @Override
- public Socket accept() throws IOException {
- Socket s = new Socket();
- implAccept(s);
- return s;
- }
- };
- try (ss) {
- ss.bind(new InetSocketAddress(0));
- assertThrows(IOE, ss::accept);
- }
- }
-
- static class NoOpSocketImpl extends SocketImpl {
- @Override protected void create(boolean b) { }
- @Override protected void connect(String s, int i) { }
- @Override protected void connect(InetAddress inetAddress, int i) { }
- @Override protected void connect(SocketAddress socketAddress, int i) { }
- @Override protected void bind(InetAddress inetAddress, int i) { }
- @Override protected void listen(int i) { }
- @Override protected void accept(SocketImpl socket) { }
- @Override protected InputStream getInputStream() { return null; }
- @Override protected OutputStream getOutputStream() { return null; }
- @Override protected int available() { return 0; }
- @Override protected void close() { }
- @Override protected void sendUrgentData(int i) { }
- @Override public void setOption(int i, Object o) { }
- @Override public Object getOption(int i) { return null; }
- }
-
- static <T extends Throwable> void assertThrows(Class<T> throwableClass,
- ThrowingRunnable runnable) {
- try {
- runnable.run();
- fail(format("Expected %s to be thrown, but nothing was thrown",
- throwableClass.getSimpleName()));
- } catch (Throwable t) {
- if (!throwableClass.isInstance(t)) {
- fail(format("Expected %s to be thrown, but %s was thrown",
- throwableClass.getSimpleName(),
- t.getClass().getSimpleName()),
- t);
- }
- out.println("caught expected exception: " + t);
- }
- }
-
- interface ThrowingRunnable {
- void run() throws Throwable;
- }
-
- @BeforeMethod
- public void breakBetweenTests() {
- System.out.println("\n-------\n");
- }
-}
--- a/test/jdk/java/net/SocketImpl/SocketImplCombinations.java Sat Mar 09 12:54:20 2019 +0000
+++ b/test/jdk/java/net/SocketImpl/SocketImplCombinations.java Mon Mar 11 19:53:08 2019 +0000
@@ -315,7 +315,7 @@
assertTrue(isPlatformSocketImpl(getSocketImpl(ss)));
assertTrue(s == socket);
SocketImpl si = getSocketImpl(s);
- assertTrue(si instanceof CustomSocketImpl);
+ assertTrue(isPlatformSocketImpl(si));
checkFields(si);
});
}
@@ -335,9 +335,9 @@
serverSocketAccept(socket, (ss, s) -> {
assertTrue(isPlatformSocketImpl(getSocketImpl(ss)));
assertTrue(s == socket);
- assertTrue(getSocketImpl(s) == si);
- assertTrue(getDelegate(si) == delegate);
- checkFields(delegate);
+ SocketImpl psi = getSocketImpl(socket);
+ assertTrue(isPlatformSocketImpl(psi));
+ checkFields(psi);
});
}
@@ -350,12 +350,11 @@
Socket socket = new Socket(clientImpl) { };
assertTrue(getSocketImpl(socket) == clientImpl);
- serverSocketAccept(socket, (ss, s) -> {
- assertTrue(isPlatformSocketImpl(getSocketImpl(ss)));
- assertTrue(s == socket);
- assertTrue(getSocketImpl(s) == clientImpl);
- checkFields(clientImpl);
- });
+ try (ServerSocket ss = serverSocketToAccept(socket)) {
+ expectThrows(IOException.class, ss::accept);
+ } finally {
+ socket.close();
+ }
}
public void testServerSocketAccept4b() throws IOException {
@@ -363,12 +362,13 @@
Socket socket = new Socket(clientImpl) { };
assertTrue(getSocketImpl(socket) == clientImpl);
- serverSocketAccept(socket, () -> new CustomSocketImpl(false), (ss, s) -> {
- assertTrue(isPlatformSocketImpl(getSocketImpl(ss)));
- assertTrue(s == socket);
- assertTrue(getSocketImpl(s) == clientImpl);
- checkFields(clientImpl);
- });
+ setSocketSocketImplFactory(() -> new CustomSocketImpl(false));
+ try (ServerSocket ss = serverSocketToAccept(socket)) {
+ expectThrows(IOException.class, ss::accept);
+ } finally {
+ setSocketSocketImplFactory(null);
+ socket.close();
+ }
}
/**