[PATCH 5/6] ws2_32: Added support for overlapped GetAddrInfoExW.

[Sebastian Lackner]

On 01.08.2017 18:29, Jacek Caban wrote:
> +static void WINAPI getaddrinfo_callback(TP_CALLBACK_INSTANCE *instance, void *context, TP_WORK *work)
> +{
> +    struct getaddrinfo_args *args = context;
> +    OVERLAPPED *overlapped = args->overlapped;
> +    struct WS_addrinfo *res;
> +
> +    overlapped->Internal = WS_getaddrinfo(args->nodename, args->servname, NULL, &res);

What if the app does busy-looping instead of using the event, and expects the result to
be set after overlapped->Internal changed?

My understanding is that setting overlapped->Internal should be the very last thing in
the function, the app could decide to deallocate / reuse the memory after the async
operation has finished.

> +    if (res)
> +    {
> +        *args->result = addrinfo_list_AtoW(res);
> +        overlapped->u.Pointer = args->result;
> +        WS_freeaddrinfo(res);
> +    }
>[...]
> +        tp_work = CreateThreadpoolWork(getaddrinfo_callback, args, NULL);
> +        if (!tp_work)
> +        {
> +            HeapFree(GetProcessHeap(), 0, args);
> +            ret = GetLastError();
> +            goto end;
> +        }
>  
> +        overlapped->Internal = WSAEINPROGRESS;
> +        SubmitThreadpoolWork(tp_work);

I don't see a CloseThreadpoolWork call or cleanup group anywhere, so there is likely a
leak in this code. Also, when all callbacks have different arguments, it would be better
to use TrySubmitThreadpoolCallback(). Work callbacks are typically used when the same
task is queued multiple times.

> +        if (local_nodenameW != nodename)
> +            HeapFree(GetProcessHeap(), 0, local_nodenameW);
> +        WSASetLastError(ERROR_IO_PENDING);
> +        return ERROR_IO_PENDING;
> +    }
> +
> +    ret = WS_getaddrinfo(nodenameA, servnameA, hints, &resA);