8235211: serviceability/attach/RemovingUnixDomainSocketTest.java fail…

…s with AttachNotSupportedException: Unable to open socket file

8244973: serviceability/attach/RemovingUnixDomainSocketTest.java fails "stderr was not empty"

Reviewed-by: phh, andrew
Backport-of: 073e095e6053550b17b1daf33df2be4f4c4b40ad

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();
   }

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();
   }

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();
   }

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));
+    }
 }

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) {