8235211: serviceability/attach/RemovingUnixDomainSocketTest.java fails with AttachNotSupportedException: Unable to open socket file

Reviewed-by: sspitsyn, ysuenaga

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) {
+      // avoid deadlock if AttachListener thread is blocked at safepoint
-      os::naked_yield();
+      ThreadBlockInVM tbivm(JavaThread::current());
+      while (AttachListener::transit_state(AL_INITIALIZING,
+                                           AL_NOT_INITIALIZED) != AL_NOT_INITIALIZED) {
+        os::naked_yield();
+      }
     }
     return is_init_trigger();
   }

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,
+      // avoid deadlock if AttachListener thread is blocked at safepoint
-
+      ThreadBlockInVM tbivm(JavaThread::current());
-                                         AL_NOT_INITIALIZED) != AL_NOT_INITIALIZED) {
+      while (AttachListener::transit_state(AL_INITIALIZING,
-      os::naked_yield();
+                                           AL_NOT_INITIALIZED) != AL_NOT_INITIALIZED) {
+        os::naked_yield();
+      }
     }
     return is_init_trigger();
   }

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) {
+      // avoid deadlock if AttachListener thread is blocked at safepoint
-      os::naked_yield();
+      ThreadBlockInVM tbivm(JavaThread::current());
+      while (AttachListener::transit_state(AL_INITIALIZING,
+                                           AL_NOT_INITIALIZED) != AL_NOT_INITIALIZED) {
+        os::naked_yield();
+      }
     }
     return is_init_trigger();
   }

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 = ProcessTools.startProcess("jcmd", new ProcessBuilder(jcmd.getCommand()));
-        Process jcmdProc = pb.start();
 
         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());
+        log("jcmd stdout: [" + out.getStdout() + "];\n" +
-        System.err.println(out.getStderr());
+            "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 class RemovingUnixDomainSocketTest {
             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 class RemovingUnixDomainSocketTest {
         }
     }
 
+    static void log(Object s) {
+        System.out.println(String.valueOf(s));
+    }
 }

test/lib/jdk/test/lib/apps/LingeredApp.java

@@ -36,6 +36,7 @@ import java.util.Collections;
 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 class LingeredApp {
     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 class LingeredApp {
     }
 
     /**
-     * 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);