[PATCH 1/2] server: Only remove non-listening fd from poll

[Zebediah Figura (she/her)]

On 7/4/21 3:11 PM, David Koolhoven wrote:
> On Fri, Jul 02, 2021 at 02:33:09PM -0500, Zebediah Figura (she/her) wrote:
>> On 7/2/21 1:06 PM, David Koolhoven wrote:
>>> This makes sure we poll listening non-connection
>>> file descriptors even if the event is POLLERR or POLLHUP.
>>>
>>> Signed-off-by: David Koolhoven <david at koolhoven-home.net>
>>> ---
>>>    server/sock.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/server/sock.c b/server/sock.c
>>> index befa9117c13..ce2f390ec05 100644
>>> --- a/server/sock.c
>>> +++ b/server/sock.c
>>> @@ -1024,7 +1024,7 @@ static void sock_poll_event( struct fd *fd, int event )
>>>            fprintf(stderr, "socket %p select event: %x\n", sock, event);
>>>    
>>>        /* we may change event later, remove from loop here */
>>> -    if (event & (POLLERR|POLLHUP)) set_fd_events( sock->fd, -1 );
>>> +    if (event & (POLLERR|POLLHUP) && sock->state != SOCK_LISTENING) set_fd_events( sock->fd, -1 );
>>>    
>>>        switch (sock->state)
>>>        {
>>>
>>
>> This doesn't look right. Why do we need to do this?
>>
> I wish I could wrap my head around a way to make it work
> without this, but without the change the next patch causes
> port to stop being used and network
> communications meant for that fd stop.

Wait, I'm confused, is this hunk necessary or not? Is Star Citizen or 
World of Warcraft getting an error on the listening descriptor, and, if 
so, why?

> I know there's some sort of restructuring of these calls
> that would do it, but I can't think of any "Leave things
> mostly the way they are" that would accomplish wineserver
> releasing back to the kernel if it's spinning in a loop in
> this situation besides something even worse like a call to
> sched_yield()

wineserver definitely shouldn't be spinning in a loop at all, so patch 
2/2 has the right idea at least in that respect.

(P.S. As I gather that this patch was written by Torge Matthies from his 
response, it would probably be good to credit him, either as the patch 
author or as someone whose patch this is based on. See also [1].)

[1] 
https://wiki.winehq.org/Submitting_Patches#Submitting_patches_written_by_somebody_else