On Sun, Sep 19, 2010 at 1:13 PM, Mike Kaplinskiy mike.kaplinskiy@gmail.com wrote:
On Sun, Sep 19, 2010 at 1:33 PM, Erich Hoover ehoover@mines.edu wrote:
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).
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-... [2/4] http://www.compholio.com/wine-kane/patches/2010-09-25/0002-server-Add-mechan... [3/4] http://www.compholio.com/wine-kane/patches/2010-09-25/0003-ntdll-Pass-ReadFi... [4/4] http://www.compholio.com/wine-kane/patches/2010-09-25/0004-ws2_32-Selectivel...
Erich Hoover ehoover@mines.edu