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

[Alan Bateman]

On 22/11/2012 21:27, Rob McKenna wrote:
> Hi folks,
>
> Looking for a review for the webrev below, which also resolves:
>
> 7175692: (process) Process.exec should use posix_spawn [macosx]
>
> For additional context and a brief description it would be well worth 
> looking at the following thread started by Michael McMahon, who did 
> the brunt of the work for this fix:
>
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-May/thread.html#1644 
>
>
> Basically the fix aims to swap fork for posix_spawn as the default 
> process launch mechanism on Solaris and Mac OSX in order to avoid swap 
> exhaustion issues encountered with fork()/exec(). It also offers a 
> flag (java.lang.useFork) to allow a user to revert to the old behaviour.
>
> I'm having trouble seeing the wood for the trees at this point so I'm 
> anticipating plenty of feedback. In particular I'd appreciate some 
> discussion on:
>
> - The binary launcher name & property name may need some work. A more 
> general property ("java.lang.launchMechanism") to allow a user to 
> specify a particular call was mooted too. It may be more future proof 
> and I'm completely open to that. (e.g. 
> launchMechanism=spawn|fork|vfork|clone - we would obviously ignore 
> inapplicable values on a per-platform basis)
> - I'd like a more robust way of checking that someone isn't trying to 
> use jprochelper outside of the context for which it is meant.
> - The decision around which call to use getting moved to the java 
> level and away from the native preprocessor.
>
> The webrev is at:
>
> http://cr.openjdk.java.net/~robm/5049299/webrev.01/ 
> <http://cr.openjdk.java.net/%7Erobm/5049299/webrev.01/>
It's great to get this one moving again, we should have helped Michael 
more to get this over the line on the first outing.

This one will require very careful review, I don't have cycles this 
week, I hope others do. For now I think that naming the trampoline 
jprochelper or jspawnhelper is okay. To make it more robust then I'd 
probably prepend a magic number or some token. Also I'd probably fstat 
stdin and uses S_FIFO to check the mode.

As the property is implementation specific then I think something like 
jdk.lang.process.useFork is a better start. It would be nice not to 
require it although I agree we have to walk carefully as this area has a 
tendency to bite back. I don't think you need to make it any more 
configurable than that.

If this is successful then I think we should look at updating the 
hotspot code too as it has the same issue with VM options such as OnError.

-Alan.