Request for Review: 5049299: (process) Use posix_spawn, not fork, on S10 to avoid swap exhaustion

[Alan Bateman]

On 10/08/2013 17:16, Rob McKenna wrote:
> Thanks for the build review Erik,
>
> Hi Alan,
>
> Thanks for the review. I'm hoping this webrev has dealt with your 
> comments.
>
> http://cr.openjdk.java.net/~robm/5049299/webrev.06/ 
> <http://cr.openjdk.java.net/%7Erobm/5049299/webrev.06/>
>
> A couple of notes:
>
> - The jexec comments in CompileLauncher referenced the old build too. 
> I figured it was a handy way to find the corresponding makefile in the 
> old build. Removed though.
>
> - I've added a return -1 default case to startChild to avoid a 
> "control reaches end of non-void function" warning on mac & linux. 
> forkAndExec looks for a negative integer specifically, but this should 
> never actually be reached.
>
> - Added the ifdef's around spawnChild and its use to avoid an implicit 
> declaration warning on Linux. (posix_spawn)
>
> I've tested with the attached (quick and very dirty) test. Forgive the 
> System.gc & the sleep, but as you can see from the output below, it 
> looks like we're correctly closing our fd's. (a much longer running 
> version followed the same pattern) Clearly the test is totally 
> unsuitable for integration, but let me know if you can think of a 
> better way to do this.
>
> bash-3.00$ 
> /suspool/home/rob/8/tl/build/solaris-sparcv9-normal-server-release/jdk/bin/java 
> FDLeakTest
> 21360 <--- this is the pid
> 10
> 3010
> 358
> 3358
> 850
> 3850
> 1288
> 4288
> 1636
> 4636
> 10
>
>     -Rob
Thanks for the update.

In UnixProcess.java.XXX then it looks like the statics javaHome and arch 
aren't needed now. You should be able to change LaunchMechanism to 
private static too. One suggestion to consider too is save the byte 
representation of the jspawnhelper path as it shouldn't be necessary to 
have to convert that to bytes each time.  It's okay to do this with 
change-set if you want to get the current patch push.

Otherwise I think this is good, you've sorted out all other issues. Also 
thanks for double checking that we aren't leaking file descriptors.

-Alan