Skip to content

Commit e5196fc

Browse files
author
Alan Bateman
committed
8358764: (sc) SocketChannel.close when thread blocked in read causes connection to be reset (win)
Reviewed-by: jpai, vyazici
1 parent c98dffa commit e5196fc

File tree

5 files changed

+243
-17
lines changed

5 files changed

+243
-17
lines changed

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,15 @@ static boolean useExclusiveBind() {
9494
return EXCLUSIVE_BIND;
9595
}
9696

97+
private static final StableValue<Boolean> SHUTDOWN_WRITE_BEFORE_CLOSE = StableValue.of();
98+
99+
/**
100+
* Tells whether a TCP connection should be shutdown for writing before closing.
101+
*/
102+
static boolean shouldShutdownWriteBeforeClose() {
103+
return SHUTDOWN_WRITE_BEFORE_CLOSE.orElseSet(Net::shouldShutdownWriteBeforeClose0);
104+
}
105+
97106
/**
98107
* Tells whether both IPV6_XXX and IP_XXX socket options should be set on
99108
* IPv6 sockets. On some kernels, both IPV6_XXX and IP_XXX socket options
@@ -462,6 +471,8 @@ private static boolean isFastTcpLoopbackRequested() {
462471
*/
463472
private static native int isExclusiveBindAvailable();
464473

474+
private static native boolean shouldShutdownWriteBeforeClose0();
475+
465476
private static native boolean shouldSetBothIPv4AndIPv6Options0();
466477

467478
private static native boolean canIPv6SocketJoinIPv4Group0();

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

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -846,7 +846,7 @@ public boolean isConnectionPending() {
846846
/**
847847
* Marks the beginning of a connect operation that might block.
848848
* @param blocking true if configured blocking
849-
* @param isa the remote address
849+
* @param sa the remote socket address
850850
* @throws ClosedChannelException if the channel is closed
851851
* @throws AlreadyConnectedException if already connected
852852
* @throws ConnectionPendingException is a connection is pending
@@ -1070,8 +1070,8 @@ public boolean finishConnect() throws IOException {
10701070
}
10711071

10721072
/**
1073-
* Closes the socket if there are no I/O operations in progress and the
1074-
* channel is not registered with a Selector.
1073+
* Closes the socket if there are no I/O operations in progress (or no I/O
1074+
* operations tracked), and the channel is not registered with a Selector.
10751075
*/
10761076
private boolean tryClose() throws IOException {
10771077
assert Thread.holdsLock(stateLock) && state == ST_CLOSING;
@@ -1096,11 +1096,21 @@ private void tryFinishClose() {
10961096
}
10971097

10981098
/**
1099-
* Closes this channel when configured in blocking mode.
1099+
* Closes this channel when configured in blocking mode. If there are no I/O
1100+
* operations in progress (or tracked), then the channel's socket is closed. If
1101+
* there are I/O operations in progress then the behavior is platform specific.
11001102
*
1101-
* If there is an I/O operation in progress then the socket is pre-closed
1102-
* and the I/O threads signalled, in which case the final close is deferred
1103-
* until all I/O operations complete.
1103+
* On Unix systems, the channel's socket is pre-closed. This unparks any virtual
1104+
* threads that are blocked in I/O operations on this channel. If there are
1105+
* platform threads blocked on the channel's socket then the socket is dup'ed
1106+
* and the platform threads signalled. The final close is deferred until all I/O
1107+
* operations complete.
1108+
*
1109+
* On Windows, the channel's socket is pre-closed. This unparks any virtual
1110+
* threads that are blocked in I/O operations on this channel. If there are no
1111+
* virtual threads blocked in I/O operations on this channel then the channel's
1112+
* socket is closed. If there are virtual threads in I/O then the final close is
1113+
* deferred until all I/O operations on virtual threads complete.
11041114
*
11051115
* Note that a channel configured blocking may be registered with a Selector
11061116
* This arises when a key is canceled and the channel configured to blocking
@@ -1112,17 +1122,17 @@ private void implCloseBlockingMode() throws IOException {
11121122
boolean connected = (state == ST_CONNECTED);
11131123
state = ST_CLOSING;
11141124

1115-
if (!tryClose()) {
1125+
if (connected && Net.shouldShutdownWriteBeforeClose()) {
11161126
// shutdown output when linger interval not set to 0
1117-
if (connected) {
1118-
try {
1119-
var SO_LINGER = StandardSocketOptions.SO_LINGER;
1120-
if ((int) Net.getSocketOption(fd, SO_LINGER) != 0) {
1121-
Net.shutdown(fd, Net.SHUT_WR);
1122-
}
1123-
} catch (IOException ignore) { }
1124-
}
1127+
try {
1128+
var SO_LINGER = StandardSocketOptions.SO_LINGER;
1129+
if ((int) Net.getSocketOption(fd, SO_LINGER) != 0) {
1130+
Net.shutdown(fd, Net.SHUT_WR);
1131+
}
1132+
} catch (IOException ignore) { }
1133+
}
11251134

1135+
if (!tryClose()) {
11261136
// prepare file descriptor for closing
11271137
nd.preClose(fd, readerThread, writerThread);
11281138
}

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

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

208+
JNIEXPORT jboolean JNICALL
209+
Java_sun_nio_ch_Net_shouldShutdownWriteBeforeClose0(JNIEnv *env, jclass clazz) {
210+
return JNI_FALSE;
211+
}
212+
208213
JNIEXPORT jboolean JNICALL
209214
Java_sun_nio_ch_Net_shouldSetBothIPv4AndIPv6Options0(JNIEnv* env, jclass cl)
210215
{

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 implRead
85+
onReach(thread, "sun.nio.ch.SocketChannelImpl.implRead", () -> {
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)