diff --git a/src/hotspot/os/aix/attachListener_aix.cpp b/src/hotspot/os/aix/attachListener_aix.cpp index 982c94a7a1d..ab81becae7c 100644 --- a/src/hotspot/os/aix/attachListener_aix.cpp +++ b/src/hotspot/os/aix/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 (c) 2012, 2018 SAP SE. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -66,10 +66,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; @@ -132,10 +132,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 { @@ -184,7 +184,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()) { @@ -376,10 +375,14 @@ AixAttachOperation* AixAttachListener::dequeue() { // We must prevent accept blocking on the socket if it has been shut down. // Therefore we allow interrupts 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? } @@ -531,9 +534,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::naked_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::naked_yield(); + } } return is_init_trigger(); } diff --git a/src/hotspot/os/bsd/attachListener_bsd.cpp b/src/hotspot/os/bsd/attachListener_bsd.cpp index ba3bd605d4d..c951aee42c4 100644 --- a/src/hotspot/os/bsd/attachListener_bsd.cpp +++ b/src/hotspot/os/bsd/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 @@ -66,7 +66,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; @@ -126,7 +126,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 @@ -502,11 +502,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::naked_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::naked_yield(); + } } return is_init_trigger(); } diff --git a/src/hotspot/os/linux/attachListener_linux.cpp b/src/hotspot/os/linux/attachListener_linux.cpp index a96ee42d77f..aa114d91232 100644 --- a/src/hotspot/os/linux/attachListener_linux.cpp +++ b/src/hotspot/os/linux/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 @@ -67,7 +67,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; @@ -127,7 +127,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 @@ -502,9 +502,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::naked_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::naked_yield(); + } } return is_init_trigger(); } diff --git a/test/hotspot/jtreg/serviceability/attach/RemovingUnixDomainSocketTest.java b/test/hotspot/jtreg/serviceability/attach/RemovingUnixDomainSocketTest.java index 2fd36be8840..6ab5cc0149e 100644 --- a/test/hotspot/jtreg/serviceability/attach/RemovingUnixDomainSocketTest.java +++ b/test/hotspot/jtreg/serviceability/attach/RemovingUnixDomainSocketTest.java @@ -29,33 +29,44 @@ * @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.addVMArgs(Utils.getTestJavaOpts()); 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.stderrShouldBeEmptyIgnoreVMWarnings(); + out.stderrShouldBeEmptyIgnoreVMWarnings() + .stderrShouldBeEmpty(); } public static void main(String... args) throws Exception { @@ -67,10 +78,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 @@ -80,4 +91,7 @@ public static void main(String... args) throws Exception { } } + static void log(Object s) { + System.out.println(String.valueOf(s)); + } } diff --git a/test/lib/jdk/test/lib/apps/LingeredApp.java b/test/lib/jdk/test/lib/apps/LingeredApp.java index 3b75b061007..18b61187af1 100644 --- a/test/lib/jdk/test/lib/apps/LingeredApp.java +++ b/test/lib/jdk/test/lib/apps/LingeredApp.java @@ -36,6 +36,7 @@ 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; @@ -224,7 +225,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) { @@ -270,7 +275,7 @@ public void waitAppReady(long timeout) throws IOException { } /** - * Waits the application to start with the default timeout. + * Waits for the application to start with the default timeout. */ public void waitAppReady() throws IOException { waitAppReady(appWaitTime);