6981138: (process) Process.waitFor() may hang if subprocess has live descendants (lnx)
authormartin
Fri, 17 Sep 2010 14:35:00 -0700
changeset 6669 8f8d4d5768ae
parent 6668 bf6309ced0b6
child 6670 ae13809f3ce7
6981138: (process) Process.waitFor() may hang if subprocess has live descendants (lnx) Summary: Do exit status handling before trying to close streams Reviewed-by: alanb, dholmes
jdk/src/solaris/classes/java/lang/UNIXProcess.java.linux
jdk/test/java/lang/ProcessBuilder/Basic.java
--- a/jdk/src/solaris/classes/java/lang/UNIXProcess.java.linux	Fri Sep 17 14:16:14 2010 -0700
+++ b/jdk/src/solaris/classes/java/lang/UNIXProcess.java.linux	Fri Sep 17 14:35:00 2010 -0700
@@ -176,7 +176,13 @@
             }});
     }
 
-    synchronized void processExited(int exitcode) {
+    void processExited(int exitcode) {
+        synchronized (this) {
+            this.exitcode = exitcode;
+            hasExited = true;
+            notifyAll();
+        }
+
         if (stdout instanceof ProcessPipeInputStream)
             ((ProcessPipeInputStream) stdout).processExited();
 
@@ -185,10 +191,6 @@
 
         if (stdin instanceof ProcessPipeOutputStream)
             ((ProcessPipeOutputStream) stdin).processExited();
-
-        this.exitcode = exitcode;
-        hasExited = true;
-        notifyAll();
     }
 
     public OutputStream getOutputStream() {
--- a/jdk/test/java/lang/ProcessBuilder/Basic.java	Fri Sep 17 14:16:14 2010 -0700
+++ b/jdk/test/java/lang/ProcessBuilder/Basic.java	Fri Sep 17 14:35:00 2010 -0700
@@ -1825,6 +1825,64 @@
         } catch (Throwable t) { unexpected(t); }
 
         //----------------------------------------------------------------
+        // Check that subprocesses which create subprocesses of their
+        // own do not cause parent to hang waiting for file
+        // descriptors to be closed.
+        //----------------------------------------------------------------
+        try {
+            if (Unix.is()
+                && new File("/bin/bash").exists()
+                && new File("/bin/sleep").exists()) {
+                final String[] cmd = { "/bin/bash", "-c", "(/bin/sleep 6666)" };
+                final ProcessBuilder pb = new ProcessBuilder(cmd);
+                final Process p = pb.start();
+                final InputStream stdout = p.getInputStream();
+                final InputStream stderr = p.getErrorStream();
+                final OutputStream stdin = p.getOutputStream();
+                final Thread reader = new Thread() {
+                    public void run() {
+                        try { stdout.read(); }
+                        catch (IOException e) {
+                            // e.printStackTrace();
+                            if (EnglishUnix.is() &&
+                                ! (e.getMessage().matches(".*Bad file descriptor.*")))
+                                unexpected(e);
+                        }
+                        catch (Throwable t) { unexpected(t); }}};
+                reader.setDaemon(true);
+                reader.start();
+                Thread.sleep(100);
+                p.destroy();
+                // Subprocess is now dead, but file descriptors remain open.
+                check(p.waitFor() != 0);
+                check(p.exitValue() != 0);
+                stdout.close();
+                stderr.close();
+                stdin.close();
+                //----------------------------------------------------------
+                // There remain unsolved issues with asynchronous close.
+                // Here's a highly non-portable experiment to demonstrate:
+                //----------------------------------------------------------
+                if (Boolean.getBoolean("wakeupJeff!")) {
+                    System.out.println("wakeupJeff!");
+                    // Initialize signal handler for INTERRUPT_SIGNAL.
+                    new FileInputStream("/bin/sleep").getChannel().close();
+                    // Send INTERRUPT_SIGNAL to every thread in this java.
+                    String[] wakeupJeff = {
+                        "/bin/bash", "-c",
+                        "/bin/ps --noheaders -Lfp $PPID | " +
+                        "/usr/bin/perl -nale 'print $F[3]' | " +
+                        // INTERRUPT_SIGNAL == 62 on my machine du jour.
+                        "/usr/bin/xargs kill -62"
+                    };
+                    new ProcessBuilder(wakeupJeff).start().waitFor();
+                    // If wakeupJeff worked, reader probably got EBADF.
+                    reader.join();
+                }
+            }
+        } catch (Throwable t) { unexpected(t); }
+
+        //----------------------------------------------------------------
         // Attempt to start process with insufficient permissions fails.
         //----------------------------------------------------------------
         try {