[PATCH v2 0/7] MR195: winhttp: Abort web socket receive operations when the socket handle is closed
I think somewhat similar should happen with normal winhttp requests (I was doing some brief tests around that back then) and that should hopefully be easy to add incrementally after this patchset. Then, sends for websockets should most likely also be cancelled the same way although that is a bit harder to test as most of the time sends are processed right away before returing from WinHttpWebSocketSend (although should be possible as out of tree test which might be flaky). The patches here address cancelling web socket receive operation (including receiving web socket close status). -- v2: winhttp/tests: Add tests for closing web socket with pending operations. winhttp: Support abort in task_socket_close(). winhttp: Abort websocket IO on handle close. winhttp: Replace pending read cancel in WinHttpWebSocketClose() with a generic cancel_queue(). winhttp: Fix dwResult field in websocket receive error result. winhttp: Implement reference counting for tasks. winhttp: Keep task in queue until completion. https://gitlab.winehq.org/wine/wine/-/merge_requests/195
From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index fe703c19109..9abe4f45598 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -135,15 +135,16 @@ void stop_queue( struct queue *queue ) TRACE("stopped %p\n", queue); } -static struct task_header *get_next_task( struct queue *queue ) +static struct task_header *get_next_task( struct queue *queue, struct task_header *prev_task ) { struct list *entry; AcquireSRWLockExclusive( &queue->lock ); assert( queue->callback_running ); - if ((entry = list_head( &queue->queued_tasks ))) - list_remove( entry ); - else + if (prev_task) + list_remove( &prev_task->entry ); + + if (!(entry = list_head( &queue->queued_tasks ))) queue->callback_running = FALSE; ReleaseSRWLockExclusive( &queue->lock ); if (!entry) return NULL; @@ -157,12 +158,12 @@ static void CALLBACK task_callback( TP_CALLBACK_INSTANCE *instance, void *ctx ) TRACE( "instance %p.\n", instance ); - task = get_next_task( queue ); + task = get_next_task( queue, NULL ); while (task) { task->callback( task ); /* Queue object may be freed by release_object() unless there is another task referencing it. */ - next_task = get_next_task( queue ); + next_task = get_next_task( queue, task ); release_object( task->obj ); free( task ); task = next_task; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/195
From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 32 +++++++++++++++++++++++++++----- dlls/winhttp/winhttp_private.h | 1 + 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 9abe4f45598..70d727c7e2e 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -135,20 +135,41 @@ void stop_queue( struct queue *queue ) TRACE("stopped %p\n", queue); } +static void addref_task( struct task_header *task ) +{ + InterlockedIncrement( &task->refs ); +} + +static void release_task( struct task_header *task ) +{ + if (!InterlockedDecrement( &task->refs )) + free( task ); +} + static struct task_header *get_next_task( struct queue *queue, struct task_header *prev_task ) { + struct task_header *task; struct list *entry; AcquireSRWLockExclusive( &queue->lock ); assert( queue->callback_running ); if (prev_task) + { list_remove( &prev_task->entry ); - - if (!(entry = list_head( &queue->queued_tasks ))) + release_task( prev_task ); + } + if ((entry = list_head( &queue->queued_tasks ))) + { + task = LIST_ENTRY( entry, struct task_header, entry ); + addref_task( task ); + } + else + { + task = NULL; queue->callback_running = FALSE; + } ReleaseSRWLockExclusive( &queue->lock ); - if (!entry) return NULL; - return LIST_ENTRY( entry, struct task_header, entry ); + return task; } static void CALLBACK task_callback( TP_CALLBACK_INSTANCE *instance, void *ctx ) @@ -165,7 +186,7 @@ static void CALLBACK task_callback( TP_CALLBACK_INSTANCE *instance, void *ctx ) /* Queue object may be freed by release_object() unless there is another task referencing it. */ next_task = get_next_task( queue, task ); release_object( task->obj ); - free( task ); + release_task( task ); task = next_task; } TRACE( "instance %p exiting.\n", instance ); @@ -178,6 +199,7 @@ static DWORD queue_task( struct queue *queue, TASK_CALLBACK task, struct task_he TRACE("queueing %p in %p\n", task_hdr, queue); task_hdr->callback = task; + task_hdr->refs = 1; task_hdr->obj = obj; addref_object( obj ); diff --git a/dlls/winhttp/winhttp_private.h b/dlls/winhttp/winhttp_private.h index 4754d9258d1..ee3cdb78157 100644 --- a/dlls/winhttp/winhttp_private.h +++ b/dlls/winhttp/winhttp_private.h @@ -281,6 +281,7 @@ struct task_header struct list entry; TASK_CALLBACK callback; struct object_header *obj; + volatile LONG refs; }; struct send_request -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/195
From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 70d727c7e2e..29887c8d7d0 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -3839,6 +3839,25 @@ static DWORD socket_receive( struct socket *socket, void *buf, DWORD len, DWORD return ret; } +static void socket_receive_complete( struct socket *socket, DWORD ret, WINHTTP_WEB_SOCKET_BUFFER_TYPE type, DWORD len ) +{ + if (!ret) + { + WINHTTP_WEB_SOCKET_STATUS status; + status.dwBytesTransferred = len; + status.eBufferType = type; + send_callback( &socket->hdr, WINHTTP_CALLBACK_STATUS_READ_COMPLETE, &status, sizeof(status) ); + } + else + { + WINHTTP_WEB_SOCKET_ASYNC_RESULT result; + result.AsyncResult.dwResult = 0; + result.AsyncResult.dwError = ret; + result.Operation = WINHTTP_WEB_SOCKET_RECEIVE_OPERATION; + send_callback( &socket->hdr, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, &result, sizeof(result) ); + } +} + static void task_socket_receive( void *ctx ) { struct socket_receive *r = ctx; @@ -3850,23 +3869,7 @@ static void task_socket_receive( void *ctx ) ret = socket_receive( socket, r->buf, r->len, &count, &type ); if (receive_io_complete( socket )) - { - if (!ret) - { - WINHTTP_WEB_SOCKET_STATUS status; - status.dwBytesTransferred = count; - status.eBufferType = type; - send_callback( &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( &socket->hdr, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, &result, sizeof(result) ); - } - } + socket_receive_complete( socket, ret, type, count ); } DWORD WINAPI WinHttpWebSocketReceive( HINTERNET hsocket, void *buf, DWORD len, DWORD *ret_len, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/195
From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 108 +++++++++++++++++++++++---------- dlls/winhttp/winhttp_private.h | 3 +- 2 files changed, 78 insertions(+), 33 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 29887c8d7d0..4242551ee3b 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -182,7 +182,7 @@ static void CALLBACK task_callback( TP_CALLBACK_INSTANCE *instance, void *ctx ) task = get_next_task( queue, NULL ); while (task) { - task->callback( task ); + task->callback( task, FALSE ); /* Queue object may be freed by release_object() unless there is another task referencing it. */ next_task = get_next_task( queue, task ); release_object( task->obj ); @@ -199,6 +199,7 @@ static DWORD queue_task( struct queue *queue, TASK_CALLBACK task, struct task_he TRACE("queueing %p in %p\n", task_hdr, queue); task_hdr->callback = task; + task_hdr->completion_sent = 0; task_hdr->refs = 1; task_hdr->obj = obj; addref_object( obj ); @@ -225,6 +226,35 @@ static DWORD queue_task( struct queue *queue, TASK_CALLBACK task, struct task_he return ERROR_SUCCESS; } +static BOOL task_needs_completion( struct task_header *task_hdr ) +{ + return !InterlockedExchange( &task_hdr->completion_sent, 1 ); +} + +static void cancel_queue( struct queue *queue ) +{ + struct task_header *task_hdr, *found; + + while (1) + { + AcquireSRWLockExclusive( &queue->lock ); + found = NULL; + LIST_FOR_EACH_ENTRY( task_hdr, &queue->queued_tasks, struct task_header, entry ) + { + if (task_needs_completion( task_hdr )) + { + found = task_hdr; + addref_task( found ); + break; + } + } + ReleaseSRWLockExclusive( &queue->lock ); + if (!found) break; + found->callback( found, TRUE ); + release_task( found ); + } +} + static void free_header( struct header *header ) { free( header->field ); @@ -2231,11 +2261,13 @@ end: return ret; } -static void task_send_request( void *ctx ) +static void task_send_request( void *ctx, BOOL abort ) { struct send_request *s = ctx; struct request *request = (struct request *)s->task_hdr.obj; + if (abort) return; + TRACE( "running %p\n", ctx ); send_request( request, s->headers, s->headers_len, s->optional, s->optional_len, s->total_len, s->context, TRUE ); @@ -2813,11 +2845,13 @@ static DWORD receive_response( struct request *request, BOOL async ) return ret; } -static void task_receive_response( void *ctx ) +static void task_receive_response( void *ctx, BOOL abort ) { struct receive_response *r = ctx; struct request *request = (struct request *)r->task_hdr.obj; + if (abort) return; + TRACE("running %p\n", ctx); receive_response( request, TRUE ); } @@ -2905,11 +2939,13 @@ done: return ret; } -static void task_query_data_available( void *ctx ) +static void task_query_data_available( void *ctx, BOOL abort ) { struct query_data *q = ctx; struct request *request = (struct request *)q->task_hdr.obj; + if (abort) return; + TRACE("running %p\n", ctx); query_data_available( request, q->available, TRUE ); } @@ -2956,11 +2992,13 @@ BOOL WINAPI WinHttpQueryDataAvailable( HINTERNET hrequest, LPDWORD available ) return !ret || ret == ERROR_IO_PENDING; } -static void task_read_data( void *ctx ) +static void task_read_data( void *ctx, BOOL abort ) { struct read_data *r = ctx; struct request *request = (struct request *)r->task_hdr.obj; + if (abort) return; + TRACE("running %p\n", ctx); read_data( request, r->buffer, r->to_read, r->read, TRUE ); } @@ -3031,11 +3069,13 @@ static DWORD write_data( struct request *request, const void *buffer, DWORD to_w return ret; } -static void task_write_data( void *ctx ) +static void task_write_data( void *ctx, BOOL abort ) { struct write_data *w = ctx; struct request *request = (struct request *)w->task_hdr.obj; + if (abort) return; + TRACE("running %p\n", ctx); write_data( request, w->buffer, w->to_write, w->written, TRUE ); } @@ -3315,13 +3355,10 @@ static void send_io_complete( struct object_header *hdr ) } /* returns FALSE if sending callback should be omitted. */ -static BOOL receive_io_complete( struct socket *socket ) +static void receive_io_complete( struct socket *socket ) { LONG count = InterlockedDecrement( &socket->hdr.pending_receives ); - 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; + assert( count >= 0 ); } static BOOL socket_can_send( struct socket *socket ) @@ -3428,12 +3465,14 @@ static DWORD socket_send( struct socket *socket, WINHTTP_WEB_SOCKET_BUFFER_TYPE return send_frame( socket, opcode, 0, buf, len, final, ovr ); } -static void task_socket_send( void *ctx ) +static void task_socket_send( void *ctx, BOOL abort ) { struct socket_send *s = ctx; struct socket *socket = (struct socket *)s->task_hdr.obj; DWORD ret; + if (abort) return; + TRACE("running %p\n", ctx); if (s->complete_async) ret = complete_send_frame( socket, &s->ovr, s->buf ); @@ -3623,11 +3662,13 @@ static DWORD receive_frame( struct socket *socket, DWORD *ret_len, enum socket_o return ERROR_SUCCESS; } -static void task_socket_send_pong( void *ctx ) +static void task_socket_send_pong( void *ctx, BOOL abort ) { struct socket_send *s = ctx; struct socket *socket = (struct socket *)s->task_hdr.obj; + if (abort) return; + TRACE("running %p\n", ctx); if (s->complete_async) complete_send_frame( socket, &s->ovr, NULL ); @@ -3858,17 +3899,23 @@ static void socket_receive_complete( struct socket *socket, DWORD ret, WINHTTP_W } } -static void task_socket_receive( void *ctx ) +static void task_socket_receive( void *ctx, BOOL abort ) { struct socket_receive *r = ctx; struct socket *socket = (struct socket *)r->task_hdr.obj; DWORD ret, count; WINHTTP_WEB_SOCKET_BUFFER_TYPE type; + if (abort) + { + socket_receive_complete( socket, ERROR_WINHTTP_OPERATION_CANCELLED, 0, 0 ); + return; + } + TRACE("running %p\n", ctx); ret = socket_receive( socket, r->buf, r->len, &count, &type ); - - if (receive_io_complete( socket )) + receive_io_complete( socket ); + if (task_needs_completion( &r->task_hdr )) socket_receive_complete( socket, ret, type, count ); } @@ -3939,12 +3986,14 @@ static void socket_shutdown_complete( struct socket *socket, DWORD ret ) } } -static void task_socket_shutdown( void *ctx ) +static void task_socket_shutdown( void *ctx, BOOL abort ) { struct socket_shutdown *s = ctx; struct socket *socket = (struct socket *)s->task_hdr.obj; DWORD ret; + if (abort) return; + TRACE("running %p\n", ctx); if (s->complete_async) ret = complete_send_frame( socket, &s->ovr, s->reason ); @@ -4073,15 +4122,18 @@ static void socket_close_complete( struct socket *socket, DWORD ret ) } } -static void task_socket_close( void *ctx ) +static void task_socket_close( void *ctx, BOOL abort ) { struct socket_shutdown *s = ctx; struct socket *socket = (struct socket *)s->task_hdr.obj; DWORD ret; + if (abort) return; + TRACE("running %p\n", ctx); ret = socket_close( socket ); + receive_io_complete( socket ); socket_close_complete( socket, ret ); } @@ -4113,25 +4165,14 @@ DWORD WINAPI WinHttpWebSocketClose( HINTERNET hsocket, USHORT status, void *reas 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) ); - } + pending_receives = InterlockedIncrement( &socket->hdr.pending_receives ); + cancel_queue( &socket->recv_q ); } if (prev_state < SOCKET_STATE_SHUTDOWN && (ret = send_socket_shutdown( socket, status, reason, len, FALSE ))) goto done; - if (!pending_receives && socket->close_frame_received) + if (pending_receives == 1 && socket->close_frame_received) { if (socket->request->connect->hdr.flags & WINHTTP_FLAG_ASYNC) socket_close_complete( socket, socket->close_frame_receive_err ); @@ -4144,7 +4185,10 @@ DWORD WINAPI WinHttpWebSocketClose( HINTERNET hsocket, USHORT status, void *reas if (!(s = calloc( 1, sizeof(*s) ))) return FALSE; if ((ret = queue_task( &socket->recv_q, task_socket_close, &s->task_hdr, &socket->hdr ))) + { + InterlockedDecrement( &socket->hdr.pending_receives ); free( s ); + } } else ret = socket_close( socket ); diff --git a/dlls/winhttp/winhttp_private.h b/dlls/winhttp/winhttp_private.h index ee3cdb78157..17102107c77 100644 --- a/dlls/winhttp/winhttp_private.h +++ b/dlls/winhttp/winhttp_private.h @@ -274,7 +274,7 @@ struct socket BOOL last_receive_final; }; -typedef void (*TASK_CALLBACK)( void *ctx ); +typedef void (*TASK_CALLBACK)( void *ctx, BOOL abort ); struct task_header { @@ -282,6 +282,7 @@ struct task_header TASK_CALLBACK callback; struct object_header *obj; volatile LONG refs; + volatile LONG completion_sent; }; struct send_request -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/195
From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/handle.c | 7 ++++++- dlls/winhttp/net.c | 10 +++++++++- dlls/winhttp/request.c | 21 +++++++++++++++++++-- dlls/winhttp/session.c | 3 +++ dlls/winhttp/winhttp_private.h | 2 ++ 5 files changed, 39 insertions(+), 4 deletions(-) diff --git a/dlls/winhttp/handle.c b/dlls/winhttp/handle.c index 38959fd6456..33a4cd6bb71 100644 --- a/dlls/winhttp/handle.c +++ b/dlls/winhttp/handle.c @@ -142,7 +142,12 @@ BOOL free_handle( HINTERNET hinternet ) LeaveCriticalSection( &handle_cs ); - if (hdr) release_object( hdr ); + if (hdr) + { + if (hdr->vtbl->handle_closing) + hdr->vtbl->handle_closing( hdr ); + release_object( hdr ); + } EnterCriticalSection( &handle_cs ); if (next_handle > handle && !handles[handle]) next_handle = handle; diff --git a/dlls/winhttp/net.c b/dlls/winhttp/net.c index 07a65c5465f..17520a54f63 100644 --- a/dlls/winhttp/net.c +++ b/dlls/winhttp/net.c @@ -269,7 +269,8 @@ void netconn_close( struct netconn *conn ) free(conn->extra_buf); DeleteSecurityContext(&conn->ssl_ctx); } - closesocket( conn->socket ); + if (conn->socket != -1) + closesocket( conn->socket ); release_host( conn->host ); free(conn); } @@ -629,6 +630,13 @@ DWORD netconn_recv( struct netconn *conn, void *buf, size_t len, int flags, int return ERROR_SUCCESS; } +void netconn_cancel_io( struct netconn *conn ) +{ + SOCKET socket = InterlockedExchange( (LONG *)&conn->socket, -1 ); + + closesocket( socket ); +} + ULONG netconn_query_data_available( struct netconn *conn ) { return conn->secure ? conn->peek_len : 0; diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 4242551ee3b..269af0826c0 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -231,9 +231,10 @@ static BOOL task_needs_completion( struct task_header *task_hdr ) return !InterlockedExchange( &task_hdr->completion_sent, 1 ); } -static void cancel_queue( struct queue *queue ) +static BOOL cancel_queue( struct queue *queue ) { struct task_header *task_hdr, *found; + BOOL cancelled = FALSE; while (1) { @@ -250,9 +251,11 @@ static void cancel_queue( struct queue *queue ) } ReleaseSRWLockExclusive( &queue->lock ); if (!found) break; + cancelled = TRUE; found->callback( found, TRUE ); release_task( found ); } + return cancelled; } static void free_header( struct header *header ) @@ -1885,7 +1888,8 @@ static void finished_reading( struct request *request ) if (!request->netconn) return; - if (request->hdr.disable_flags & WINHTTP_DISABLE_KEEP_ALIVE) close = TRUE; + if (request->netconn->socket == -1) close = TRUE; + else if (request->hdr.disable_flags & WINHTTP_DISABLE_KEEP_ALIVE) close = TRUE; else if (!query_headers( request, WINHTTP_QUERY_CONNECTION, NULL, connection, &size, NULL ) || !query_headers( request, WINHTTP_QUERY_PROXY_CONNECTION, NULL, connection, &size, NULL )) { @@ -3121,6 +3125,18 @@ BOOL WINAPI WinHttpWriteData( HINTERNET hrequest, const void *buffer, DWORD to_w return !ret; } +static void socket_handle_closing( struct object_header *hdr ) +{ + struct socket *socket = (struct socket *)hdr; + BOOL pending_tasks; + + pending_tasks = cancel_queue( &socket->send_q ); + pending_tasks = cancel_queue( &socket->recv_q ) || pending_tasks; + + if (pending_tasks) + netconn_cancel_io( socket->request->netconn ); +} + static BOOL socket_query_option( struct object_header *hdr, DWORD option, void *buffer, DWORD *buflen ) { FIXME( "unimplemented option %lu\n", option ); @@ -3151,6 +3167,7 @@ static BOOL socket_set_option( struct object_header *hdr, DWORD option, void *bu static const struct object_vtbl socket_vtbl = { + socket_handle_closing, socket_destroy, socket_query_option, socket_set_option, diff --git a/dlls/winhttp/session.c b/dlls/winhttp/session.c index 3b00d2448d1..01e95895eb3 100644 --- a/dlls/winhttp/session.c +++ b/dlls/winhttp/session.c @@ -242,6 +242,7 @@ static BOOL session_set_option( struct object_header *hdr, DWORD option, void *b static const struct object_vtbl session_vtbl = { + NULL, session_destroy, session_query_option, session_set_option @@ -382,6 +383,7 @@ static BOOL connect_query_option( struct object_header *hdr, DWORD option, void static const struct object_vtbl connect_vtbl = { + NULL, connect_destroy, connect_query_option, NULL @@ -1085,6 +1087,7 @@ static BOOL request_set_option( struct object_header *hdr, DWORD option, void *b static const struct object_vtbl request_vtbl = { + NULL, request_destroy, request_query_option, request_set_option diff --git a/dlls/winhttp/winhttp_private.h b/dlls/winhttp/winhttp_private.h index 17102107c77..98e05f068ba 100644 --- a/dlls/winhttp/winhttp_private.h +++ b/dlls/winhttp/winhttp_private.h @@ -30,6 +30,7 @@ struct object_header; struct object_vtbl { + void (*handle_closing) ( struct object_header * ); void (*destroy)( struct object_header * ); BOOL (*query_option)( struct object_header *, DWORD, void *, DWORD * ); BOOL (*set_option)( struct object_header *, DWORD, void *, DWORD ); @@ -370,6 +371,7 @@ DWORD netconn_recv( struct netconn *, void *, size_t, int, int * ) DECLSPEC_HIDD DWORD netconn_resolve( WCHAR *, INTERNET_PORT, struct sockaddr_storage *, int ) DECLSPEC_HIDDEN; DWORD netconn_secure_connect( struct netconn *, WCHAR *, DWORD, CredHandle *, BOOL ) DECLSPEC_HIDDEN; DWORD netconn_send( struct netconn *, const void *, size_t, int *, WSAOVERLAPPED * ) DECLSPEC_HIDDEN; +void netconn_cancel_io( struct netconn *conn ) DECLSPEC_HIDDEN; DWORD netconn_set_timeout( struct netconn *, BOOL, int ) DECLSPEC_HIDDEN; BOOL netconn_is_alive( struct netconn * ) DECLSPEC_HIDDEN; const void *netconn_get_certificate( struct netconn * ) DECLSPEC_HIDDEN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/195
From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 269af0826c0..2754dc29163 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -4145,13 +4145,18 @@ static void task_socket_close( void *ctx, BOOL abort ) struct socket *socket = (struct socket *)s->task_hdr.obj; DWORD ret; - if (abort) return; + if (abort) + { + socket_close_complete( socket, ERROR_WINHTTP_OPERATION_CANCELLED ); + return; + } TRACE("running %p\n", ctx); ret = socket_close( socket ); receive_io_complete( socket ); - socket_close_complete( socket, ret ); + if (task_needs_completion( &s->task_hdr )) + socket_close_complete( socket, ret ); } DWORD WINAPI WinHttpWebSocketClose( HINTERNET hsocket, USHORT status, void *reason, DWORD len ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/195
From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/tests/notification.c | 190 +++++++++++++++++++++++++++++- 1 file changed, 189 insertions(+), 1 deletion(-) diff --git a/dlls/winhttp/tests/notification.c b/dlls/winhttp/tests/notification.c index 50874a49a07..55823b42b85 100644 --- a/dlls/winhttp/tests/notification.c +++ b/dlls/winhttp/tests/notification.c @@ -727,6 +727,55 @@ static const struct notification websocket_test3[] = { 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, NF_MAIN_THREAD }, + { winhttp_websocket_close, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, NF_SAVE_BUFFER }, + { winhttp_websocket_close, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, + + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_CLOSING_CONNECTION, NF_WINE_ALLOW }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_CONNECTION_CLOSED, NF_WINE_ALLOW }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, +}; + +static struct notification websocket_test4[] = +{ + { winhttp_open_request, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_RESOLVING_NAME, NF_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_NAME_RESOLVED, NF_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTING_TO_SERVER }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE, NF_SIGNAL }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, + { 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, NF_SAVE_BUFFER }, + { winhttp_websocket_close, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, + + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_CLOSING_CONNECTION, NF_WINE_ALLOW }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_CONNECTION_CLOSED, NF_WINE_ALLOW }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, +}; + +static const struct notification websocket_test5[] = +{ + { winhttp_open_request, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_RESOLVING_NAME, NF_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_NAME_RESOLVED, NF_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTING_TO_SERVER }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE, NF_SIGNAL }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, + { 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_shutdown, WINHTTP_CALLBACK_STATUS_SHUTDOWN_COMPLETE, NF_MAIN_THREAD }, { winhttp_websocket_shutdown, WINHTTP_CALLBACK_STATUS_READ_COMPLETE, NF_SAVE_BUFFER | NF_SIGNAL }, { winhttp_websocket_close, WINHTTP_CALLBACK_STATUS_CLOSE_COMPLETE, @@ -1065,7 +1114,8 @@ static void test_websocket(BOOL secure) WaitForSingleObject( info.wait, INFINITE ); end_test( &info, __LINE__ ); - /* Test socket shutdown while receive is pending. */ + + /* Test socket handle close while web socket close is pending. */ info.test = websocket_test3; info.count = ARRAY_SIZE( websocket_test3 ); info.index = 0; @@ -1112,6 +1162,144 @@ static void test_websocket(BOOL secure) WaitForSingleObject( info.wait, INFINITE ); ok( buffer[0] == 'R', "unexpected data\n" ); + setup_test( &info, winhttp_websocket_close, __LINE__ ); + + err = pWinHttpWebSocketReceive( socket, buffer, sizeof(buffer), &size, &type ); + ok( err == ERROR_SUCCESS, "got %lu\n", err ); + + err = pWinHttpWebSocketClose( socket, 1000, (void *)"success", sizeof("success") ); + ok( err == ERROR_SUCCESS, "got %lu\n", err ); + + info.buflen = 0xdeadbeef; + WinHttpCloseHandle( socket ); + WaitForSingleObject( info.wait, INFINITE ); + + ok( info.buflen == sizeof(*result), "got %u\n", info.buflen ); + result = (WINHTTP_WEB_SOCKET_ASYNC_RESULT *)info.buffer; + ok( result->Operation == WINHTTP_WEB_SOCKET_CLOSE_OPERATION, "got %u\n", result->Operation ); + todo_wine ok( !result->AsyncResult.dwResult, "got %Iu\n", result->AsyncResult.dwResult ); + ok( result->AsyncResult.dwError == ERROR_WINHTTP_OPERATION_CANCELLED, "got %lu\n", result->AsyncResult.dwError ); + + setup_test( &info, winhttp_close_handle, __LINE__ ); + WinHttpCloseHandle( request ); + WaitForSingleObject( info.wait, INFINITE ); + end_test( &info, __LINE__ ); + + /* Test socket handle close while receive is pending. */ + info.test = websocket_test4; + info.count = ARRAY_SIZE( websocket_test4 ); + info.index = 0; + + setup_test( &info, winhttp_open_request, __LINE__ ); + request = WinHttpOpenRequest( connection, NULL, L"/", NULL, NULL, NULL, secure ? WINHTTP_FLAG_SECURE : 0); + ok( request != NULL, "got %lu\n", err ); + + if (secure) + { + flags = SECURITY_FLAG_IGNORE_UNKNOWN_CA | SECURITY_FLAG_IGNORE_CERT_DATE_INVALID | + SECURITY_FLAG_IGNORE_CERT_CN_INVALID; + ret = WinHttpSetOption(request, WINHTTP_OPTION_SECURITY_FLAGS, &flags, sizeof(flags)); + ok( ret, "failed to set security flags %lu\n", GetLastError() ); + } + + ret = WinHttpSetOption( request, WINHTTP_OPTION_UPGRADE_TO_WEB_SOCKET, NULL, 0 ); + ok( ret, "got %lu\n", GetLastError() ); + + setup_test( &info, winhttp_send_request, __LINE__ ); + ret = WinHttpSendRequest( request, NULL, 0, NULL, 0, 0, 0 ); + ok( ret, "got %lu\n", GetLastError() ); + WaitForSingleObject( info.wait, INFINITE ); + + setup_test( &info, winhttp_receive_response, __LINE__ ); + ret = WinHttpReceiveResponse( request, NULL ); + ok( ret, "got %lu\n", err ); + WaitForSingleObject( info.wait, INFINITE ); + + size = sizeof(status); + ret = WinHttpQueryHeaders( request, WINHTTP_QUERY_STATUS_CODE|WINHTTP_QUERY_FLAG_NUMBER, NULL, &status, &size, NULL ); + ok( ret, "failed unexpectedly %lu\n", err ); + ok( status == 101, "got %lu\n", status ); + + setup_test( &info, winhttp_websocket_complete_upgrade, __LINE__ ); + socket = pWinHttpWebSocketCompleteUpgrade( request, (DWORD_PTR)context ); + ok( socket != NULL, "got %lu\n", err ); + WaitForSingleObject( info.wait, INFINITE ); + + setup_test( &info, winhttp_websocket_receive, __LINE__ ); + buffer[0] = 0; + err = pWinHttpWebSocketReceive( socket, buffer, sizeof(buffer), &size, &type ); + ok( err == ERROR_SUCCESS, "got %lu\n", err ); + WaitForSingleObject( info.wait, INFINITE ); + ok( buffer[0] == 'R', "unexpected data\n" ); + + setup_test( &info, winhttp_websocket_close, __LINE__ ); + + info.buflen = 0xdeadbeef; + + err = pWinHttpWebSocketReceive( socket, buffer, sizeof(buffer), &size, &type ); + ok( err == ERROR_SUCCESS, "got %lu\n", err ); + + WinHttpCloseHandle( socket ); + WaitForSingleObject( info.wait, INFINITE ); + + ok( info.buflen == sizeof(*result), "got %u\n", info.buflen ); + result = (WINHTTP_WEB_SOCKET_ASYNC_RESULT *)info.buffer; + ok( result->Operation == WINHTTP_WEB_SOCKET_RECEIVE_OPERATION, "got %u\n", result->Operation ); + ok( !result->AsyncResult.dwResult, "got %Iu\n", result->AsyncResult.dwResult ); + ok( result->AsyncResult.dwError == ERROR_WINHTTP_OPERATION_CANCELLED, "got %lu\n", result->AsyncResult.dwError ); + + setup_test( &info, winhttp_close_handle, __LINE__ ); + WinHttpCloseHandle( request ); + WaitForSingleObject( info.wait, INFINITE ); + end_test( &info, __LINE__ ); + + /* Test socket shutdown while receive is pending. */ + info.test = websocket_test5; + info.count = ARRAY_SIZE( websocket_test5 ); + info.index = 0; + + setup_test( &info, winhttp_open_request, __LINE__ ); + request = WinHttpOpenRequest( connection, NULL, L"/", NULL, NULL, NULL, secure ? WINHTTP_FLAG_SECURE : 0); + ok( request != NULL, "got %lu\n", err ); + + if (secure) + { + flags = SECURITY_FLAG_IGNORE_UNKNOWN_CA | SECURITY_FLAG_IGNORE_CERT_DATE_INVALID | + SECURITY_FLAG_IGNORE_CERT_CN_INVALID; + ret = WinHttpSetOption(request, WINHTTP_OPTION_SECURITY_FLAGS, &flags, sizeof(flags)); + ok( ret, "failed to set security flags %lu\n", GetLastError() ); + } + + ret = WinHttpSetOption( request, WINHTTP_OPTION_UPGRADE_TO_WEB_SOCKET, NULL, 0 ); + ok( ret, "got %lu\n", GetLastError() ); + + setup_test( &info, winhttp_send_request, __LINE__ ); + ret = WinHttpSendRequest( request, NULL, 0, NULL, 0, 0, 0 ); + ok( ret, "got %lu\n", GetLastError() ); + WaitForSingleObject( info.wait, INFINITE ); + + setup_test( &info, winhttp_receive_response, __LINE__ ); + ret = WinHttpReceiveResponse( request, NULL ); + ok( ret, "got %lu\n", err ); + WaitForSingleObject( info.wait, INFINITE ); + + size = sizeof(status); + ret = WinHttpQueryHeaders( request, WINHTTP_QUERY_STATUS_CODE|WINHTTP_QUERY_FLAG_NUMBER, NULL, &status, &size, NULL ); + ok( ret, "failed unexpectedly %lu\n", err ); + ok( status == 101, "got %lu\n", status ); + + setup_test( &info, winhttp_websocket_complete_upgrade, __LINE__ ); + socket = pWinHttpWebSocketCompleteUpgrade( request, (DWORD_PTR)context ); + ok( socket != NULL, "got %lu\n", err ); + WaitForSingleObject( info.wait, INFINITE ); + + setup_test( &info, winhttp_websocket_receive, __LINE__ ); + buffer[0] = 0; + err = pWinHttpWebSocketReceive( socket, buffer, sizeof(buffer), &size, &type ); + ok( err == ERROR_SUCCESS, "got %lu\n", err ); + WaitForSingleObject( info.wait, INFINITE ); + ok( buffer[0] == 'R', "unexpected data\n" ); + err = pWinHttpWebSocketReceive( socket, buffer, sizeof(buffer), &size, &type ); ok( err == ERROR_SUCCESS, "got %lu\n", err ); err = pWinHttpWebSocketReceive( socket, buffer, sizeof(buffer), &size, &type ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/195
v2 changes: - Make queue cancelling behaviour in `socket_handle_closing() independent of connection type (always as in 'secure'); - Change (fix) the condition for aborting socket operations in socket_handle_closing(). It doesn't currently make any difference as web socket connections are not cached anyway but it looks cleaner and will make more sense if the same code is reused for normal winhttp connections; - Get rid of NF_OTHER_THREAD test flag introduced in v1 version and some of new NF_MAIN_THREAD flags. It turns out that most of the new tests which were using these flags are flaky, and now also we introduce the difference in Wine in this part by always taking 'secure' cancel style. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/195#note_1653
This merge request was approved by Hans Leidekker. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/195
participants (3)
-
Hans Leidekker (＠hans) -
Paul Gofman
-
Paul Gofman (＠gofman)