RFC: Revised patchset to fix Bug 7929 (C&C 3 network does not work)

[Erich Hoover]

On Sun, Sep 19, 2010 at 1:13 PM, Mike Kaplinskiy
<mike.kaplinskiy at gmail.com> wrote:
> On Sun, Sep 19, 2010 at 1:33 PM, Erich Hoover <ehoover at mines.edu> wrote:
>> On Sat, Sep 18, 2010 at 9:59 PM, Mike Kaplinskiy <mike.kaplinskiy at gmail.com> wrote:
>>> There's enough #ifdef's to make this mostly unreadable.
>> Sorry about this, I was concerned that pushing these out so that
>> changes do not appear directly in the relevant code would make things
>> too obfuscated.  Please look at the revised patchset (at bottom).
> Thanks that helps a bit. You could still do a few more of those
> cleanups.
These defines are no-longer in socket.c at all and they are extremely
minimal in the shim ...

> Also please don't make a function like WS2_shim_poll; either
> make it poll-like or call it WS2_shim_select.
I apologize here, this is an unfortunate case of over-zealous
copy-pasting from the old code - please see the revised patches below.

> ...  you can still have funky stuff happening (returning 0 from
> recv because all the packets were filtered out). You really need to
> return EWOULDBLOCK from WS2_recv for it all to fix itself.
When these additions filter out a packet after a recv() it then
re-calls recv(), so it would return EWOULDBLOCK if there were no
un-filtered packets.

>>> You do some filtering in the server, some filtering in wine-userspace ...
>> The filtering in the server is done to handle throwing packets away
>> from async polls and calls that come through ReadFile.
> That's what I mean - if you need to do filtering for ReadFile - do it
> in ReadFile (conditionally, there is a way to check if the fd is a
> socket). Besides the little scheme in the server would only fix async
> ReadFile requests. If you just called ReadFile with some data pending
> you would still get it even if it wasn't to the right interface.
Please examine the revised patches below, patch 3 redirects ReadFile
and WriteFile.

>>> Alexandre's comments from last year still apply.
>> Unless you're talking about the problem where descriptors are not
>> invariant enough then I don't see how, these changes should make the
>> peek/read combo safe across threads and processes.
> This will probably have to be Alexandre's call. I was referring to his
> first comment mostly.
>
>>> For example, now we wouldn't be able to use poll anywhere reliably on sockets.
>> This case should be handled in one of the patches, the revised set
>> (see bottom) should make this more obvious.  It looks like I missed
>> locking around some polls, this issue should be corrected in the
>> revised set.
> That just fixes the problem in ws2_32. We still can't use poll on
> sockets anywhere else (notably the server). As a consequence FD_READ
> socket events would now fire for every packet. To fix that you filter
> out POLLIN when necessary (which is ok), just your reasoning for doing
> it is a little wrong. WSARecv overlapped requests end up on that queue
> as well.
It seems the comment I put on the server side is from before I had a
better understanding of the server-side poll, I apologize and have
corrected this issue in the patches below.  I believe that I've now
covered the different socket poll() cases, is there something else I'm
missing here?  This issue seemed to be the crux behind previous
reservations on implementing this feature (such as Alexandre's first
comment), so I would like to make sure that I have satisfactorily
resolved this problem.

Thank you for your help on all these issues, it is much appreciated.
I believe the patches below will be more to your liking:

[1/4] http://www.compholio.com/wine-kane/patches/2010-09-25/0001-ws2_32-tests-Add-UDP-broadcast-tests.patch
[2/4] http://www.compholio.com/wine-kane/patches/2010-09-25/0002-server-Add-mechanism-for-storing-an-interface-ID-wit.patch
[3/4] http://www.compholio.com/wine-kane/patches/2010-09-25/0003-ntdll-Pass-ReadFile-and-WriteFile-requests-on-socket.patch
[4/4] http://www.compholio.com/wine-kane/patches/2010-09-25/0004-ws2_32-Selectively-bind-UDP-sockets-to-interfaces-wh.patch

Erich Hoover
ehoover at mines.edu