Signed-off-by: Paul Gofman pgofman@codeweavers.com --- Supersedes 225419.
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.
v2: - lock before incrementing and checking pending_sends, unlock after pending_sends is finalized (but before sending app callback). This way we are guaranteed that: - sync send is never attempted if async is queued; - attempt to execute next send will come to pending_sends increment only if the concurring send is complete or queued (so in the latter case that next send will be queued as well).
dlls/winhttp/request.c | 15 +++++++++++---- dlls/winhttp/winhttp_private.h | 1 + 2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 728b1895ba7..a3a6389a938 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; @@ -3391,6 +3392,7 @@ DWORD WINAPI WinHttpWebSocketSend( HINTERNET hsocket, WINHTTP_WEB_SOCKET_BUFFER_ return ERROR_OUTOFMEMORY; }
+ AcquireSRWLockExclusive( &socket->send_lock ); async_send = InterlockedIncrement( &socket->hdr.pending_sends ) > 1 || socket->hdr.recursion_count >= 3; if (!async_send) { @@ -3419,10 +3421,11 @@ DWORD WINAPI WinHttpWebSocketSend( HINTERNET hsocket, WINHTTP_WEB_SOCKET_BUFFER_ free( s ); } } - else + else InterlockedDecrement( &socket->hdr.pending_sends ); + ReleaseSRWLockExclusive( &socket->send_lock ); + if (!async_send) { TRACE("sent sync.\n"); - InterlockedDecrement( &socket->hdr.pending_sends ); free( s ); socket_send_complete( socket, ret, type, len ); ret = ERROR_SUCCESS; @@ -3535,6 +3538,7 @@ static DWORD socket_send_pong( struct socket *socket )
if (!(s = malloc( sizeof(*s) ))) return ERROR_OUTOFMEMORY;
+ AcquireSRWLockExclusive( &socket->send_lock ); async_send = InterlockedIncrement( &socket->hdr.pending_sends ) > 1; if (!async_send) { @@ -3563,6 +3567,7 @@ static DWORD socket_send_pong( struct socket *socket ) InterlockedDecrement( &socket->hdr.pending_sends ); free( s ); } + ReleaseSRWLockExclusive( &socket->send_lock ); return ret; } return send_frame( socket, SOCKET_OPCODE_PONG, 0, NULL, 0, TRUE, NULL ); @@ -3817,6 +3822,7 @@ static DWORD send_socket_shutdown( struct socket *socket, USHORT status, const v
if (!(s = malloc( sizeof(*s) ))) return FALSE;
+ AcquireSRWLockExclusive( &socket->send_lock ); async_send = InterlockedIncrement( &socket->hdr.pending_sends ) > 1 || socket->hdr.recursion_count >= 3; if (!async_send) { @@ -3845,9 +3851,10 @@ static DWORD send_socket_shutdown( struct socket *socket, USHORT status, const v free( s ); } } - else + else InterlockedDecrement( &socket->hdr.pending_sends ); + ReleaseSRWLockExclusive( &socket->send_lock ); + if (!async_send) { - InterlockedDecrement( &socket->hdr.pending_sends ); free( s ); if (send_callback) { 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