[PATCH v2 2/3] winhttp: Manage task cleanup in task_callback() and queue_task().

Paul Gofman pgofman at codeweavers.com
Tue Mar 15 11:07:24 CDT 2022


Signed-off-by: Paul Gofman <pgofman at codeweavers.com>
---
     v2:
          - don't release object in WinHttpWriteData on queue_task() error;
          - free task structure in caller on queue_task() error.

 dlls/winhttp/request.c         | 164 +++++++++++----------------------
 dlls/winhttp/winhttp_private.h |   9 +-
 2 files changed, 55 insertions(+), 118 deletions(-)

diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c
index 36a3bb909b9..a825ac71d33 100644
--- a/dlls/winhttp/request.c
+++ b/dlls/winhttp/request.c
@@ -151,9 +151,12 @@ static void CALLBACK task_callback( TP_CALLBACK_INSTANCE *instance, void *ctx, T
     struct task_header *task_hdr = ctx;
 
     task_hdr->callback( task_hdr );
+    release_object( task_hdr->obj );
+    free( task_hdr );
 }
 
-static DWORD queue_task( struct queue *queue, TASK_CALLBACK task, struct task_header *task_hdr )
+static DWORD queue_task( struct queue *queue, TASK_CALLBACK task, struct task_header *task_hdr,
+                         struct object_header *obj )
 {
     TP_WORK *work;
     DWORD ret;
@@ -163,6 +166,8 @@ static DWORD queue_task( struct queue *queue, TASK_CALLBACK task, struct task_he
     if (!(work = CreateThreadpoolWork( task_callback, task_hdr, &queue->env ))) return GetLastError();
     TRACE("queueing %p in %p\n", task_hdr, queue);
     task_hdr->callback = task;
+    task_hdr->obj = obj;
+    addref_object( obj );
     SubmitThreadpoolWork( work );
     CloseThreadpoolWork( work );
 
@@ -2178,13 +2183,12 @@ end:
 static void task_send_request( void *ctx )
 {
     struct send_request *s = ctx;
+    struct request *request = (struct request *)s->task_hdr.obj;
 
     TRACE( "running %p\n", ctx );
-    send_request( s->request, s->headers, s->headers_len, s->optional, s->optional_len, s->total_len, s->context, TRUE );
+    send_request( request, s->headers, s->headers_len, s->optional, s->optional_len, s->total_len, s->context, TRUE );
 
-    release_object( &s->request->hdr );
     free( s->headers );
-    free( s );
 }
 
 /***********************************************************************
@@ -2218,7 +2222,6 @@ BOOL WINAPI WinHttpSendRequest( HINTERNET hrequest, const WCHAR *headers, DWORD
         struct send_request *s;
 
         if (!(s = malloc( sizeof(*s) ))) return FALSE;
-        s->request      = request;
         s->headers      = strdupW( headers );
         s->headers_len  = headers_len;
         s->optional     = optional;
@@ -2226,10 +2229,8 @@ BOOL WINAPI WinHttpSendRequest( HINTERNET hrequest, const WCHAR *headers, DWORD
         s->total_len    = total_len;
         s->context      = context;
 
-        addref_object( &request->hdr );
-        if ((ret = queue_task( &request->queue, task_send_request, &s->task_hdr )))
+        if ((ret = queue_task( &request->queue, task_send_request, &s->task_hdr, &request->hdr )))
         {
-            release_object( &request->hdr );
             free( s->headers );
             free( s );
         }
@@ -2764,12 +2765,10 @@ static DWORD receive_response( struct request *request, BOOL async )
 static void task_receive_response( void *ctx )
 {
     struct receive_response *r = ctx;
+    struct request *request = (struct request *)r->task_hdr.obj;
 
     TRACE("running %p\n", ctx);
-    receive_response( r->request, TRUE );
-
-    release_object( &r->request->hdr );
-    free( r );
+    receive_response( request, TRUE );
 }
 
 /***********************************************************************
@@ -2799,14 +2798,9 @@ BOOL WINAPI WinHttpReceiveResponse( HINTERNET hrequest, LPVOID reserved )
         struct receive_response *r;
 
         if (!(r = malloc( sizeof(*r) ))) return FALSE;
-        r->request = request;
 
-        addref_object( &request->hdr );
-        if ((ret = queue_task( &request->queue, task_receive_response, &r->task_hdr )))
-        {
-            release_object( &request->hdr );
+        if ((ret = queue_task( &request->queue, task_receive_response, &r->task_hdr, &request->hdr )))
             free( r );
-        }
     }
     else ret = receive_response( request, FALSE );
 
@@ -2863,12 +2857,10 @@ done:
 static void task_query_data_available( void *ctx )
 {
     struct query_data *q = ctx;
+    struct request *request = (struct request *)q->task_hdr.obj;
 
     TRACE("running %p\n", ctx);
-    query_data_available( q->request, q->available, TRUE );
-
-    release_object( &q->request->hdr );
-    free( q );
+    query_data_available( request, q->available, TRUE );
 }
 
 /***********************************************************************
@@ -2899,16 +2891,12 @@ BOOL WINAPI WinHttpQueryDataAvailable( HINTERNET hrequest, LPDWORD available )
         struct query_data *q;
 
         if (!(q = malloc( sizeof(*q) ))) return FALSE;
-        q->request   = request;
         q->available = available;
 
-        addref_object( &request->hdr );
-        if ((ret = queue_task( &request->queue, task_query_data_available, &q->task_hdr )))
-        {
-            release_object( &request->hdr );
+        if ((ret = queue_task( &request->queue, task_query_data_available, &q->task_hdr, &request->hdr )))
             free( q );
-        }
-        else ret = ERROR_IO_PENDING;
+        else
+            ret = ERROR_IO_PENDING;
     }
     else ret = query_data_available( request, available, async );
 
@@ -2920,12 +2908,10 @@ BOOL WINAPI WinHttpQueryDataAvailable( HINTERNET hrequest, LPDWORD available )
 static void task_read_data( void *ctx )
 {
     struct read_data *r = ctx;
+    struct request *request = (struct request *)r->task_hdr.obj;
 
     TRACE("running %p\n", ctx);
-    read_data( r->request, r->buffer, r->to_read, r->read, TRUE );
-
-    release_object( &r->request->hdr );
-    free( r );
+    read_data( request, r->buffer, r->to_read, r->read, TRUE );
 }
 
 /***********************************************************************
@@ -2956,18 +2942,14 @@ BOOL WINAPI WinHttpReadData( HINTERNET hrequest, void *buffer, DWORD to_read, DW
         struct read_data *r;
 
         if (!(r = malloc( sizeof(*r) ))) return FALSE;
-        r->request = request;
         r->buffer  = buffer;
         r->to_read = to_read;
         r->read    = read;
 
-        addref_object( &request->hdr );
-        if ((ret = queue_task( &request->queue, task_read_data, &r->task_hdr )))
-        {
-            release_object( &request->hdr );
+        if ((ret = queue_task( &request->queue, task_read_data, &r->task_hdr, &request->hdr )))
             free( r );
-        }
-        else ret = ERROR_IO_PENDING;
+        else
+            ret = ERROR_IO_PENDING;
     }
     else ret = read_data( request, buffer, to_read, read, async );
 
@@ -3001,12 +2983,10 @@ static DWORD write_data( struct request *request, const void *buffer, DWORD to_w
 static void task_write_data( void *ctx )
 {
     struct write_data *w = ctx;
+    struct request *request = (struct request *)w->task_hdr.obj;
 
     TRACE("running %p\n", ctx);
-    write_data( w->request, w->buffer, w->to_write, w->written, TRUE );
-
-    release_object( &w->request->hdr );
-    free( w );
+    write_data( request, w->buffer, w->to_write, w->written, TRUE );
 }
 
 /***********************************************************************
@@ -3036,17 +3016,12 @@ BOOL WINAPI WinHttpWriteData( HINTERNET hrequest, const void *buffer, DWORD to_w
         struct write_data *w;
 
         if (!(w = malloc( sizeof(*w) ))) return FALSE;
-        w->request  = request;
         w->buffer   = buffer;
         w->to_write = to_write;
         w->written  = written;
 
-        addref_object( &request->hdr );
-        if ((ret = queue_task( &request->queue, task_write_data, &w->task_hdr )))
-        {
-            release_object( &request->hdr );
+        if ((ret = queue_task( &request->queue, task_write_data, &w->task_hdr, &request->hdr )))
             free( w );
-        }
     }
     else ret = write_data( request, buffer, to_write, written, FALSE );
 
@@ -3403,19 +3378,17 @@ static DWORD socket_send( struct socket *socket, WINHTTP_WEB_SOCKET_BUFFER_TYPE
 static void task_socket_send( void *ctx )
 {
     struct socket_send *s = ctx;
+    struct socket *socket = (struct socket *)s->task_hdr.obj;
     DWORD ret;
 
     TRACE("running %p\n", ctx);
 
-    if (s->complete_async) ret = complete_send_frame( s->socket, &s->ovr, s->buf );
-    else                   ret = socket_send( s->socket, s->type, s->buf, s->len, NULL );
+    if (s->complete_async) ret = complete_send_frame( socket, &s->ovr, s->buf );
+    else                   ret = socket_send( socket, s->type, s->buf, s->len, NULL );
 
-    send_io_complete( &s->socket->hdr );
-    InterlockedExchange( &s->socket->pending_noncontrol_send, 0 );
-    socket_send_complete( s->socket, ret, s->type, s->len );
-
-    release_object( &s->socket->hdr );
-    free( s );
+    send_io_complete( &socket->hdr );
+    InterlockedExchange( &socket->pending_noncontrol_send, 0 );
+    socket_send_complete( socket, ret, s->type, s->len );
 }
 
 DWORD WINAPI WinHttpWebSocketSend( HINTERNET hsocket, WINHTTP_WEB_SOCKET_BUFFER_TYPE type, void *buf, DWORD len )
@@ -3481,21 +3454,14 @@ DWORD WINAPI WinHttpWebSocketSend( HINTERNET hsocket, WINHTTP_WEB_SOCKET_BUFFER_
         {
             s->complete_async = complete_async;
             TRACE("queueing, complete_async %#x.\n", complete_async);
-            s->socket = socket;
             s->type   = type;
             s->buf    = buf;
             s->len    = len;
 
-            addref_object( &socket->hdr );
-            if ((ret = queue_task( &socket->send_q, task_socket_send, &s->task_hdr )))
-            {
-                InterlockedDecrement( &socket->hdr.pending_sends );
-                InterlockedExchange( &socket->pending_noncontrol_send, 0 );
-                release_object( &socket->hdr );
+            if ((ret = queue_task( &socket->send_q, task_socket_send, &s->task_hdr, &socket->hdr )))
                 free( s );
-            }
         }
-        else
+        if (!async_send || ret)
         {
             InterlockedDecrement( &socket->hdr.pending_sends );
             InterlockedExchange( &socket->pending_noncontrol_send, 0 );
@@ -3607,16 +3573,14 @@ static DWORD receive_frame( struct socket *socket, DWORD *ret_len, enum socket_o
 static void task_socket_send_pong( void *ctx )
 {
     struct socket_send *s = ctx;
+    struct socket *socket = (struct socket *)s->task_hdr.obj;
 
     TRACE("running %p\n", ctx);
 
-    if (s->complete_async) complete_send_frame( s->socket, &s->ovr, NULL );
-    else                   send_frame( s->socket, SOCKET_OPCODE_PONG, 0, NULL, 0, TRUE, NULL );
-
-    send_io_complete( &s->socket->hdr );
+    if (s->complete_async) complete_send_frame( socket, &s->ovr, NULL );
+    else                   send_frame( socket, SOCKET_OPCODE_PONG, 0, NULL, 0, TRUE, NULL );
 
-    release_object( &s->socket->hdr );
-    free( s );
+    send_io_complete( &socket->hdr );
 }
 
 static DWORD socket_send_pong( struct socket *socket )
@@ -3644,12 +3608,9 @@ static DWORD socket_send_pong( struct socket *socket )
         if (async_send)
         {
             s->complete_async = complete_async;
-            s->socket = socket;
-            addref_object( &socket->hdr );
-            if ((ret = queue_task( &socket->send_q, task_socket_send_pong, &s->task_hdr )))
+            if ((ret = queue_task( &socket->send_q, task_socket_send_pong, &s->task_hdr, &socket->hdr )))
             {
                 InterlockedDecrement( &socket->hdr.pending_sends );
-                release_object( &socket->hdr );
                 free( s );
             }
         }
@@ -3828,20 +3789,21 @@ static DWORD socket_receive( struct socket *socket, void *buf, DWORD len, DWORD
 static void task_socket_receive( void *ctx )
 {
     struct socket_receive *r = ctx;
+    struct socket *socket = (struct socket *)r->task_hdr.obj;
     DWORD ret, count;
     WINHTTP_WEB_SOCKET_BUFFER_TYPE type;
 
     TRACE("running %p\n", ctx);
-    ret = socket_receive( r->socket, r->buf, r->len, &count, &type );
+    ret = socket_receive( socket, r->buf, r->len, &count, &type );
 
-    if (receive_io_complete( r->socket ))
+    if (receive_io_complete( 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) );
+            send_callback( &socket->hdr, WINHTTP_CALLBACK_STATUS_READ_COMPLETE, &status, sizeof(status) );
         }
         else
         {
@@ -3849,12 +3811,9 @@ static void task_socket_receive( void *ctx )
             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) );
+            send_callback( &socket->hdr, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, &result, sizeof(result) );
         }
     }
-
-    release_object( &r->socket->hdr );
-    free( r );
 }
 
 DWORD WINAPI WinHttpWebSocketReceive( HINTERNET hsocket, void *buf, DWORD len, DWORD *ret_len,
@@ -3896,15 +3855,12 @@ DWORD WINAPI WinHttpWebSocketReceive( HINTERNET hsocket, void *buf, DWORD len, D
             InterlockedDecrement( &socket->hdr.pending_receives );
             return ERROR_OUTOFMEMORY;
         }
-        r->socket = socket;
         r->buf    = buf;
         r->len    = len;
 
-        addref_object( &socket->hdr );
-        if ((ret = queue_task( &socket->recv_q, task_socket_receive, &r->task_hdr )))
+        if ((ret = queue_task( &socket->recv_q, task_socket_receive, &r->task_hdr, &socket->hdr )))
         {
             InterlockedDecrement( &socket->hdr.pending_receives );
-            release_object( &socket->hdr );
             free( r );
         }
     }
@@ -3930,17 +3886,16 @@ static void socket_shutdown_complete( struct socket *socket, DWORD ret )
 static void task_socket_shutdown( void *ctx )
 {
     struct socket_shutdown *s = ctx;
+    struct socket *socket = (struct socket *)s->task_hdr.obj;
     DWORD ret;
 
     TRACE("running %p\n", ctx);
 
-    if (s->complete_async) ret = complete_send_frame( s->socket, &s->ovr, s->reason );
-    else                   ret = send_frame( s->socket, SOCKET_OPCODE_CLOSE, s->status, s->reason, s->len, TRUE, NULL );
+    if (s->complete_async) ret = complete_send_frame( socket, &s->ovr, s->reason );
+    else                   ret = send_frame( socket, SOCKET_OPCODE_CLOSE, s->status, s->reason, s->len, TRUE, NULL );
 
-    send_io_complete( &s->socket->hdr );
-    if (s->send_callback) socket_shutdown_complete( s->socket, ret );
-    release_object( &s->socket->hdr );
-    free( s );
+    send_io_complete( &socket->hdr );
+    if (s->send_callback) socket_shutdown_complete( socket, ret );
 }
 
 static DWORD send_socket_shutdown( struct socket *socket, USHORT status, const void *reason, DWORD len,
@@ -3972,17 +3927,14 @@ static DWORD send_socket_shutdown( struct socket *socket, USHORT status, const v
         if (async_send)
         {
             s->complete_async = complete_async;
-            s->socket = socket;
             s->status = status;
             memcpy( s->reason, reason, len );
             s->len    = len;
             s->send_callback = send_callback;
 
-            addref_object( &socket->hdr );
-            if ((ret = queue_task( &socket->send_q, task_socket_shutdown, &s->task_hdr )))
+            if ((ret = queue_task( &socket->send_q, task_socket_shutdown, &s->task_hdr, &socket->hdr )))
             {
                 InterlockedDecrement( &socket->hdr.pending_sends );
-                release_object( &socket->hdr );
                 free( s );
             }
         }
@@ -4068,15 +4020,13 @@ static void socket_close_complete( struct socket *socket, DWORD ret )
 static void task_socket_close( void *ctx )
 {
     struct socket_shutdown *s = ctx;
+    struct socket *socket = (struct socket *)s->task_hdr.obj;
     DWORD ret;
 
     TRACE("running %p\n", ctx);
 
-    ret = socket_close( s->socket );
-    socket_close_complete( s->socket, ret );
-
-    release_object( &s->socket->hdr );
-    free( s );
+    ret = socket_close( socket );
+    socket_close_complete( socket, ret );
 }
 
 DWORD WINAPI WinHttpWebSocketClose( HINTERNET hsocket, USHORT status, void *reason, DWORD len )
@@ -4137,14 +4087,8 @@ DWORD WINAPI WinHttpWebSocketClose( HINTERNET hsocket, USHORT status, void *reas
         struct socket_shutdown *s;
 
         if (!(s = calloc( 1, sizeof(*s) ))) return FALSE;
-        s->socket = socket;
-
-        addref_object( &socket->hdr );
-        if ((ret = queue_task( &socket->recv_q, task_socket_close, &s->task_hdr )))
-        {
-            release_object( &socket->hdr );
+        if ((ret = queue_task( &socket->recv_q, task_socket_close, &s->task_hdr, &socket->hdr )))
             free( s );
-        }
     }
     else ret = socket_close( socket );
 
diff --git a/dlls/winhttp/winhttp_private.h b/dlls/winhttp/winhttp_private.h
index 96bd7b53747..35f3e64b5a3 100644
--- a/dlls/winhttp/winhttp_private.h
+++ b/dlls/winhttp/winhttp_private.h
@@ -278,12 +278,12 @@ typedef void (*TASK_CALLBACK)( void *ctx );
 struct task_header
 {
     TASK_CALLBACK callback;
+    struct object_header *obj;
 };
 
 struct send_request
 {
     struct task_header task_hdr;
-    struct request *request;
     WCHAR *headers;
     DWORD headers_len;
     void *optional;
@@ -295,20 +295,17 @@ struct send_request
 struct receive_response
 {
     struct task_header task_hdr;
-    struct request *request;
 };
 
 struct query_data
 {
     struct task_header task_hdr;
-    struct request *request;
     DWORD *available;
 };
 
 struct read_data
 {
     struct task_header task_hdr;
-    struct request *request;
     void *buffer;
     DWORD to_read;
     DWORD *read;
@@ -317,7 +314,6 @@ struct read_data
 struct write_data
 {
     struct task_header task_hdr;
-    struct request *request;
     const void *buffer;
     DWORD to_write;
     DWORD *written;
@@ -326,7 +322,6 @@ struct write_data
 struct socket_send
 {
     struct task_header task_hdr;
-    struct socket *socket;
     WINHTTP_WEB_SOCKET_BUFFER_TYPE type;
     const void *buf;
     DWORD len;
@@ -337,7 +332,6 @@ struct socket_send
 struct socket_receive
 {
     struct task_header task_hdr;
-    struct socket *socket;
     void *buf;
     DWORD len;
 };
@@ -345,7 +339,6 @@ struct socket_receive
 struct socket_shutdown
 {
     struct task_header task_hdr;
-    struct socket *socket;
     USHORT status;
     char reason[123];
     DWORD len;
-- 
2.35.1




More information about the wine-devel mailing list