Review request for 5049299

[Martin Buchholz]

Following up some more...
I did some cleanup of the USE_CLONE vs. non-USE_CLONE
code and tested both of them (on Linux at least).

I took the liberty of removing the references to fork1,
since OpenJDK7 only supports Solaris10 going forward.

The main fork logic now looks like this:

    {
#if USE_CLONE
        /* See clone(2).
         * Instead of worrying about which direction the stack grows, just
         * allocate twice as much and start the stack in the middle. */
        const int stack_size = 64 * 1024;
        if ((clone_stack = NEW(char, 2 * stack_size)) == NULL) goto Catch;
        resultPid = clone(childProcess, clone_stack + stack_size,
                          /* CLONE_VFORK | // works, but unnecessary */
                          CLONE_VM | SIGCHLD, c);
#else
        /* From fork(2): In Solaris 10, a call to fork() is identical
         * to a call to fork1(); only the calling thread is replicated
         * in the child process. This is the POSIX-specified behavior
         * for fork(). */
        resultPid = fork();
        if (resultPid == 0) {
          childProcess(c);
          assert(0);  /* childProcess must not return */
        }
#endif
    }

Patch updated on cr.openjdk.java.net.

Martin

On Tue, Jun 9, 2009 at 11:42, Martin Buchholz <martinrb at google.com> wrote:

>
>
> On Tue, Jun 9, 2009 at 09:28, Michael McMahon <Michael.McMahon at sun.com>wrote:
>
>> Martin Buchholz wrote:
>>
>>> In the old implementation, we used the strategy of
>>> fork+mutate environ+execvp
>>> and we got the traditional shell script semantics
>>> from our use of execvp.
>>> Since environ is now a global shared-with-parent variable,
>>> we can't mutate it any more, therefore can't use execvp,
>>> and must implement the missing execvpe ourselves.
>>>
>>
> One more note:  we could try to have two implementations of
> execvpe, one that mutated environ and one that didn't,
> since each has its own advantages,
> but I think that would be even harder to understand and maintain.
>
> Hmmmmmmmmmmmmmmmmmm.............................
> Looking at the code again, it seems like it would not be too much work.
>
> Here's an untested example of how we could do both.
>
> static void
> execve_with_shell_fallback(const char *file,
>                            const char *argv[],
>                            const char *const envp[])
> {
> #if USE_CLONE
>     execve(file, (char **) argv, (char **) envp);
>     if (errno == ENOEXEC)
>         execve_as_traditional_shell_script(file, argv, envp);
> #else
>     extern char **environ;
>     environ = (char **) envp;
>     execvp(file, (char **) argv);
> #endif
> }
>
> What do you think?
>
> Martin
>
>
>>
>>> Now, strictly speaking, execvp's traditional shell script semantics
>>> is an unspecified implementation detail of execvp;
>>> on other Unix platforms we might not need this. We can leave this to,
>>> e.g. the BSD porters reading this,
>>> to add some #ifdefs.
>>>
>>>  Ok, got you.
>>
>>> (It would also be good if you ran the updated tests on
>>> Solaris with the old implementation to confirm my understanding)
>>>
>>
>> I'm just building it now, and will run the test then.
>>
>> Thanks,
>> Michael.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/core-libs-dev/attachments/20090609/7b5b0eb9/attachment.html>