Re: [Monotone-devel] hang on Win32/MinGW with sync file:

[Matthew Gregan]

At 2007-12-04T19:33:35-0500, Stephen Leake wrote:
> Currently, all code outside netxx_pipe.cc uses netxx_pipe.cc. So I don't
> see what we are gaining by replacing Unix pipes by TCP sockets.

Simplicity.  See Zack's summary below.

> I'm not clear what you mean by "cross platform".
> 
> To me, that means "not needing '#ifdef OS'". Clearly, netxx_pipe.cc
> needs '#ifdef OS'.
> 
> netxx_pipe.cc is mostly either WIN32 or Unix; almost none of it is shared.

The current netxx_pipe.cc is a bit of an exception to the rest of the code
in monotone (except maybe tester.cc, but that's special).  In general, when
we need to use some platform-specific API, we wrap it up and put appropriate
implementations in unix/ and win32/.  The user of the wrapped API #includes
platform.hh and can write general cross-platform code and expect that the
wrapped call will behave "the same" across platforms.

So the idea in this case is to put a socketpair() implementation into unix/
and win32 and netxx_pipe.cc becomes OS agnostic.

> Why should we change the unix part of netxx_pipe.cc? It's not broken;
> let's not "fix" it.

Simplicity.  See Zack's summary below.

Also, we're not "fixing" it, we're reworking the code with the intention of
making it better.  If it turns out that we're really better off using pipes
on Unix and socket pairs on Win32, fine, but I'd rather that we tried the
solution that resulted in the least, simplest code first.

At 2007-12-04T16:44:25-0800, Zack Weinberg wrote:
> Unless I am very confused, the idea is to be able to use the current
> Unix-specific code (replacing two calls to pipe() with one call to
> socketpair()) on both Unix and Windows; and the benefit of this is that
> the low-level Windows API for asynchronous I/O has fundamentally different
> semantics from the low-level Unix API, and the rest of netxx expects the
> Unix semantics, which is why we are getting hangs.

Right, that's the plan.

> ... wait, are we actually going to be able to do async I/O with ncm's fake
> socketpair()?  I do not fully understand the significance of the
> "make_overlapped" flag, but I see comments in netxx_pipe.cc averring that
> async I/O only works on Windows in overlapped mode, and comments in
> socketpair.c saying the socket is not suitable for use as a child process
> stdio handle if overlapped mode is on.

Yes, calling select() on these non-overlapped sockets still works fine.  The
flag for dumb_socketpair() is to do with Win32-specific overlapped I/O
model.  Passing 1 would get you the standard behaviour from WinSock.  In our
case, we need to pass 0 to ensure one of the sockets is not created in
overlapped mode.  This is the socket we pass to the child process as a stdio
handle.

Overlapped I/O is also asynchronous, but you need to use a different
programming model and APIs to take advantage of it (e.g. you need to use
WSASend() with a WSAOVERLAPPED structure, plain send() is non-overlapped no
matter what mode the socket was created in).

The problem I ran into when first working on this, after much hair-tearing,
is that a process expects its stdin/stdout/stderr handles to be
non-overlapped.  The process calls functions like {Read,Write}File() on the
stdio handles as if they are non-overlapped.  These functions notice the
handle has overlapped mode set and try to work in overlapped mode even
though the caller isn't expecting them to.  Chaos ensues.

Cheers,
-mjg
-- 
Matthew Gregan                     |/
                                  /|                    address@hidden