Request to review JDK-8028094

[Balchandra Vaidya]

Thanks Martin.  I will incorporate your suggested improvements, and
will send out another webrev.

Regards
Balchandra

On 19/11/2013 22:53, Martin Buchholz wrote:
> 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 <mailto: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/
>     <http://cr.openjdk.java.net/%7Ebvaidya/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
>
>
>