Hi Rob, This is only a superficial scan.
The changes in java/java/makefile look pretty horrible. What are all those -R entries?
We will need equivalent changes for the new build system before this is pushed.
Is the spawn use BSD specific (as per UnixProcess.java.BSD) or Apple specific (as per __APPLE_ in UNIXProcess_md.c) ?
Are the UnixProcess.java files similar enough that we could use a single template and generate the per-OS variants?
In UNIXProcess_md.c: 209 #ifdef _CS_PATH 210 char *pathbuf; 211 size_t n; 212 n = confstr(_CS_PATH,NULL,(size_t) 0); 213 pathbuf = malloc(n); 214 if (pathbuf == NULL) 215 abort(); 216 confstr(_CS_PATH, pathbuf, n); 217 return pathbuf; 218 #else what is _CS_PATH and why are we calling abort()? !!!! What is all the xx_ naming ?? David ----- On 23/11/2012 7:27 AM, 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/> Thanks a lot, -Rob
