Re: wine/server sock.c

[Martin Wilck]

Am Fre, 2002-06-21 um 21.06 schrieb Alexandre Julliard:
> Modified files:
>    server         : sock.c
>
> Log message:
>    Attempt at fixing POLLHUP handling for Linux 2.2.x.

I am still wondering about this ... what exactly is fixed by the patch?
Was there a bug report, a failing application, or whatever?

Martin

wine/server sock.c

[Alexandre Julliard]

Martin Wilck  writes:
> I am still wondering about this ... what exactly is fixed by the patch?
> Was there a bug report, a failing application, or whatever?

The regression test was failing because the client didn't receive the
FD_CLOSE notification. With the fix it now works both in 2.2 and 2.4
kernels.

wine/server sock.c

[Martin Wilck]

Am Die, 2002-06-25 um 19.09 schrieb Alexandre Julliard:
> The regression test was failing because the client didn't receive the
> FD_CLOSE notification. With the fix it now works both in 2.2 and 2.4
> kernels.

OK - but I am not sure if the second change can't break something for
2.4 or other POSIX systems - here POLLHUP would IMO mean that the peer
has closed both sides of the connection, justifying to reset FD_WRITE
and FD_WINE_CONNECTED just as we do with POLLERR. Perhaps we should
carry out a small test at wineserver startup to determine whether a
shutdown() can cause a POLLHUP to be received (below a small
proof-of-concept test program that shows how that could work - I tested
it on 2.2 and 2.4, it yields the expected results).

I have another issue that I am not sure how to fix yet: For UDP sockets,
it is perfectly legal to receive 0 bytes, thus an empty receive should
not be interpreted as a hangup.
I think this requires that we store the socket type (STREAM or DGRAM) 
in the server socket structure and test for empty receives only for
DGRAM sockets.

Regards,
Martin

PS I'd be grateful if you could also help me out with that GUID
   question...

#include <stdio.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/poll.h>
#include <sys/socket.h>

typedef enum {
    SOCK_SHUTDOWN_ERROR = -1,
    SOCK_SHUTDOWN_EOF = 0,
    SOCK_SHUTDOWN_POLLHUP = 1
} sock_shutdown_t;

static sock_shutdown_t sock_shutdown_pollhup = SOCK_SHUTDOWN_ERROR;

static sock_shutdown_t sock_check_pollhup (void)
{
    sock_shutdown_t ret = SOCK_SHUTDOWN_ERROR;
    int fd[2], n;
    struct pollfd pfd;
    char dummy;

if ( socketpair ( AF_UNIX, SOCK_STREAM, 0, fd ) ) goto out;
    if ( shutdown ( fd[0], 1 ) ) goto out;

pfd.fd = fd[1];
    pfd.events = POLLIN;
    pfd.revents = 0;

n = poll ( &pfd, 1, 0 );
    if ( n != 1 ) goto out; /* error or timeout */
    if ( pfd.revents & POLLHUP ) 
        ret = SOCK_SHUTDOWN_POLLHUP;
    else if ( pfd.revents & POLLIN &&
              read ( fd[1], &dummy, 1 ) == 0 )
        ret = SOCK_SHUTDOWN_EOF;

out:
    close ( fd[0] );
    close ( fd[1] );
    return ret;
}

int main (void)
{
    sock_shutdown_pollhup = sock_check_pollhup ();
    switch ( sock_shutdown_pollhup )
    {
    case SOCK_SHUTDOWN_EOF:
        printf ( "shutdown() causes EOF\n" );
        break;
    case SOCK_SHUTDOWN_POLLHUP:
        printf ( "shutdown() causes POLLHUP\n" );
        break;
    default:
        printf ( "ERROR in sock_check_pollhup()\n" );
        return 257;
    }
    return 0;
}

wine/server sock.c

[Alexandre Julliard]

Martin Wilck  writes:
> OK - but I am not sure if the second change can't break something for
> 2.4 or other POSIX systems - here POLLHUP would IMO mean that the peer
> has closed both sides of the connection, justifying to reset FD_WRITE
> and FD_WINE_CONNECTED just as we do with POLLERR. Perhaps we should
> carry out a small test at wineserver startup to determine whether a
> shutdown() can cause a POLLHUP to be received (below a small
> proof-of-concept test program that shows how that could work - I tested
> it on 2.2 and 2.4, it yields the expected results).

We may want to do something like that yes. I'm not sure the extra
FD_WRITE will really be a problem in practice, but if we can avoid it
it's certainly preferable.
> PS I'd be grateful if you could also help me out with that GUID
>    question...

I guess you should simply use the same GUIDs than Microsoft does.