[PATCH v2 3/5] winhttp: Send callback for pending receives right away when closing websocket.

[Hans Leidekker]

From: Paul Gofman <pgofman(a)codeweavers.com>

v2: Fix printf format warning.

Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com>
Signed-off-by: Hans Leidekker <hans(a)codeweavers.com>
---
 dlls/winhttp/request.c            | 65 ++++++++++++++++++++++---------
 dlls/winhttp/tests/notification.c | 18 ++++++++-
 2 files changed, 63 insertions(+), 20 deletions(-)

diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c
index 7666bae4e68..39223d41bbb 100644
--- a/dlls/winhttp/request.c
+++ b/dlls/winhttp/request.c
@@ -3244,10 +3244,14 @@ static void send_io_complete( struct object_header *hdr )
     assert( count >= 0 );
 }
 
-static void receive_io_complete( struct socket *socket )
+/* returns FALSE if sending callback should be omitted. */
+static BOOL receive_io_complete( struct socket *socket )
 {
     LONG count = InterlockedDecrement( &socket->hdr.pending_receives );
-    assert( count >= 0 );
+    assert( count >= 0 || socket->state == SOCKET_STATE_CLOSED);
+    /* count is reset to zero during websocket close so if count went negative
+     * then WinHttpWebSocketClose() is to send the callback. */
+    return count >= 0;
 }
 
 static enum socket_opcode map_buffer_type( WINHTTP_WEB_SOCKET_BUFFER_TYPE type )
@@ -3620,22 +3624,24 @@ static void CALLBACK task_socket_receive( TP_CALLBACK_INSTANCE *instance, void *
 
     TRACE("running %p\n", work);
     ret = socket_receive( r->socket, r->buf, r->len, &count, &type );
-    receive_io_complete( r->socket );
 
-    if (!ret)
-    {
-        WINHTTP_WEB_SOCKET_STATUS status;
-        status.dwBytesTransferred = count;
-        status.eBufferType        = type;
-        send_callback( &r->socket->hdr, WINHTTP_CALLBACK_STATUS_READ_COMPLETE, &status, sizeof(status) );
-    }
-    else
+    if (receive_io_complete( r->socket ))
     {
-        WINHTTP_WEB_SOCKET_ASYNC_RESULT result;
-        result.AsyncResult.dwResult = API_READ_DATA;
-        result.AsyncResult.dwError  = ret;
-        result.Operation = WINHTTP_WEB_SOCKET_RECEIVE_OPERATION;
-        send_callback( &r->socket->hdr, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, &result, sizeof(result) );
+        if (!ret)
+        {
+            WINHTTP_WEB_SOCKET_STATUS status;
+            status.dwBytesTransferred = count;
+            status.eBufferType        = type;
+            send_callback( &r->socket->hdr, WINHTTP_CALLBACK_STATUS_READ_COMPLETE, &status, sizeof(status) );
+        }
+        else
+        {
+            WINHTTP_WEB_SOCKET_ASYNC_RESULT result;
+            result.AsyncResult.dwResult = API_READ_DATA;
+            result.AsyncResult.dwError  = ret;
+            result.Operation = WINHTTP_WEB_SOCKET_RECEIVE_OPERATION;
+            send_callback( &r->socket->hdr, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, &result, sizeof(result) );
+        }
     }
 
     release_object( &r->socket->hdr );
@@ -3719,7 +3725,7 @@ static DWORD send_socket_shutdown( struct socket *socket, USHORT status, const v
 {
     DWORD ret;
 
-    socket->state = SOCKET_STATE_SHUTDOWN;
+    if (socket->state < SOCKET_STATE_SHUTDOWN) socket->state = SOCKET_STATE_SHUTDOWN;
 
     if (socket->request->connect->hdr.flags & WINHTTP_FLAG_ASYNC)
     {
@@ -3816,6 +3822,8 @@ static void CALLBACK task_socket_close( TP_CALLBACK_INSTANCE *instance, void *ct
 
 DWORD WINAPI WinHttpWebSocketClose( HINTERNET hsocket, USHORT status, void *reason, DWORD len )
 {
+    enum socket_state prev_state;
+    LONG pending_receives = 0;
     struct socket *socket;
     DWORD ret;
 
@@ -3835,10 +3843,29 @@ DWORD WINAPI WinHttpWebSocketClose( HINTERNET hsocket, USHORT status, void *reas
         return ERROR_INVALID_OPERATION;
     }
 
-    if (socket->state < SOCKET_STATE_SHUTDOWN
+    prev_state = socket->state;
+    socket->state = SOCKET_STATE_CLOSED;
+
+    if (socket->request->connect->hdr.flags & WINHTTP_FLAG_ASYNC)
+    {
+        /* When closing the socket pending receives are cancelled. Setting socket->hdr.pending_receives to zero
+         * will prevent pending receives from sending callbacks. */
+        pending_receives = InterlockedExchange( &socket->hdr.pending_receives, 0 );
+        assert( pending_receives >= 0 );
+        if (pending_receives)
+        {
+            WINHTTP_WEB_SOCKET_ASYNC_RESULT result;
+
+            result.AsyncResult.dwResult = 0;
+            result.AsyncResult.dwError = ERROR_WINHTTP_OPERATION_CANCELLED;
+            result.Operation = WINHTTP_WEB_SOCKET_RECEIVE_OPERATION;
+            send_callback( &socket->hdr, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, &result, sizeof(result) );
+        }
+    }
+
+    if (prev_state < SOCKET_STATE_SHUTDOWN
         && (ret = send_socket_shutdown( socket, status, reason, len, FALSE ))) goto done;
 
-    socket->state = SOCKET_STATE_CLOSED;
     if (socket->request->connect->hdr.flags & WINHTTP_FLAG_ASYNC)
     {
         struct socket_shutdown *s;
diff --git a/dlls/winhttp/tests/notification.c b/dlls/winhttp/tests/notification.c
index 2707151e00f..c57c0ef598a 100644
--- a/dlls/winhttp/tests/notification.c
+++ b/dlls/winhttp/tests/notification.c
@@ -63,6 +63,7 @@ struct notification
 #define NF_WINE_ALLOW  0x0002  /* wine sends notification when it should not */
 #define NF_SIGNAL      0x0004  /* signal wait handle when notified */
 #define NF_MAIN_THREAD 0x0008  /* the operation completes synchronously and callback is called from the main thread */
+#define NF_SAVE_BUFFER 0x0010  /* save buffer data when notified */
 
 struct info
 {
@@ -75,6 +76,8 @@ struct info
     DWORD main_thread_id;
     DWORD last_thread_id;
     DWORD last_status;
+    char buffer[256];
+    unsigned int buflen;
 };
 
 struct test_request
@@ -118,6 +121,11 @@ static void CALLBACK check_notification( HINTERNET handle, DWORD_PTR context, DW
         ok(GetCurrentThreadId() == info->main_thread_id, "%u: expected callback to be called from the same thread\n",
                 info->line);
     }
+    if (info->test[info->index].flags & NF_SAVE_BUFFER)
+    {
+        info->buflen = buflen;
+        memcpy( info->buffer, buffer, min( buflen, sizeof(info->buffer) ));
+    }
 
     if (status_ok && function_ok && info->test[info->index++].flags & NF_SIGNAL)
     {
@@ -694,7 +702,7 @@ static const struct notification websocket_test2[] =
     { winhttp_receive_response,           WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_SIGNAL },
     { winhttp_websocket_complete_upgrade, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED, NF_SIGNAL },
     { winhttp_websocket_receive,          WINHTTP_CALLBACK_STATUS_READ_COMPLETE, NF_SIGNAL },
-    { winhttp_websocket_close,            WINHTTP_CALLBACK_STATUS_REQUEST_ERROR },
+    { winhttp_websocket_close,            WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, NF_MAIN_THREAD | NF_SAVE_BUFFER},
     { winhttp_websocket_close,            WINHTTP_CALLBACK_STATUS_CLOSE_COMPLETE, NF_SIGNAL },
     { winhttp_close_handle,               WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING },
     { winhttp_close_handle,               WINHTTP_CALLBACK_STATUS_CLOSING_CONNECTION, NF_WINE_ALLOW },
@@ -707,6 +715,7 @@ static const struct notification websocket_test2[] =
 static void test_websocket(BOOL secure)
 {
     HANDLE session, connection, request, socket, event;
+    WINHTTP_WEB_SOCKET_ASYNC_RESULT *result;
     WINHTTP_WEB_SOCKET_BUFFER_TYPE type;
     DWORD size, status, err;
     BOOL ret, unload = TRUE;
@@ -951,6 +960,13 @@ static void test_websocket(BOOL secure)
     setup_test( &info, winhttp_websocket_close, __LINE__ );
     ret = pWinHttpWebSocketClose( socket, 1000, (void *)"success", sizeof("success") );
     ok( err == ERROR_SUCCESS, "got %u\n", err );
+    ok( info.buflen == sizeof(*result), "got unexpected buflen %u.\n", info.buflen );
+    result = (WINHTTP_WEB_SOCKET_ASYNC_RESULT *)info.buffer;
+    ok( result->Operation == WINHTTP_WEB_SOCKET_RECEIVE_OPERATION, "got unexpected operation %u.\n",
+        result->Operation );
+    ok( !result->AsyncResult.dwResult, "got unexpected result %lu.\n", result->AsyncResult.dwResult );
+    ok( result->AsyncResult.dwError == ERROR_WINHTTP_OPERATION_CANCELLED, "got unexpected error %u.\n",
+        result->AsyncResult.dwError );
 
     close_status = 0xdead;
     size = sizeof(buffer) + 1;
-- 
2.30.2

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=106083

Your paranoid android.


=== w8adm (32 bit report) ===

winhttp:
notification.c:256: Test failed: failed to send request 12007
notification.c:258: Test failed: unexpected function 3, expected 4. probably some notifications were missing
notification.c:260: Test failed: failed to receive response 12019
notification.c:264: Test failed: failed unexpectedly 12019
notification.c:265: Test failed: request failed unexpectedly 0
notification.c:268: Test failed: unexpected function 3, expected 13. probably some notifications were missing
notification.c:116: Test failed: 268: expected status 0x00000002 got 0x00000800
notification.c:117: Test failed: 268: expected function 3 got 13
notification: Timeout