diff --git a/hotspot/src/os/aix/vm/attachListener_aix.cpp b/hotspot/src/os/aix/vm/attachListener_aix.cpp index be53760672a..0d2a97cc77e 100644 --- a/hotspot/src/os/aix/vm/attachListener_aix.cpp +++ b/hotspot/src/os/aix/vm/attachListener_aix.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2020, Oracle and/or its affiliates. All rights reserved. * Copyright 2012, 2013 SAP AG. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -65,10 +65,10 @@ class AixAttachListener: AllStatic { static char _path[UNIX_PATH_MAX]; static bool _has_path; // Shutdown marker to prevent accept blocking during clean-up. - static bool _shutdown; + static volatile bool _shutdown; // the file descriptor for the listening socket - static int _listener; + static volatile int _listener; static bool _atexit_registered; @@ -131,10 +131,10 @@ class AixAttachOperation: public AttachOperation { // statics char AixAttachListener::_path[UNIX_PATH_MAX]; bool AixAttachListener::_has_path; -int AixAttachListener::_listener = -1; +volatile int AixAttachListener::_listener = -1; bool AixAttachListener::_atexit_registered = false; // Shutdown marker to prevent accept blocking during clean-up -bool AixAttachListener::_shutdown = false; +volatile bool AixAttachListener::_shutdown = false; // Supporting class to help split a buffer into individual components class ArgumentIterator : public StackObj { @@ -179,7 +179,6 @@ extern "C" { AixAttachListener::set_shutdown(true); int s = AixAttachListener::listener(); if (s != -1) { - AixAttachListener::set_listener(-1); ::shutdown(s, 2); } if (AixAttachListener::has_path()) { @@ -363,10 +362,14 @@ AixAttachOperation* AixAttachListener::dequeue() { // We must prevent accept blocking on the socket if it has been shut down. // Therefore we allow interrups and check whether we have been shut down already. if (AixAttachListener::is_shutdown()) { + ::close(listener()); + set_listener(-1); return NULL; } - s=::accept(listener(), &addr, &len); + s = ::accept(listener(), &addr, &len); if (s == -1) { + ::close(listener()); + set_listener(-1); return NULL; // log a warning? } @@ -529,9 +532,13 @@ bool AttachListener::check_socket_file() { listener_cleanup(); // wait to terminate current attach listener instance... - while (AttachListener::transit_state(AL_INITIALIZING, - AL_NOT_INITIALIZED) != AL_NOT_INITIALIZED) { - os::yield(); + { + // avoid deadlock if AttachListener thread is blocked at safepoint + ThreadBlockInVM tbivm(JavaThread::current()); + while (AttachListener::transit_state(AL_INITIALIZING, + AL_NOT_INITIALIZED) != AL_NOT_INITIALIZED) { + os::yield(); + } } return is_init_trigger(); } diff --git a/hotspot/src/os/bsd/vm/attachListener_bsd.cpp b/hotspot/src/os/bsd/vm/attachListener_bsd.cpp index 449dd7a9c02..8f8b3872cb9 100644 --- a/hotspot/src/os/bsd/vm/attachListener_bsd.cpp +++ b/hotspot/src/os/bsd/vm/attachListener_bsd.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 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 @@ -65,7 +65,7 @@ class BsdAttachListener: AllStatic { static bool _has_path; // the file descriptor for the listening socket - static int _listener; + static volatile int _listener; static bool _atexit_registered; @@ -125,7 +125,7 @@ class BsdAttachOperation: public AttachOperation { // statics char BsdAttachListener::_path[UNIX_PATH_MAX]; bool BsdAttachListener::_has_path; -int BsdAttachListener::_listener = -1; +volatile int BsdAttachListener::_listener = -1; bool BsdAttachListener::_atexit_registered = false; // Supporting class to help split a buffer into individual components @@ -494,11 +494,13 @@ bool AttachListener::check_socket_file() { listener_cleanup(); // wait to terminate current attach listener instance... - - while (AttachListener::transit_state(AL_INITIALIZING, - - AL_NOT_INITIALIZED) != AL_NOT_INITIALIZED) { - os::yield(); + { + // avoid deadlock if AttachListener thread is blocked at safepoint + ThreadBlockInVM tbivm(JavaThread::current()); + while (AttachListener::transit_state(AL_INITIALIZING, + AL_NOT_INITIALIZED) != AL_NOT_INITIALIZED) { + os::yield(); + } } return is_init_trigger(); } diff --git a/hotspot/src/os/linux/vm/attachListener_linux.cpp b/hotspot/src/os/linux/vm/attachListener_linux.cpp index 3423fb108df..3eea7281cff 100644 --- a/hotspot/src/os/linux/vm/attachListener_linux.cpp +++ b/hotspot/src/os/linux/vm/attachListener_linux.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 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 @@ -65,7 +65,7 @@ class LinuxAttachListener: AllStatic { static bool _has_path; // the file descriptor for the listening socket - static int _listener; + static volatile int _listener; static bool _atexit_registered; @@ -125,7 +125,7 @@ class LinuxAttachOperation: public AttachOperation { // statics char LinuxAttachListener::_path[UNIX_PATH_MAX]; bool LinuxAttachListener::_has_path; -int LinuxAttachListener::_listener = -1; +volatile int LinuxAttachListener::_listener = -1; bool LinuxAttachListener::_atexit_registered = false; // Supporting class to help split a buffer into individual components @@ -489,9 +489,13 @@ bool AttachListener::check_socket_file() { listener_cleanup(); // wait to terminate current attach listener instance... - while (AttachListener::transit_state(AL_INITIALIZING, - AL_NOT_INITIALIZED) != AL_NOT_INITIALIZED) { - os::yield(); + { + // avoid deadlock if AttachListener thread is blocked at safepoint + ThreadBlockInVM tbivm(JavaThread::current()); + while (AttachListener::transit_state(AL_INITIALIZING, + AL_NOT_INITIALIZED) != AL_NOT_INITIALIZED) { + os::yield(); + } } return is_init_trigger(); } diff --git a/hotspot/test/serviceability/attach/RemovingUnixDomainSocketTest.java b/hotspot/test/serviceability/attach/RemovingUnixDomainSocketTest.java index 0330f52f7b8..d9e158db190 100644 --- a/hotspot/test/serviceability/attach/RemovingUnixDomainSocketTest.java +++ b/hotspot/test/serviceability/attach/RemovingUnixDomainSocketTest.java @@ -28,31 +28,43 @@ * @run main RemovingUnixDomainSocketTest */ +import java.io.File; import java.io.IOException; import java.nio.file.Path; +import java.util.concurrent.TimeUnit; +import jdk.test.lib.Utils; import jdk.test.lib.apps.LingeredApp; import jdk.test.lib.JDKToolLauncher; import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.process.ProcessTools; public class RemovingUnixDomainSocketTest { + // timeout (in seconds) + private static final long timeout = Utils.adjustTimeout(60); + private static void runJCmd(long pid) throws InterruptedException, IOException { JDKToolLauncher jcmd = JDKToolLauncher.createUsingTestJDK("jcmd"); jcmd.addToolArg(Long.toString(pid)); jcmd.addToolArg("VM.version"); - ProcessBuilder pb = new ProcessBuilder(jcmd.getCommand()); - Process jcmdProc = pb.start(); + Process jcmdProc = ProcessTools.startProcess("jcmd", new ProcessBuilder(jcmd.getCommand())); OutputAnalyzer out = new OutputAnalyzer(jcmdProc); - jcmdProc.waitFor(); + if (!jcmdProc.waitFor(timeout, TimeUnit.SECONDS)) { + log("jcmd is still running after " + timeout + " seconds, terminating..."); + jcmdProc.destroy(); + jcmdProc.waitFor(); + } - System.out.println(out.getStdout()); - System.err.println(out.getStderr()); + log("jcmd stdout: [" + out.getStdout() + "];\n" + + "jcmd stderr: [" + out.getStderr() + "]\n" + + "jcmd exitValue = " + out.getExitValue()); - out.stderrShouldBeEmpty(); + out.shouldHaveExitValue(0) + .stderrShouldBeEmptyIgnoreVMWarnings(); } public static void main(String... args) throws Exception { @@ -64,10 +76,10 @@ public static void main(String... args) throws Exception { runJCmd(app.getPid()); // Remove unix domain socket file - var sockFile = Path.of(System.getProperty("java.io.tmpdir"), + File sockFile = Path.of(System.getProperty("java.io.tmpdir"), ".java_pid" + app.getPid()) .toFile(); - System.out.println("Remove " + sockFile.toString()); + log("Remove " + sockFile.toString()); sockFile.delete(); // Access to Attach Listener again @@ -77,4 +89,7 @@ public static void main(String... args) throws Exception { } } + static void log(Object s) { + System.out.println(String.valueOf(s)); + } } diff --git a/jdk/test/lib/jdk/test/lib/apps/LingeredApp.java b/jdk/test/lib/jdk/test/lib/apps/LingeredApp.java index 7bff8b5c60f..0070dcfd717 100644 --- a/jdk/test/lib/jdk/test/lib/apps/LingeredApp.java +++ b/jdk/test/lib/jdk/test/lib/apps/LingeredApp.java @@ -40,8 +40,11 @@ import java.util.Date; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import java.util.UUID; + +import jdk.test.lib.Utils; import jdk.test.lib.process.OutputBuffer; import jdk.test.lib.process.ProcessTools; import jdk.test.lib.process.StreamPumper; @@ -230,7 +233,11 @@ public void deleteLock() throws IOException { public void waitAppTerminate() { // This code is modeled after tail end of ProcessTools.getOutput(). try { - appProcess.waitFor(); + // If the app hangs, we don't want to wait for the to test timeout. + if (!appProcess.waitFor(Utils.adjustTimeout(appWaitTime), TimeUnit.SECONDS)) { + appProcess.destroy(); + appProcess.waitFor(); + } outPumperThread.join(); errPumperThread.join(); } catch (InterruptedException e) {