http://bugs.winehq.org/show_bug.cgi?id=9425
--- Comment #31 from Ruediger Meier sweet_f_a@gmx.de 2012-02-09 16:41:02 CST --- (In reply to comment #30)
(In reply to comment #28)
- POLLHUP can happen always without subscribing it.
Since a file descriptor can (and usually will) be listed twice in our pollfd array it's undefined which particular pollfd struct will receive a POLLHUP. I saw any thinkable cases happen. It's only luck if a POLLHUP will finally make it into exceptfds.
Yes, it can happen and it's quite natural sometimes. For example a broken connection is detected when select() sets the READFD and when you try to recv() on that fd you will receive 0 meaning the connection was half-closed, wine's current implementation lets that happen. poll() should behave the same in all systems (posix compliance) but it's well know it does not =)
In my attached test case I have no connection yet, It's about non-blocking sockets where you simply cannot detect "Connection refused". You will get "timeout" instead. In real world this mostly doesn't matters but I can imagine or construct use cases where this little thing could lead to really bad behaviour.
- IMO it's questionable whether we should always put POLLHUP into exceptfds at
all.
I've been thinking about this but only tests can prove this and it will probably vary in the different windows versions.
I guess not. For example with blocking sockets (and no OOB) you should never ever set exceptfds but wine does it sometimes. However this is maybe really no issue because if a programmer is stupid enough to select exceptfds where he would not need it then he probably does something useful when this case happens anyway :)
BUT the important point here in 1.and 2. is related to your fix "avoid false-positive write FDs". These removed POLLHUPs have to go either to readfds or exceptfds because if not then the programmer will think it's just timeout, probably trying select again, probably closing that connection after a timeout period. He does this although he could have known much earlier that connection is dead. Application will still work perfectly but it could have overall performance impact.
- Since we ignore some events (so possibly all events) we must poll again to
really timeout instead of returning 0 without timeouting.
Usually wine coding strategy is to implement things required for real life applications to run. There are plenty network applications running really well in wine currently, [...]
So you fix bugs not if you see them clearly but only after a user run into them?
- POLLPRI (OOB data) is completely ignored currently.
POLLPRI is not needed there currently, usually the strategy is to use the OOBINLINE check and then recv() with MSG_OOB. The wine server already takes care of some POLLPRI stuff as seen in http://source.winehq.org/source/server/sock.c#L324 select() does not return weather there is or not priority data. A real life app example is putty which works in wine currently using the telnet protocol. Maybe you say we should test (POLLIN | POLLPRI) in poll to return the readfds? I don't have objections if that's the case.
POLLPRI has always to go into exceptfds, thats the easiest one to fix.
- What about POLLERR and POLLNVAL? Can't they happen because of our checks
before calling poll?
Yeah, I think POLLNVAL will never happen. And POLLERR is also not required because the current conditions for read/except will allow setting the fd in this case.
Haven't checked that completely. I think you are right about POLLNVAL but have no finally opinion about POLLERR yet although I guess it should only go to writefds.
To fix 1. we need would need a real mapping fd -> "select fds" rather than just mapping array positions. If we had this then we also could more cleanly sort the events into the right "select fds" when we think about 2-5.
I don't understand what's wrong with the current implementation =)
I've simply noticed odd behavior. I am not the usual wine user which is happy about utorrent or whatever. Porting my own developed stuff to win32 I've tried to use wine instead of a real-pain-windows but run into these issues.
To convert the write fd errors in exceptions you could use a fd_set to insert the writefd errors and then in the exceptfs For you could check this fd_set together with the curren check.
If nobody else picks this up I would try to fix some of the points. At the moment less on time maybe when I'll feel the need to do sports at some weekend.