Request to review JDK-8028094

[Martin Buchholz]

I see this is already submitted.

I thought I could do better than using pkill, but no - there is no
convenient communication from the java process to the grandchild.  This is
a hairy test!

Nevertheless, I would like you to incorporate the following improvements:
- invoke pkill directly
- do some extra checking
- join with reader thread
- don't fail if pkill is not available

--- a/test/java/lang/ProcessBuilder/Basic.java
+++ b/test/java/lang/ProcessBuilder/Basic.java
@@ -2016,7 +2016,7 @@
                 && new File("/bin/bash").exists()
                 && new File("/bin/sleep").exists()) {
                 final String[] cmd = { "/bin/bash", "-c", "(/bin/sleep
6666)" };
-                final String[] cmdkill = { "/bin/bash", "-c",
"(/usr/bin/pkill -f \"sleep 6666\")" };
+                final String[] cmdkill = { "/usr/bin/pkill", "-f", "sleep
6666" };
                 final ProcessBuilder pb = new ProcessBuilder(cmd);
                 final Process p = pb.start();
                 final InputStream stdout = p.getInputStream();
@@ -2044,7 +2044,9 @@
                 stdout.close();
                 stderr.close();
                 stdin.close();
-                new ProcessBuilder(cmdkill).start();
+                // Try to clean up the sleep process, but don't fret about
it.
+                try { new ProcessBuilder(cmdkill).start(); }
+                catch (IOException noPkillCommandInstalled) { }

 //----------------------------------------------------------
                 // There remain unsolved issues with asynchronous close.
                 // Here's a highly non-portable experiment to demonstrate:
@@ -2063,8 +2065,14 @@
                     };
                     new ProcessBuilder(wakeupJeff).start().waitFor();
                     // If wakeupJeff worked, reader probably got EBADF.
-                    reader.join();
                 }
+                long startTime = System.nanoTime();
+                long timeout = 60L * 1000L;
+                reader.join(timeout);
+                long elapsedTimeMillis =
+                    (System.nanoTime() - startTime) / (1024L * 1024L);
+                check(elapsedTimeMillis < timeout);
+                check(!reader.isAlive());
             }
         } catch (Throwable t) { unexpected(t); }




On Tue, Nov 19, 2013 at 4:21 AM, Balchandra Vaidya <
balchandra.vaidya at oracle.com> wrote:

>
> Hi,
>
> Here is one possible solution for the issue JDK-8028094.
>     http://cr.openjdk.java.net/~bvaidya/8/8028094/webrev/
>
> I am not sure pkill is available in all Unix flavor at /usr/bin directory,
> but it is present in Solaris and OEL 6. I have tested on Solaris
> and OEL and "sleep 6666" is no longer left over.
>
>
> Thanks
> Balchandra
>
>
>