Signed-off-by: Paul Gofman pgofman@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
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@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
-
Hans Leidekker -
Paul Gofman