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.