[PATCH] winhttp: Use lock in send_frame().

Paul Gofman pgofman at codeweavers.com
Wed Feb 2 06:03:13 CST 2022


Eh, I am sorry, I realized that the patch probably doesn't really fully 
solve the described possible race between queued and immediate send. I 
will revise it and send another version. It should probably be locked 
before incrementing the counter.

On 2/2/22 11:53, Paul Gofman wrote:
> Signed-off-by: Paul Gofman <pgofman at codeweavers.com>
> ---
>       While web socket sends are not actually supposed to be queued before the previous one completed, send for pongs
>       or socket shutdown may be called in parallel. Only the first call will go for synchronous send attempt but the
>       next queued call have a chance of hitting socket_send() before the first sync one returns.
>       The sync outside of send_frame() (which is needed if the send wasn't completed synchronously) is provided by
>       the queueing logic around pending_sends counter.
>
>   dlls/winhttp/request.c         | 12 +++++++++---
>   dlls/winhttp/winhttp_private.h |  1 +
>   2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c
> index 728b1895ba7..756b40d8987 100644
> --- a/dlls/winhttp/request.c
> +++ b/dlls/winhttp/request.c
> @@ -3111,6 +3111,7 @@ HINTERNET WINAPI WinHttpWebSocketCompleteUpgrade( HINTERNET hrequest, DWORD_PTR
>       socket->hdr.callback = request->hdr.callback;
>       socket->hdr.notify_mask = request->hdr.notify_mask;
>       socket->hdr.context = context;
> +    InitializeSRWLock( &socket->send_lock );
>   
>       addref_object( &request->hdr );
>       socket->request = request;
> @@ -3153,6 +3154,8 @@ static DWORD send_frame( struct socket *socket, enum socket_opcode opcode, USHOR
>   
>       TRACE( "sending %02x frame, len %u.\n", opcode, len );
>   
> +    AcquireSRWLockExclusive( &socket->send_lock );
> +
>       if (opcode == SOCKET_OPCODE_CLOSE) len += sizeof(status);
>   
>       hdr[0] = final ? (char)FIN_BIT : 0;
> @@ -3188,7 +3191,8 @@ static DWORD send_frame( struct socket *socket, enum socket_opcode opcode, USHOR
>           if (!(new = realloc( socket->send_frame_buffer, new_size )))
>           {
>               ERR("Out of memory, buffer_size %u.\n", buffer_size);
> -            return ERROR_OUTOFMEMORY;
> +            ret = ERROR_OUTOFMEMORY;
> +            goto done;
>           }
>           socket->send_frame_buffer = new;
>           socket->send_frame_buffer_size = new_size;
> @@ -3231,13 +3235,15 @@ static DWORD send_frame( struct socket *socket, enum socket_opcode opcode, USHOR
>                   memmove( socket->send_frame_buffer, socket->send_frame_buffer + sent_size, offset - sent_size );
>                   socket->bytes_in_send_frame_buffer = offset - sent_size;
>               }
> -            return ret;
> +            goto done;
>           }
>           assert( sent_size == offset );
>           offset = 0;
>           buflen -= len;
>       }
> -    return ERROR_SUCCESS;
> +done:
> +    ReleaseSRWLockExclusive( &socket->send_lock );
> +    return ret;
>   }
>   
>   static DWORD complete_send_frame( struct socket *socket, WSAOVERLAPPED *ovr, const char *buf )
> diff --git a/dlls/winhttp/winhttp_private.h b/dlls/winhttp/winhttp_private.h
> index 3e4e1eb298d..732f0afaa88 100644
> --- a/dlls/winhttp/winhttp_private.h
> +++ b/dlls/winhttp/winhttp_private.h
> @@ -259,6 +259,7 @@ struct socket
>       unsigned int send_remaining_size;
>       unsigned int bytes_in_send_frame_buffer;
>       unsigned int client_buffer_offset;
> +    SRWLOCK send_lock;
>   };
>   
>   struct send_request





More information about the wine-devel mailing list