8223442: java/nio/channels/SocketChannel/AdaptorStreams.java testConcurrentTimedReadWrite3(): failure

Reviewed-by: michaelm

src/java.base/windows/classes/sun/nio/ch/DatagramDispatcher.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 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
@@ -25,19 +25,16 @@
 
 package sun.nio.ch;
 
-import java.io.*;
-import java.net.*;
+import java.io.FileDescriptor;
+import java.io.IOException;
 
 /**
  * Allows different platforms to call different native methods
  * for read and write operations.
  */
 
-class DatagramDispatcher extends NativeDispatcher
-{
-    static {
-        IOUtil.load();
-    }
+class DatagramDispatcher extends NativeDispatcher {
+    DatagramDispatcher() { }
 
     int read(FileDescriptor fd, long address, int len) throws IOException {
         return read0(fd, address, len);
@@ -56,18 +53,24 @@ class DatagramDispatcher extends NativeDispatcher
     }
 
     void close(FileDescriptor fd) throws IOException {
-        SocketDispatcher.close0(fd);
+        SocketDispatcher.invalidateAndClose(fd);
     }
 
-    static native int read0(FileDescriptor fd, long address, int len)
+    // -- Native methods --
+
+    private static native int read0(FileDescriptor fd, long address, int len)
         throws IOException;
 
-    static native long readv0(FileDescriptor fd, long address, int len)
+    private static native long readv0(FileDescriptor fd, long address, int len)
         throws IOException;
 
-    static native int write0(FileDescriptor fd, long address, int len)
+    private static native int write0(FileDescriptor fd, long address, int len)
         throws IOException;
 
-    static native long writev0(FileDescriptor fd, long address, int len)
+    private static native long writev0(FileDescriptor fd, long address, int len)
         throws IOException;
+
+    static {
+        IOUtil.load();
+    }
 }

src/java.base/windows/classes/sun/nio/ch/SocketDispatcher.java

@@ -28,12 +28,18 @@ package sun.nio.ch;
 import java.io.FileDescriptor;
 import java.io.IOException;
 
+import jdk.internal.access.JavaIOFileDescriptorAccess;
+import jdk.internal.access.SharedSecrets;
+
 /**
  * Allows different platforms to call different native methods
  * for read and write operations.
  */
 
 class SocketDispatcher extends NativeDispatcher {
+    private static final JavaIOFileDescriptorAccess fdAccess =
+            SharedSecrets.getJavaIOFileDescriptorAccess();
+
     SocketDispatcher() { }
 
     int read(FileDescriptor fd, long address, int len) throws IOException {
@@ -53,30 +59,35 @@ class SocketDispatcher extends NativeDispatcher {
     }
 
     void preClose(FileDescriptor fd) throws IOException {
-        preClose0(fd);
+        throw new UnsupportedOperationException();
     }
 
     void close(FileDescriptor fd) throws IOException {
-        close0(fd);
+        invalidateAndClose(fd);
+    }
+
+    static void invalidateAndClose(FileDescriptor fd) throws IOException {
+        assert fd.valid();
+        int fdVal = fdAccess.get(fd);
+        fdAccess.set(fd, -1);
+        close0(fdVal);
     }
 
     // -- Native methods --
 
-    static native int read0(FileDescriptor fd, long address, int len)
+    private static native int read0(FileDescriptor fd, long address, int len)
         throws IOException;
 
-    static native long readv0(FileDescriptor fd, long address, int len)
+    private static native long readv0(FileDescriptor fd, long address, int len)
         throws IOException;
 
-    static native int write0(FileDescriptor fd, long address, int len)
+    private static native int write0(FileDescriptor fd, long address, int len)
         throws IOException;
 
-    static native long writev0(FileDescriptor fd, long address, int len)
+    private static native long writev0(FileDescriptor fd, long address, int len)
         throws IOException;
 
-    static native void preClose0(FileDescriptor fd) throws IOException;
-
-    static native void close0(FileDescriptor fd) throws IOException;
+    private static native void close0(int fdVal) throws IOException;
 
     static {
         IOUtil.load();

src/java.base/windows/native/libnio/ch/Net.c

@@ -620,7 +620,6 @@ Java_sun_nio_ch_Net_poll(JNIEnv* env, jclass this, jobject fdo, jint events, jlo
     int rv;
     int revents = 0;
     struct timeval t;
-    int lastError = 0;
     fd_set rd, wr, ex;
     jint fd = fdval(env, fdo);
 
@@ -643,7 +642,7 @@ Java_sun_nio_ch_Net_poll(JNIEnv* env, jclass this, jobject fdo, jint events, jlo
 
     /* save last winsock error */
     if (rv == SOCKET_ERROR) {
-        handleSocketError(env, lastError);
+        handleSocketError(env, WSAGetLastError());
         return IOS_THROWN;
     } else if (rv >= 0) {
         rv = 0;

src/java.base/windows/native/libnio/ch/SocketDispatcher.c

@@ -280,24 +280,8 @@ Java_sun_nio_ch_SocketDispatcher_writev0(JNIEnv *env, jclass clazz,
 }
 
 JNIEXPORT void JNICALL
-Java_sun_nio_ch_SocketDispatcher_preClose0(JNIEnv *env, jclass clazz,
-                                           jobject fdo)
+Java_sun_nio_ch_SocketDispatcher_close0(JNIEnv *env, jclass clazz, jint fd)
 {
-    jint fd = fdval(env, fdo);
-    struct linger l;
-    int len = sizeof(l);
-    if (getsockopt(fd, SOL_SOCKET, SO_LINGER, (char *)&l, &len) == 0) {
-        if (l.l_onoff == 0) {
-            shutdown(fd, SD_SEND);
-        }
-    }
-}
-
-JNIEXPORT void JNICALL
-Java_sun_nio_ch_SocketDispatcher_close0(JNIEnv *env, jclass clazz,
-                                         jobject fdo)
-{
-    jint fd = fdval(env, fdo);
     if (closesocket(fd) == SOCKET_ERROR) {
         JNU_ThrowIOExceptionWithLastError(env, "Socket close failed");
     }

test/jdk/java/nio/channels/SocketChannel/AdaptorStreams.java

@@ -31,6 +31,8 @@ import java.io.Closeable;
 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.SocketTimeoutException;
@@ -55,7 +57,7 @@ public class AdaptorStreams {
         withConnection((sc, peer) -> {
             peer.getOutputStream().write(99);
             int n = sc.socket().getInputStream().read();
-            assertTrue(n == 99);
+            assertEquals(n, 99);
         });
     }
 
@@ -66,7 +68,7 @@ public class AdaptorStreams {
         withConnection((sc, peer) -> {
             scheduleWrite(peer.getOutputStream(), 99, 1000);
             int n = sc.socket().getInputStream().read();
-            assertTrue(n == 99);
+            assertEquals(n, 99);
         });
     }
 
@@ -77,7 +79,7 @@ public class AdaptorStreams {
         withConnection((sc, peer) -> {
             peer.close();
             int n = sc.socket().getInputStream().read();
-            assertTrue(n == -1);
+            assertEquals(n, -1);
         });
     }
 
@@ -88,7 +90,7 @@ public class AdaptorStreams {
         withConnection((sc, peer) -> {
             scheduleClose(peer, 1000);
             int n = sc.socket().getInputStream().read();
-            assertTrue(n == -1);
+            assertEquals(n, -1);
         });
     }
 
@@ -158,7 +160,7 @@ public class AdaptorStreams {
             Socket s = sc.socket();
             s.setSoTimeout(1000);
             int n = s.getInputStream().read();
-            assertTrue(n == 99);
+            assertEquals(n, 99);
         });
     }
 
@@ -171,7 +173,7 @@ public class AdaptorStreams {
             Socket s = sc.socket();
             s.setSoTimeout(5000);
             int n = s.getInputStream().read();
-            assertTrue(n == 99);
+            assertEquals(n, 99);
         });
     }
 
@@ -326,7 +328,7 @@ public class AdaptorStreams {
             // test read when bytes are available
             peer.getOutputStream().write(99);
             int n = s.getInputStream().read();
-            assertTrue(n == 99);
+            assertEquals(n, 99);
         });
     }
 
@@ -350,7 +352,7 @@ public class AdaptorStreams {
             // test read blocking until bytes are available
             scheduleWrite(peer.getOutputStream(), 99, 500);
             int n = s.getInputStream().read();
-            assertTrue(n == 99);
+            assertEquals(n, 99);
         });
     }
 
@@ -370,7 +372,7 @@ public class AdaptorStreams {
             // test write
             s.getOutputStream().write(99);
             int n = peer.getInputStream().read();
-            assertTrue(n == 99);
+            assertEquals(n, 99);
         });
     }
 
@@ -396,7 +398,7 @@ public class AdaptorStreams {
             peer.getOutputStream().write(99);
             s.setSoTimeout(60*1000);
             int n = s.getInputStream().read();
-            assertTrue(n == 99);
+            assertEquals(n, 99);
         });
     }
 
@@ -421,7 +423,7 @@ public class AdaptorStreams {
             scheduleWrite(peer.getOutputStream(), 99, 500);
             s.setSoTimeout(60*1000);
             int n = s.getInputStream().read();
-            assertTrue(n == 99);
+            assertEquals(n, 99);
         });
     }
 
@@ -442,7 +444,7 @@ public class AdaptorStreams {
             // test write
             s.getOutputStream().write(99);
             int n = peer.getInputStream().read();
-            assertTrue(n == 99);
+            assertEquals(n, 99);
         });
     }
 
@@ -462,10 +464,14 @@ public class AdaptorStreams {
     static void withConnection(ThrowingBiConsumer<SocketChannel, Socket> consumer)
         throws Exception
     {
-        try (ServerSocket ss = new ServerSocket(0);
-             SocketChannel sc = SocketChannel.open(ss.getLocalSocketAddress());
-             Socket peer = ss.accept()) {
-            consumer.accept(sc, peer);
+        var loopback = InetAddress.getLoopbackAddress();
+        try (ServerSocket ss = new ServerSocket()) {
+            ss.bind(new InetSocketAddress(loopback, 0));
+            try (SocketChannel sc = SocketChannel.open(ss.getLocalSocketAddress())) {
+                try (Socket peer = ss.accept()) {
+                    consumer.accept(sc, peer);
+                }
+            }
         }
     }