On Sat, Sep 18, 2010 at 9:59 PM, Mike Kaplinskiy mike.kaplinskiy@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).
Pick a place to do the filtering and stick with it. 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.
... and some calls aren't filtered at all (see blocking recv's).
The blocking WS_recv() is forwarded to WS2_recvfrom(), which forwards to the "async" WS2_recv()...
I would suggest forwarding ReadFile/WriteFile to WSARecv/WSASend for sockets and then taking care of it in ws2_32 (we might end up needing this soon anyway for other annoying reasons).
I'll have to look into this, I am not familiar enough with how Wine handles these operations to make such a change at this time. The ReadFile portion is at least handled through the server-side filtering, though a WriteFile request would go out on all interfaces I'd say that's "not too terrible".
Even if your async reads are woken, you should be lenient enough in WS2_recv to return EWOULDBLOCK if no packets matching the interface are available. Since you don't do this now blocking recv's are broken.
This scenario is part of why some of the filtering occurs on the server side, in all honestly, fixing this issue took a long time to work out. It was previously pointed out that returning EWOULDBLOCK in these scenarios would break "broken" applications that do not handle EWOULDBLOCK.
Also, FYI our *nix sockets are always non-blocking, you don't need MSG_DONTWAIT.
This is contrary to both my experience and the manual pages for recv() and fcntl()... *nix sockets are non-blocking when O_NONBLOCK is set, which is not necessarily the case as Wine handles both types of sockets.
So what if an application likes to bind to an interface AND use IP_PKTINFO?
According to MSDN, while getting the IP_PKTINFO flag is supported, setting it is not.
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.
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.
If you really want to get this to work consider writing something to LD_PRELOAD that emulates most of the network syscalls (+ read) with windows-style bind'ing correctly. I'd argue this may be easier and faster than putting the fix it into wine. This may be more user-friendly than having people compile wine themselves.
I'm not interested in taking the easy way out here, I would much rather put the appropriate effort into having Wine handle this scenario than making a one-off fix.
On Sat, Sep 18, 2010 at 10:22 PM, Mike Kaplinskiy mike.kaplinskiy@gmail.com wrote:
Ah sorry, forgot to mention one more thing - fd's are different on the server side and the client side and between processes. So you're locking different semaphores everywhere effectively not locking anything.
I did not see this in testing, but I understand that it's possible that different systems do not behave the same way. In the revised set (see bottom) I have implemented Damjan's suggestion, which works just as effectively in the testing I've done.
Thank you both for your comments, I hope you find the revised patches below to be more palatable.
[1/3] http://www.compholio.com/wine-kane/patches/2010-09-19/0001-ws2_32-tests-Add-... [2/3] http://www.compholio.com/wine-kane/patches/2010-09-19/0002-server-Add-mechan... [3/3] http://www.compholio.com/wine-kane/patches/2010-09-19/0003-ws2_32-Patch-to-s...
Erich Hoover ehoover@mines.edu