Skip to content

Commit e3c50e1

Browse files
committed
8358764: (sc) SocketChannel.close when thread blocked in read causes connection to be reset (win)
Reviewed-by: mdoerr Backport-of: 83f9c250221f707be484e0163fe9040f99474412
1 parent 9bf2571 commit e3c50e1

File tree

5 files changed

+246
-8
lines changed

5 files changed

+246
-8
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 fastLoopback;
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 volatile boolean checkedIPv6;
@@ -106,6 +109,13 @@ static boolean useExclusiveBind() {
106109
return exclusiveBind;
107110
}
108111

112+
/**
113+
* Tells whether a TCP connection should be shutdown for writing before closing.
114+
*/
115+
static boolean shouldShutdownWriteBeforeClose() {
116+
return SHUTDOWN_WRITE_BEFORE_CLOSE;
117+
}
118+
109119
/**
110120
* Tells whether both IPV6_XXX and IP_XXX socket options should be set on
111121
* IPv6 sockets. On some kernels, both IPV6_XXX and IP_XXX socket options
@@ -506,6 +516,8 @@ public static boolean isFastTcpLoopbackRequested() {
506516
*/
507517
private static native int isExclusiveBindAvailable();
508518

519+
private static native boolean shouldShutdownWriteBeforeClose0();
520+
509521
private static native boolean shouldSetBothIPv4AndIPv6Options0();
510522

511523
private static native boolean canIPv6SocketJoinIPv4Group0();
@@ -832,5 +844,6 @@ static native int blockOrUnblock6(boolean block, FileDescriptor fd, byte[] group
832844
}
833845

834846
fastLoopback = isFastTcpLoopbackRequested();
847+
SHUTDOWN_WRITE_BEFORE_CLOSE = shouldShutdownWriteBeforeClose0();
835848
}
836849
}

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

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,7 @@ public boolean isConnectionPending() {
743743
/**
744744
* Marks the beginning of a connect operation that might block.
745745
* @param blocking true if configured blocking
746-
* @param isa the remote address
746+
* @param sa the remote socket address
747747
* @throws ClosedChannelException if the channel is closed
748748
* @throws AlreadyConnectedException if already connected
749749
* @throws ConnectionPendingException is a connection is pending
@@ -970,8 +970,8 @@ public boolean finishConnect() throws IOException {
970970
}
971971

972972
/**
973-
* Closes the socket if there are no I/O operations in progress and the
974-
* channel is not registered with a Selector.
973+
* Closes the socket if there are no I/O operations in progress (or no I/O
974+
* operations tracked), and the channel is not registered with a Selector.
975975
*/
976976
private boolean tryClose() throws IOException {
977977
assert Thread.holdsLock(stateLock) && state == ST_CLOSING;
@@ -996,11 +996,16 @@ private void tryFinishClose() {
996996
}
997997

998998
/**
999-
* Closes this channel when configured in blocking mode.
999+
* Closes this channel when configured in blocking mode. If there are no I/O
1000+
* operations in progress (or tracked), then the channel's socket is closed. If
1001+
* there are I/O operations in progress then the behavior is platform specific.
10001002
*
1001-
* If there is an I/O operation in progress then the socket is pre-closed
1002-
* and the I/O threads signalled, in which case the final close is deferred
1003-
* until all I/O operations complete.
1003+
* On Unix systems, the channel's socket is pre-closed. The socket is dup'ed
1004+
* and the threads signalled. The final close is deferred until all I/O
1005+
* operations complete.
1006+
*
1007+
* On Windows, the channel's socket is pre-closed. The channel's
1008+
* socket is closed.
10041009
*
10051010
* Note that a channel configured blocking may be registered with a Selector
10061011
* This arises when a key is canceled and the channel configured to blocking
@@ -1009,7 +1014,19 @@ private void tryFinishClose() {
10091014
private void implCloseBlockingMode() throws IOException {
10101015
synchronized (stateLock) {
10111016
assert state < ST_CLOSING;
1017+
boolean connected = (state == ST_CONNECTED);
10121018
state = ST_CLOSING;
1019+
1020+
if (connected && Net.shouldShutdownWriteBeforeClose()) {
1021+
// shutdown output when linger interval not set to 0
1022+
try {
1023+
var SO_LINGER = StandardSocketOptions.SO_LINGER;
1024+
if ((int) Net.getSocketOption(fd, SO_LINGER) != 0) {
1025+
Net.shutdown(fd, Net.SHUT_WR);
1026+
}
1027+
} catch (IOException ignore) { }
1028+
}
1029+
10131030
if (!tryClose()) {
10141031
long reader = readerThread;
10151032
long writer = writerThread;

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, 2021, 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: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
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.Executors;
44+
import java.util.concurrent.ThreadFactory;
45+
import java.util.concurrent.atomic.AtomicBoolean;
46+
import java.util.stream.Stream;
47+
48+
import org.junit.jupiter.params.ParameterizedTest;
49+
import org.junit.jupiter.params.provider.MethodSource;
50+
import static org.junit.jupiter.api.Assertions.*;
51+
52+
class PeerReadsAfterAsyncClose {
53+
54+
static Stream<ThreadFactory> factories() {
55+
return Stream.of(Executors.defaultThreadFactory());
56+
}
57+
58+
/**
59+
* Close SocketChannel while a thread is blocked reading from the channel's socket.
60+
*/
61+
@ParameterizedTest
62+
@MethodSource("factories")
63+
void testCloseDuringSocketChannelRead(ThreadFactory factory) throws Exception {
64+
var loopback = InetAddress.getLoopbackAddress();
65+
try (var listener = new ServerSocket()) {
66+
listener.bind(new InetSocketAddress(loopback, 0));
67+
68+
try (SocketChannel sc = SocketChannel.open(listener.getLocalSocketAddress());
69+
Socket peer = listener.accept()) {
70+
71+
// start thread to read from channel
72+
var cceThrown = new AtomicBoolean();
73+
Thread thread = factory.newThread(() -> {
74+
try {
75+
sc.read(ByteBuffer.allocate(1));
76+
fail();
77+
} catch (ClosedChannelException e) {
78+
cceThrown.set(true);
79+
} catch (Throwable e) {
80+
e.printStackTrace();
81+
}
82+
});
83+
thread.start();
84+
try {
85+
// close SocketChannel when thread sampled in read()
86+
onReach(thread, "sun.nio.ch.SocketChannelImpl.read", () -> {
87+
try {
88+
sc.close();
89+
} catch (IOException ignore) { }
90+
});
91+
92+
// peer should read EOF
93+
int n = peer.getInputStream().read();
94+
assertEquals(-1, n);
95+
} finally {
96+
thread.join();
97+
}
98+
assertEquals(true, cceThrown.get(), "ClosedChannelException not thrown");
99+
}
100+
}
101+
}
102+
103+
/**
104+
* Close Socket while a thread is blocked reading from the socket.
105+
*/
106+
@ParameterizedTest
107+
@MethodSource("factories")
108+
void testCloseDuringSocketUntimedRead(ThreadFactory factory) throws Exception {
109+
testCloseDuringSocketRead(factory, 0);
110+
}
111+
112+
/**
113+
* Close Socket while a thread is blocked reading from the socket with a timeout.
114+
*/
115+
@ParameterizedTest
116+
@MethodSource("factories")
117+
void testCloseDuringSockeTimedRead(ThreadFactory factory) throws Exception {
118+
testCloseDuringSocketRead(factory, 60_000);
119+
}
120+
121+
private void testCloseDuringSocketRead(ThreadFactory factory, int timeout) throws Exception {
122+
var loopback = InetAddress.getLoopbackAddress();
123+
try (var listener = new ServerSocket()) {
124+
listener.bind(new InetSocketAddress(loopback, 0));
125+
126+
try (Socket s = new Socket(loopback, listener.getLocalPort());
127+
Socket peer = listener.accept()) {
128+
129+
// start thread to read from socket
130+
var seThrown = new AtomicBoolean();
131+
Thread thread = factory.newThread(() -> {
132+
try {
133+
s.setSoTimeout(timeout);
134+
s.getInputStream().read();
135+
fail();
136+
} catch (SocketException e) {
137+
seThrown.set(true);
138+
} catch (Throwable e) {
139+
e.printStackTrace();
140+
}
141+
});
142+
thread.start();
143+
try {
144+
// close Socket when thread sampled in implRead
145+
onReach(thread, "sun.nio.ch.NioSocketImpl.implRead", () -> {
146+
try {
147+
s.close();
148+
} catch (IOException ignore) { }
149+
});
150+
151+
// peer should read EOF
152+
int n = peer.getInputStream().read();
153+
assertEquals(-1, n);
154+
} finally {
155+
thread.join();
156+
}
157+
assertEquals(true, seThrown.get(), "SocketException not thrown");
158+
}
159+
}
160+
}
161+
162+
/**
163+
* Runs the given action when the given target thread is sampled at the given
164+
* location. The location takes the form "{@code c.m}" where
165+
* {@code c} is the fully qualified class name and {@code m} is the method name.
166+
*/
167+
private void onReach(Thread target, String location, Runnable action) {
168+
int index = location.lastIndexOf('.');
169+
String className = location.substring(0, index);
170+
String methodName = location.substring(index + 1);
171+
Thread tDaemon = new Thread(() -> {
172+
try {
173+
boolean found = false;
174+
while (!found) {
175+
found = contains(target.getStackTrace(), className, methodName);
176+
if (!found) {
177+
Thread.sleep(20);
178+
}
179+
}
180+
action.run();
181+
} catch (Exception e) {
182+
e.printStackTrace();
183+
}
184+
});
185+
tDaemon.setDaemon(true);
186+
tDaemon.start();
187+
}
188+
189+
/**
190+
* Returns true if the given stack trace contains an element for the given class
191+
* and method name.
192+
*/
193+
private boolean contains(StackTraceElement[] stack, String className, String methodName) {
194+
return Arrays.stream(stack)
195+
.anyMatch(e -> className.equals(e.getClassName())
196+
&& methodName.equals(e.getMethodName()));
197+
}
198+
}

0 commit comments

Comments
 (0)