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);
On 01.08.2017 18:52, Sebastian Lackner wrote:
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.
It's not clear for me if it's supposed to be guaranteed, but it surely won't hurt. I will change it.
- 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.
TrySubmitThreadpoolCallback() looks like what I need, I will change it.
Thanks, Jacek