Skip to content

Commit

Permalink
8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly
Browse files Browse the repository at this point in the history
PlainSocketImpl.socketAccept() handles EINTR incorrectly

Reviewed-by: phh
Backport-of: 955aee3
  • Loading branch information
Andrei Pangin authored and Paul Hohensee committed Aug 30, 2023
1 parent b77c161 commit d76e6ea
Show file tree
Hide file tree
Showing 8 changed files with 480 additions and 24 deletions.
6 changes: 5 additions & 1 deletion make/common/TestFilesCompilation.gmk
Expand Up @@ -62,7 +62,11 @@ define SetupTestFilesCompilationBody
$1_OUTPUT_SUBDIR := lib
$1_BASE_CFLAGS := $(CFLAGS_JDKLIB)
$1_BASE_CXXFLAGS := $(CXXFLAGS_JDKLIB)
$1_LDFLAGS := $(LDFLAGS_JDKLIB) $(call SET_SHARED_LIBRARY_ORIGIN)
ifeq ($(call isTargetOs, windows), false)
$1_LDFLAGS := $(LDFLAGS_JDKLIB) $(call SET_SHARED_LIBRARY_ORIGIN) -pthread
else
$1_LDFLAGS := $(LDFLAGS_JDKLIB) $(call SET_SHARED_LIBRARY_ORIGIN)
endif
$1_COMPILATION_TYPE := LIBRARY
else ifeq ($$($1_TYPE), PROGRAM)
$1_PREFIX = exe
Expand Down
16 changes: 10 additions & 6 deletions src/java.base/aix/native/libnet/aix_close.c
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2019, SAP SE and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
Expand Down Expand Up @@ -531,12 +531,16 @@ int NET_Timeout(JNIEnv *env, int s, long timeout, jlong nanoTimeStamp) {
* has expired return 0 (indicating timeout expired).
*/
if (rv < 0 && errno == EINTR) {
jlong newNanoTime = JVM_NanoTime(env, 0);
nanoTimeout -= newNanoTime - prevNanoTime;
if (nanoTimeout < NET_NSEC_PER_MSEC) {
return 0;
if (timeout > 0) {
jlong newNanoTime = JVM_NanoTime(env, 0);
nanoTimeout -= newNanoTime - prevNanoTime;
if (nanoTimeout < NET_NSEC_PER_MSEC) {
return 0;
}
prevNanoTime = newNanoTime;
} else {
continue; // timeout is -1, so loop again.
}
prevNanoTime = newNanoTime;
} else {
return rv;
}
Expand Down
16 changes: 10 additions & 6 deletions src/java.base/linux/native/libnet/linux_close.c
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2020, 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
Expand Down Expand Up @@ -437,12 +437,16 @@ int NET_Timeout(JNIEnv *env, int s, long timeout, jlong nanoTimeStamp) {
* has expired return 0 (indicating timeout expired).
*/
if (rv < 0 && errno == EINTR) {
jlong newNanoTime = JVM_NanoTime(env, 0);
nanoTimeout -= newNanoTime - prevNanoTime;
if (nanoTimeout < NET_NSEC_PER_MSEC) {
return 0;
if (timeout > 0) {
jlong newNanoTime = JVM_NanoTime(env, 0);
nanoTimeout -= newNanoTime - prevNanoTime;
if (nanoTimeout < NET_NSEC_PER_MSEC) {
return 0;
}
prevNanoTime = newNanoTime;
} else {
continue; // timeout is -1, so loop again.
}
prevNanoTime = newNanoTime;
} else {
return rv;
}
Expand Down
25 changes: 14 additions & 11 deletions src/java.base/macosx/native/libnet/bsd_close.c
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2020, 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
Expand Down Expand Up @@ -472,17 +472,20 @@ int NET_Timeout(JNIEnv *env, int s, long timeout, jlong nanoTimeStamp) {
* has expired return 0 (indicating timeout expired).
*/
if (rv < 0 && errno == EINTR) {
jlong newNanoTime = JVM_NanoTime(env, 0);
nanoTimeout -= newNanoTime - prevNanoTime;
if (nanoTimeout < NET_NSEC_PER_MSEC) {
if (allocated != 0)
free(fdsp);
return 0;
if (timeout > 0) {
jlong newNanoTime = JVM_NanoTime(env, 0);
nanoTimeout -= newNanoTime - prevNanoTime;
if (nanoTimeout < NET_NSEC_PER_MSEC) {
if (allocated != 0)
free(fdsp);
return 0;
}
prevNanoTime = newNanoTime;
t.tv_sec = nanoTimeout / NET_NSEC_PER_SEC;
t.tv_usec = (nanoTimeout % NET_NSEC_PER_SEC) / NET_NSEC_PER_USEC;
} else {
continue; // timeout is -1, so loop again.
}
prevNanoTime = newNanoTime;
t.tv_sec = nanoTimeout / NET_NSEC_PER_SEC;
t.tv_usec = (nanoTimeout % NET_NSEC_PER_SEC) / NET_NSEC_PER_USEC;

} else {
if (allocated != 0)
free(fdsp);
Expand Down
37 changes: 37 additions & 0 deletions test/jdk/java/net/Socket/NativeThread.java
@@ -0,0 +1,37 @@
/*
* Copyright (c) 2020, 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.
*/

public class NativeThread {

public static final int SIGPIPE;

static {
SIGPIPE = getSIGPIPE();
}

public static native long getID();

public static native int signal(long threadId, int sig);

private static native int getSIGPIPE();
}
153 changes: 153 additions & 0 deletions test/jdk/java/net/Socket/SocketAcceptInterruptTest.java
@@ -0,0 +1,153 @@
/*
* Copyright (c) 2020, 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
* @bug 8237858
* @summary PlainSocketImpl.socketAccept() handles EINTR incorrectly
* @requires (os.family != "windows")
* @compile NativeThread.java
* @run main/othervm/native -Djdk.net.usePlainSocketImpl=true SocketAcceptInterruptTest 0
* @run main/othervm/native -Djdk.net.usePlainSocketImpl=true SocketAcceptInterruptTest 5000
* @run main/othervm/native SocketAcceptInterruptTest 0
* @run main/othervm/native SocketAcceptInterruptTest 5000
*/
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.InetAddress;
import java.net.ServerSocket;
import java.net.*;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;

public class SocketAcceptInterruptTest {

public static void main(String[] args) throws Exception {
System.loadLibrary("NativeThread");
InetAddress loopback = InetAddress.getLoopbackAddress();
ExecutorService executor = Executors.newFixedThreadPool(1);

try ( ServerSocket ss = new ServerSocket(0, 50, loopback);) {
Server server = new Server(ss, Integer.parseInt(args[0]));
Future<Result> future = executor.submit(server);
long threadId = server.getID();

sendSignal(threadId, ss);
sleep(100);
// In failing case server socket will be closed, so we do need to check first
if (!ss.isClosed()) {
// After sending SIGPIPE, create client socket and connect to server
try ( Socket s = new Socket(loopback, ss.getLocalPort()); InputStream in = s.getInputStream();) {
in.read(); // reading one byte is enought for test.
}
}
Result result = future.get();
if (result.status == Result.FAIL) {
throw result.exception;
}
} finally {
executor.shutdown();
}
System.out.println("OK!");
}

private static void sendSignal(long threadId, ServerSocket ss) {
System.out.println("Sending SIGPIPE to ServerSocket thread.");
int count = 0;
while (!ss.isClosed() && count++ < 20) {
sleep(10);
if (NativeThread.signal(threadId, NativeThread.SIGPIPE) != 0) {
throw new RuntimeException("Failed to interrupt the server thread.");
}
}
}

private static void sleep(long time) {
try {
Thread.sleep(time);
} catch (InterruptedException e) {
// ignore the exception.
}
}

static class Server implements Callable<Result> {

private volatile long threadId;
private final ServerSocket serverSocket;
private final int timeout;

public Server(ServerSocket ss, int timeout) {
serverSocket = ss;
this.timeout = timeout;
}

@Override
public Result call() {
try {
threadId = NativeThread.getID();
serverSocket.setSoTimeout(timeout);
try ( Socket socket = serverSocket.accept();
OutputStream outputStream = socket.getOutputStream();) {
outputStream.write("Hello!".getBytes());
return new Result(Result.SUCCESS, null);
}
} catch (IOException e) {
close();
return new Result(Result.FAIL, e);
}
}

long getID() {
while (threadId == 0) {
sleep(5);
}
return threadId;
}

private void close() {
if (!serverSocket.isClosed()) {
try {
serverSocket.close();
} catch (IOException ex) {
// ignore the exception
}
}
}
}

static class Result {

static final int SUCCESS = 0;
static final int FAIL = 1;
final int status;
final Exception exception;

public Result(int status, Exception ex) {
this.status = status;
exception = ex;
}
}
}

1 comment on commit d76e6ea

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.