Re: [PATCH v4 2/2] ws2_32/tests: Add tests for terminated thread asyncs completion.

[Zebediah Figura]

On 6/6/22 16:55, Paul Gofman wrote:
> - /* asyncs without completion port are always cancelled on thread exit. */
> - for (i = 0; i < ARRAY_SIZE(tests_no_port); ++i)
> - {
> -
>    winetest_push_context("test %u", i);
> -
>    memset(&io, 0xcc, sizeof(io));
> -
>    ResetEvent(event);
> -
>    ret = thread_NtDeviceIoControlFile(i, (HANDLE)listener, tests_no_port[i].event ? event : NULL,

Shouldn't "i" here be "tests_no_port[i].kill_thread"?
> -
>            tests_no_port[i].apc, tests_no_port[i].apc_context, &io,
> -
>            IOCTL_AFD_POLL, in_params, params_size, out_params, params_size);
> -
>    ok(ret == STATUS_PENDING, "got %#x\n", ret);
> -
>    ok(io.Status == STATUS_CANCELLED, "got %#lx\n", io.Status);
> -
>    if (tests_no_port[i].event)
> -
>    {
> -
>        ret = WaitForSingleObject(event, 1000);
> -
>        ok(!ret, "got %#x\n", ret);
> -
>    }
> -
>    winetest_pop_context();
> - }
> -
> - SleepEx(0, TRUE);
> - ok(!test_async_thread_termination_apc_count, "got APC.\n");
> -
> - port = CreateIoCompletionPort((HANDLE)listener, NULL, 0, 0);
> -
> - for (i = 0; i < 2; ++i)
> - {
> -
>    winetest_push_context("test %u", i);
> -
>    memset(&io, 0xcc, sizeof(io));
> -
>    ResetEvent(event);
> -
>    ret = thread_NtDeviceIoControlFile(i, (HANDLE)listener, event, NULL, (void *)0xdeadbeef, &io,
> -
>            IOCTL_AFD_POLL, in_params, params_size, out_params, params_size);
> -
>    ok(ret == STATUS_PENDING, "got %#x\n", ret);
> -
>    ok(io.Status == STATUS_CANCELLED, "got %#lx\n", io.Status);
> -
>    ret = WaitForSingleObject(event, 1000);
> -
>    ok(!ret, "got %#x\n", ret);
> -
> -
>    memset(&io, 0xcc, sizeof(io));
> -
>    key = 0xcc;
> -
>    value = 0;
> -
>    ret = NtRemoveIoCompletion(port, &key, &value, &io, &zero);
> -
>    ok(!ret, "got %#x\n", ret);
> -
>    ok(!key, "got key %#Ix\n", key);
> -
>    ok(value == 0xdeadbeef, "got value %#Ix\n", value);
> -
>    ok(io.Status == STATUS_CANCELLED, "got %#lx\n", io.Status);
> -
>    winetest_pop_context();
> - }
> -
> - for (i = 0; i < 2; ++i)
> - {
> -
>    winetest_push_context("test %u", i);
> -
>    memset(&io, 0xcc, sizeof(io));
> -
>    ResetEvent(event);
> -
>    ret = thread_NtDeviceIoControlFile(i, (HANDLE)listener, event, NULL, NULL, &io,
> -
>            IOCTL_AFD_POLL, in_params, params_size, out_params, params_size);
> -
>    ok(ret == STATUS_PENDING, "got %#x\n", ret);
> -
>    ok(io.Status == STATUS_CANCELLED, "got %#lx\n", io.Status);
> -
> -
>    ret = NtRemoveIoCompletion(port, &key, &value, &io, &zero);
> -
>    ok(ret == WAIT_TIMEOUT, "got %#x\n", ret);
> -
>    winetest_pop_context();
> - }
> -
> - for (i = 0; i < 2; ++i)
> - {
> -
>    winetest_push_context("test %u", i);
> -
>    memset(&io, 0xcc, sizeof(io));
> -
>    ret = thread_NtDeviceIoControlFile(i, (HANDLE)listener, NULL, NULL, NULL, &io,
> -
>            IOCTL_AFD_POLL, in_params, params_size, out_params, params_size);
> -
>    ok(ret == STATUS_PENDING, "got %#x\n", ret);
> -
>    ok(io.Status == STATUS_CANCELLED, "got %#lx\n", io.Status);
> -
> -
>    ret = NtRemoveIoCompletion(port, &key, &value, &io, &zero);
> -
>    ok(ret == WAIT_TIMEOUT, "got %#x\n", ret);
> -
>    winetest_pop_context();
> - }
> -
> - /* async is not cancelled if there is a completion port and no event. */
> - for (i = 0; i < 2; ++i)
> - {
> -
>    winetest_push_context("test %u", i);
> -
>    memset(&io, 0xcc, sizeof(io));
> -
>    ret = thread_NtDeviceIoControlFile(i, (HANDLE)listener, NULL, NULL, (void *)0xdeadbeef, &io,
> -
>            IOCTL_AFD_POLL, in_params, params_size, out_params, params_size);
> -
>    ok(ret == STATUS_PENDING, "got %#x\n", ret);
> -
>    ok(io.Status == 0xcccccccc, "got %#lx\n", io.Status);
> -
> -
>    memset(&io, 0xcc, sizeof(io));
> -
>    key = 0xcc;
> -
>    value = 0;
> -
>    ret = NtRemoveIoCompletion(port, &key, &value, &io, &zero);
> -
>    ok(ret == WAIT_TIMEOUT, "got %#x\n", ret);
> -
>    CancelIoEx((HANDLE)listener, NULL);
> -
>    ret = NtRemoveIoCompletion(port, &key, &value, &io, &zero);
> -
>    ok(!ret, "got %#x\n", ret);
> -
>    ok(!key, "got key %#Ix\n", key);
> -
>    ok(value == 0xdeadbeef, "got value %#Ix\n", value);
> -
>    ok(io.Status == STATUS_CANCELLED, "got %#lx\n", io.Status);
> -
>    winetest_pop_context();
> - }

FWIW, in cases like this I often find it preferable to iterate over the 
same loop but do something like

if (tests[i].apc_context && !tests[i].event)
     {
         ok(io.Status == 0xcccccccc, "got %#lx\n", io.Status);
         /* other tests... */
     }
     else
     {
         ok(io.Status == STATUS_CANCELLED, "got %#lx\n", io.Status);
         /* other tests... */
     }

Just something to consider.