[MLton] Re: [MLton-commit] r6699

[Matthew Fluet]

On Wed, 17 Jun 2009, Wesley W. Terpstra wrote:
> On Tue, Jun 16, 2009 at 5:36 PM, Matthew Fluet <fluet at tti-c.org> wrote:
>> In any case, there seems to be a problem with CreateProcess, possibly independent of the argument munging.  The child process seems to to be created:
>>
>> [fluet at winxp-cygwin tmp]$ ./args-create 'C:\WINDOWS\system32\finger.exe'
>> cmd: ./args-create
>> arg: C:\WINDOWS\system32\finger.exe
>> unhandled exception: SysErr: No child processes [child]
>
> I suspect this is why:
>
>> /* 20070822, fluet: The following 'pure win32' implementation of cwait
>>  * no longer works on recent Cygwin versions.  It always takes the
>>  * {errno = ECHILD; return -1} branch, even when the child process
>>  * exists.
>>  */
>>
>> /* Cygwin replaces cwait with a call to waitpid.
>>  * waitpid only works when the process was created by cygwin and there
>>  * is a secret magical pipe for sending signals and exit statuses over.
>>  * Screw that. We implement our own cwait using pure win32.
>>  */
>> /* C_Errno_t(C_PId_t) MLton_Process_cwait(C_PId_t pid, Ref(C_Status_t) status) {
>
> The code you removed was designed to work when reaping child cygwin
> processes in addition to native applications (like finger). Perhaps
> restore this function?

It seems that there are multiple, independent(?) issues.  Certainly one is 
that a Cygwin program doesn't like to be started via CreateProcess with a 
non-NULL lpEnvironment argument; I can't find any documentation for this, 
but I can definitely demonstrate it with some short C programs.  The 
<src>/runtime/platform/windows.c Windows_Process_create function always 
uses a non-NULL lpEnvironment argument; in the case that the child is 
meant to inherit its parent's environment, the whole environment is copied 
via Posix.ProcEnv.environ.

The other issue is that I don't believe that the int/pid_t returned by 
spawn{,p}{,e} is "the same" as the LPROCESS_INFORMATION.hProcess returned 
by CreateProcess.  I don't believe that the old code really works when 
reaping child processes referenced by their spawn{,p}{,e} pid_t (whether 
or not they are cygwin processes).  This is consistent with my comments 
made at the time of the last release: at that time, there were no 
regressions that tested MLton.Process.{spawn{,e,p},create} but the 
compiler itself used MLton.Process.spawn to invoke gcc and 
MLton_Process_cwait for the implementation of Posix.Process.waitpid to 
synchronize on the termination of gcc.  This is also consistent with my 
current experiments, where finger.exe launched with MLton.Process.spawn 
could by synchronized on via Posix.Process.waitpid.

On the other hand, we probably do need GetExitCodeProcess to synchronize 
on the termination of a program launched via CreateProcess (for which we 
have only the LPROCESS_INFORMATION.hProcess handle).  Again, this is 
consistent with my experiments above, where sending the 
LPROCESS_INFORMATION.hProcess handle to MLton_Process_cwait yields the "no 
child process [child]" error.

It turns out that the MLton.Process.create result doesn't actually provide 
a means to extract the process id, although MLton.Process.Child.getPid 
would be trivial.  This is a good thing, because it means that 
MLton.Process.reap can always use the old MLton_Process_cwait (with 
GetExitCodeProcess), because it will necessarily be a 
LPROCESS_INFORMATION.hProcess handle, while Posix.Process.waitpid can use 
the new MLton_Process_cwait, because it will necessarily be a 
spawn{,e}{,p} pid_t.  (Actually, I think the Cygwin waitpid does work fine 
with a spawn{,e}{,p} pid_t, so there doesn't need to be a special case.)