Skip to content

Commit 83f9c25

Browse files
committed
8358764: (sc) SocketChannel.close when thread blocked in read causes connection to be reset (win)
Reviewed-by: stuefe Backport-of: e5196fc24d2ec9e581af7803ac47036111fee029
1 parent 413fa29 commit 83f9c25

File tree

5 files changed

+245
-17
lines changed

5 files changed

+245
-17
lines changed

src/java.base/share/classes/sun/nio/ch/Net.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ public String name() {
7070
// set to true if the fast tcp loopback should be enabled on Windows
7171
private static final boolean FAST_LOOPBACK;
7272

73+
// set to true if shut down before close should be enabled on Windows
74+
private static final boolean SHUTDOWN_WRITE_BEFORE_CLOSE;
75+
7376
// -- Miscellaneous utilities --
7477

7578
private static final boolean IPV6_AVAILABLE;
@@ -96,6 +99,13 @@ static boolean useExclusiveBind() {
9699
return EXCLUSIVE_BIND;
97100
}
98101

102+
/**
103+
* Tells whether a TCP connection should be shutdown for writing before closing.
104+
*/
105+
static boolean shouldShutdownWriteBeforeClose() {
106+
return SHUTDOWN_WRITE_BEFORE_CLOSE;
107+
}
108+
99109
/**
100110
* Tells whether both IPV6_XXX and IP_XXX socket options should be set on
101111
* IPv6 sockets. On some kernels, both IPV6_XXX and IP_XXX socket options
@@ -516,6 +526,8 @@ private static boolean isFastTcpLoopbackRequested() {
516526
*/
517527
private static native int isExclusiveBindAvailable();
518528

529+
private static native boolean shouldShutdownWriteBeforeClose0();
530+
519531
private static native boolean shouldSetBothIPv4AndIPv6Options0();
520532

521533
private static native boolean canIPv6SocketJoinIPv4Group0();
@@ -842,6 +854,7 @@ static native int blockOrUnblock6(boolean block, FileDescriptor fd, byte[] group
842854

843855
IPV6_AVAILABLE = isIPv6Available0();
844856
SO_REUSEPORT_AVAILABLE = isReusePortAvailable0();
857+
SHUTDOWN_WRITE_BEFORE_CLOSE = shouldShutdownWriteBeforeClose0();
845858
}
846859

847860
private static AssertionError shouldNotReachHere() {

src/java.base/share/classes/sun/nio/ch/SocketChannelImpl.java

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,7 @@ public boolean isConnectionPending() {
769769
/**
770770
* Marks the beginning of a connect operation that might block.
771771
* @param blocking true if configured blocking
772-
* @param isa the remote address
772+
* @param sa the remote socket address
773773
* @throws ClosedChannelException if the channel is closed
774774
* @throws AlreadyConnectedException if already connected
775775
* @throws ConnectionPendingException is a connection is pending
@@ -997,8 +997,8 @@ public boolean finishConnect() throws IOException {
997997
}
998998

999999
/**
1000-
* Closes the socket if there are no I/O operations in progress and the
1001-
* channel is not registered with a Selector.
1000+
* Closes the socket if there are no I/O operations in progress (or no I/O
1001+
* operations tracked), and the channel is not registered with a Selector.
10021002
*/
10031003
private boolean tryClose() throws IOException {
10041004
assert Thread.holdsLock(stateLock) && state == ST_CLOSING;
@@ -1023,11 +1023,21 @@ private void tryFinishClose() {
10231023
}
10241024

10251025
/**
1026-
* Closes this channel when configured in blocking mode.
1026+
* Closes this channel when configured in blocking mode. If there are no I/O
1027+
* operations in progress (or tracked), then the channel's socket is closed. If
1028+
* there are I/O operations in progress then the behavior is platform specific.
10271029
*
1028-
* If there is an I/O operation in progress then the socket is pre-closed
1029-
* and the I/O threads signalled, in which case the final close is deferred
1030-
* until all I/O operations complete.
1030+
* On Unix systems, the channel's socket is pre-closed. This unparks any virtual
1031+
* threads that are blocked in I/O operations on this channel. If there are
1032+
* platform threads blocked on the channel's socket then the socket is dup'ed
1033+
* and the platform threads signalled. The final close is deferred until all I/O
1034+
* operations complete.
1035+
*
1036+
* On Windows, the channel's socket is pre-closed. This unparks any virtual
1037+
* threads that are blocked in I/O operations on this channel. If there are no
1038+
* virtual threads blocked in I/O operations on this channel then the channel's
1039+
* socket is closed. If there are virtual threads in I/O then the final close is
1040+
* deferred until all I/O operations on virtual threads complete.
10311041
*
10321042
* Note that a channel configured blocking may be registered with a Selector
10331043
* This arises when a key is canceled and the channel configured to blocking
@@ -1039,17 +1049,17 @@ private void implCloseBlockingMode() throws IOException {
10391049
boolean connected = (state == ST_CONNECTED);
10401050
state = ST_CLOSING;
10411051

1042-
if (!tryClose()) {
1052+
if (connected && Net.shouldShutdownWriteBeforeClose()) {
10431053
// shutdown output when linger interval not set to 0
1044-
if (connected) {
1045-
try {
1046-
var SO_LINGER = StandardSocketOptions.SO_LINGER;
1047-
if ((int) Net.getSocketOption(fd, SO_LINGER) != 0) {
1048-
Net.shutdown(fd, Net.SHUT_WR);
1049-
}
1050-
} catch (IOException ignore) { }
1051-
}
1054+
try {
1055+
var SO_LINGER = StandardSocketOptions.SO_LINGER;
1056+
if ((int) Net.getSocketOption(fd, SO_LINGER) != 0) {
1057+
Net.shutdown(fd, Net.SHUT_WR);
1058+
}
1059+
} catch (IOException ignore) { }
1060+
}
10521061

1062+
if (!tryClose()) {
10531063
long reader = readerThread;
10541064
long writer = writerThread;
10551065
if (NativeThread.isVirtualThread(reader)

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,11 @@ Java_sun_nio_ch_Net_isExclusiveBindAvailable(JNIEnv *env, jclass clazz) {
200200
return -1;
201201
}
202202

203+
JNIEXPORT jboolean JNICALL
204+
Java_sun_nio_ch_Net_shouldShutdownWriteBeforeClose0(JNIEnv *env, jclass clazz) {
205+
return JNI_FALSE;
206+
}
207+
203208
JNIEXPORT jboolean JNICALL
204209
Java_sun_nio_ch_Net_shouldSetBothIPv4AndIPv6Options0(JNIEnv* env, jclass cl)
205210
{

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2001, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2001, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -117,6 +117,11 @@ Java_sun_nio_ch_Net_isExclusiveBindAvailable(JNIEnv *env, jclass clazz) {
117117
return 1;
118118
}
119119

120+
JNIEXPORT jboolean JNICALL
121+
Java_sun_nio_ch_Net_shouldShutdownWriteBeforeClose0(JNIEnv *env, jclass clazz) {
122+
return JNI_TRUE;
123+
}
124+
120125
JNIEXPORT jboolean JNICALL
121126
Java_sun_nio_ch_Net_shouldSetBothIPv4AndIPv6Options0(JNIEnv* env, jclass cl)
122127
{
Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
/*
2+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/*
25+
* @test
26+
* @bug 8358764
27+
* @summary Test closing a socket while a thread is blocked in read. The connection
28+
* should be closed gracefuly so that the peer reads EOF.
29+
* @run junit PeerReadsAfterAsyncClose
30+
*/
31+
32+
import java.io.IOException;
33+
import java.net.InetAddress;
34+
import java.net.InetSocketAddress;
35+
import java.net.ServerSocket;
36+
import java.net.Socket;
37+
import java.net.SocketException;
38+
import java.nio.ByteBuffer;
39+
import java.nio.channels.ClosedChannelException;
40+
import java.nio.channels.SocketChannel;
41+
import java.util.Arrays;
42+
import java.util.Objects;
43+
import java.util.concurrent.ThreadFactory;
44+
import java.util.concurrent.atomic.AtomicBoolean;
45+
import java.util.stream.Stream;
46+
47+
import org.junit.jupiter.params.ParameterizedTest;
48+
import org.junit.jupiter.params.provider.MethodSource;
49+
import static org.junit.jupiter.api.Assertions.*;
50+
51+
class PeerReadsAfterAsyncClose {
52+
53+
static Stream<ThreadFactory> factories() {
54+
return Stream.of(Thread.ofPlatform().factory(), Thread.ofVirtual().factory());
55+
}
56+
57+
/**
58+
* Close SocketChannel while a thread is blocked reading from the channel's socket.
59+
*/
60+
@ParameterizedTest
61+
@MethodSource("factories")
62+
void testCloseDuringSocketChannelRead(ThreadFactory factory) throws Exception {
63+
var loopback = InetAddress.getLoopbackAddress();
64+
try (var listener = new ServerSocket()) {
65+
listener.bind(new InetSocketAddress(loopback, 0));
66+
67+
try (SocketChannel sc = SocketChannel.open(listener.getLocalSocketAddress());
68+
Socket peer = listener.accept()) {
69+
70+
// start thread to read from channel
71+
var cceThrown = new AtomicBoolean();
72+
Thread thread = factory.newThread(() -> {
73+
try {
74+
sc.read(ByteBuffer.allocate(1));
75+
fail();
76+
} catch (ClosedChannelException e) {
77+
cceThrown.set(true);
78+
} catch (Throwable e) {
79+
e.printStackTrace();
80+
}
81+
});
82+
thread.start();
83+
try {
84+
// close SocketChannel when thread sampled in read()
85+
onReach(thread, "sun.nio.ch.SocketChannelImpl.read", () -> {
86+
try {
87+
sc.close();
88+
} catch (IOException ignore) { }
89+
});
90+
91+
// peer should read EOF
92+
int n = peer.getInputStream().read();
93+
assertEquals(-1, n);
94+
} finally {
95+
thread.join();
96+
}
97+
assertEquals(true, cceThrown.get(), "ClosedChannelException not thrown");
98+
}
99+
}
100+
}
101+
102+
/**
103+
* Close Socket while a thread is blocked reading from the socket.
104+
*/
105+
@ParameterizedTest
106+
@MethodSource("factories")
107+
void testCloseDuringSocketUntimedRead(ThreadFactory factory) throws Exception {
108+
testCloseDuringSocketRead(factory, 0);
109+
}
110+
111+
/**
112+
* Close Socket while a thread is blocked reading from the socket with a timeout.
113+
*/
114+
@ParameterizedTest
115+
@MethodSource("factories")
116+
void testCloseDuringSockeTimedRead(ThreadFactory factory) throws Exception {
117+
testCloseDuringSocketRead(factory, 60_000);
118+
}
119+
120+
private void testCloseDuringSocketRead(ThreadFactory factory, int timeout) throws Exception {
121+
var loopback = InetAddress.getLoopbackAddress();
122+
try (var listener = new ServerSocket()) {
123+
listener.bind(new InetSocketAddress(loopback, 0));
124+
125+
try (Socket s = new Socket(loopback, listener.getLocalPort());
126+
Socket peer = listener.accept()) {
127+
128+
// start thread to read from socket
129+
var seThrown = new AtomicBoolean();
130+
Thread thread = factory.newThread(() -> {
131+
try {
132+
s.setSoTimeout(timeout);
133+
s.getInputStream().read();
134+
fail();
135+
} catch (SocketException e) {
136+
seThrown.set(true);
137+
} catch (Throwable e) {
138+
e.printStackTrace();
139+
}
140+
});
141+
thread.start();
142+
try {
143+
// close Socket when thread sampled in implRead
144+
onReach(thread, "sun.nio.ch.NioSocketImpl.implRead", () -> {
145+
try {
146+
s.close();
147+
} catch (IOException ignore) { }
148+
});
149+
150+
// peer should read EOF
151+
int n = peer.getInputStream().read();
152+
assertEquals(-1, n);
153+
} finally {
154+
thread.join();
155+
}
156+
assertEquals(true, seThrown.get(), "SocketException not thrown");
157+
}
158+
}
159+
}
160+
161+
/**
162+
* Runs the given action when the given target thread is sampled at the given
163+
* location. The location takes the form "{@code c.m}" where
164+
* {@code c} is the fully qualified class name and {@code m} is the method name.
165+
*/
166+
private void onReach(Thread target, String location, Runnable action) {
167+
int index = location.lastIndexOf('.');
168+
String className = location.substring(0, index);
169+
String methodName = location.substring(index + 1);
170+
Thread.ofPlatform().daemon(true).start(() -> {
171+
try {
172+
boolean found = false;
173+
while (!found) {
174+
found = contains(target.getStackTrace(), className, methodName);
175+
if (!found) {
176+
Thread.sleep(20);
177+
}
178+
}
179+
action.run();
180+
} catch (Exception e) {
181+
e.printStackTrace();
182+
}
183+
});
184+
}
185+
186+
/**
187+
* Returns true if the given stack trace contains an element for the given class
188+
* and method name.
189+
*/
190+
private boolean contains(StackTraceElement[] stack, String className, String methodName) {
191+
return Arrays.stream(stack)
192+
.anyMatch(e -> className.equals(e.getClassName())
193+
&& methodName.equals(e.getMethodName()));
194+
}
195+
}

0 commit comments

Comments
 (0)