[MLton] Finished (?) MLton.Child

[Wesley W. Terpstra]

On Wed, Nov 24, 2004 at 05:53:59PM -0800, Stephen Weeks wrote:
> * Cygwin does not use mmap by default.  However, I added a runtime
>   switch, "use-mmap", that lets one enable mmap-based memory
>   allocation on the command line.  This has not been well tested.

That seems like a good compromise.

> * I tested on all our platforms using a Linux -> <platform> cross
>   compiler.  Everything worked fine, except for Cygwin.  With the 
>   CreateProcess-based implementation, the test code dies when
>   attempting to reap the process.  The error is
>     unhandled exception: SysErr: No child processes [child]

CreateProcess does not work with cygwin; I said so in the comments.
That codepath is completely untested by me. I was using fork().

However, the problem here is probably that you are using cygwin's waitpid.
You will notice that in platform/mingw.c waitpid calls _cwait. Maybe if
cygwin were also changed to use _cwait during reap this would work.

There is a cwait in process.h; it might do the same thing... no _ though.

The same goes for kill(); it's not a process id and this will fail.
You need to use TerminateProcess (see kill from mingw.c).

The main reason I used fork() on cygwin is that I don't know how to turn a
HANDLE into a cygwin file descriptor; there is no _open_osfhandle. On the
other hand, I suppose right now the code is using cygwin's pipe(), so maybe
things will just magically work out. Did you see valid output prior to reap?

get_osfhandle in io.h may or may not be the same as _get_osfhandle.
This may get you the HANDLE back, but the important direction is to turn the
HANDLE into a read()able fd. I will need to look in the (ick) cygwin source.

>   With the fork-based implementation, the test code hangs.  I'd be
>   interested to see if you get the same.

I will try cvs/HEAD tomorrow to see this hang.
I definitely did not experience this in my cygwin->cygwin (+fork) builds.
You earlier comment about a 'reap' problem with CP has also given me hope 
for that method. :) If VirtualAlloc and CreateProcess can coexist: great!

I've reviewed the code you committed and it looks more or less the same.
The main difference is how you changed the mmap code around.
This might be the root of problem. What version of Windows?
Can you give me a copy of the test code you were using?

> * It seems wrong that on Cygwin MLton.Process.create expects
>   Windows-style paths, whereas the rest of Cygwin expects Unix-style
>   paths.

It's a very windowsy system call...
I am surprised that it's available at all.

Btw, it returns a HANDLE (a pointer to a Windows resource), not a pid.
This is why mingw works---I declared 'pid' there to be pointer cast to int.

> * At some point, the {searchPath: bool} argument to Process.create got
>   dropped.  I guess the idea is that we could have a separate function
>   "val which: string -> string" that looks in the path for an
>   executable?

I found no portable way to do this under *nix, so I dropped it.
Windows has a method SearchPath which does the job.

I agree that a 'which' method call is the best way to do it if it is
desirable at all. However, it's also an unsafe thing to rely on, 
especially if you are using fork.

Consider: you run which and find an executable.
Then you fork, but something else (re)moves the executable.
The child process then tries to exec() and it fails.

The same problem can happen with an unsearched command, true.
However, an unsearched command implies that the filename is somewhat stable.

> datatype z = datatype PosixPrimitive.file_desc

What is this for?

>            local
>               val null = if useCreate then "nul" else "/dev/null"
>            in
>               val null = File null
>            end

/me writes down this trick for avoiding the polymorphic value rule.

>        if (useWindows)
>                _setmode(fd, _O_BINARY);
>        else
>                /* cygwin has a different method for working with its fds */
>                setmode(fd, O_BINARY);

I am not sure this is a good idea.
Are both of these even defined on both platforms?

> static HANDLE dupHandle (int fd) {
>        HANDLE raw, dupd;
>
>        raw = (HANDLE)_get_osfhandle (fd);

cygwin.c:
> #include "create.c"

!? How does this compile? I see no definition for _get_osfhandle in cygwin.
There's a get_osfhandle in io.h, but where is _get_osfhandle?

-- 
Wesley W. Terpstra <wesley@terpstra.ca>