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

Hans Leidekker hans at codeweavers.com
Tue Mar 15 10:37:23 CDT 2022


> On 3/15/22 18:26, Hans Leidekker wrote:
> > On Mon, 2022-03-14 at 20:04 +0300, Paul Gofman wrote:
> >> diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c
> >> index 36a3bb909b9..f98f5f30c2a 100644
> >> --- a/dlls/winhttp/request.c
> >> +++ b/dlls/winhttp/request.c
> >> @@ -151,18 +151,27 @@ 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;
> >>   
> >>       if ((ret = start_queue( queue ))) return ret;
> >>   
> >> -    if (!(work = CreateThreadpoolWork( task_callback, task_hdr, &queue->env ))) return GetLastError();
> >> +    if (!(work = CreateThreadpoolWork( task_callback, task_hdr, &queue->env )))
> >> +    {
> >> +        free( task_hdr );
> >> +        return GetLastError();
> >> +    }
> > You missed freeing task_hdr when start_queue() fails, though I'd prefer freeing
> > it in the callers. Then you also wouldn't need that headers_dup variable in
> > WinHttpSendRequest().
> 
> Do you mean always leaving the free on queue_task() failure where it is 
> before the patch?

Yes.

> You signed off the last patch, do you mean I can just resend this one?

Yes, it looks good.





More information about the wine-devel mailing list