From f0d5db5c63bd387b4880703c98ca02c4e532b743 Mon Sep 17 00:00:00 2001 From: Alvin Wong Date: Tue, 8 Nov 2022 23:05:22 +0800 Subject: [PATCH] Try to avoid random crash with disconnected.cpp test There are some test cases which tests throwing an hresult_error from the completed handler. This relies on `winrt::impl::invoke` catching the exception and handling it using `winrt::to_hresult`. In some test cases, the completed handler may run on a worker thread, so the test tries to wait for the handler to be called using an event, but it does not wait until `winrt::impl::invoke` has finished handling the exception. An extremely rare race condition may happen: 1. The completed handler on a worker thread sets the event. 2. The test case, having finished waiting for the event on the main thread, leaves the test case. 3. Another test case, `custom_error`, starts on the main thread and sets `winrt_to_hresult_handler` to a custom handler 4. The completed handler, which is still running on the worker thread, throws the `hresult_error` exception. 5. `winrt::impl::invoke` catches the exception, then tries to handle it with `winrt::to_hresult`. 6. `winrt::to_hresult` notices the custom `winrt_to_hresult_handler` from the `custom_error` test and calls it, instead of going through the usual code flow that is expected by the original test case. 7. The `winrt_to_hresult_handler` doesn't know how to handle `hresult_error`, so it ends up unhandled and terminates the program. This is a real crash I have got when testing a mingw build with some tests excluded, which triggered this exact sequence of events. In theory this can also happen when testing with `--order rand` to randomize test order if you are very unlucky. I cannot think of a way to really wait for the completed handler to finish, so I added a 500ms sleep at the end of the test to hopefully minimize the chance of this happening again. --- test/test/disconnected.cpp | 4 ++++ test/test_win7/disconnected.cpp | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/test/test/disconnected.cpp b/test/test/disconnected.cpp index 816344c35..ac97f8477 100644 --- a/test/test/disconnected.cpp +++ b/test/test/disconnected.cpp @@ -105,6 +105,8 @@ TEST_CASE("disconnected,handler,3") }); WaitForSingleObject(signal.get(), INFINITE); + // Give some time for to_hresult() to complete. + Sleep(500); } TEST_CASE("disconnected,handler,4") @@ -134,6 +136,8 @@ TEST_CASE("disconnected,handler,5") }); WaitForSingleObject(signal.get(), INFINITE); + // Give some time for to_hresult() to complete. + Sleep(500); } // Custom action to simulate an out-of-process server that crashes before it can complete. diff --git a/test/test_win7/disconnected.cpp b/test/test_win7/disconnected.cpp index 17411e99e..1757937d0 100644 --- a/test/test_win7/disconnected.cpp +++ b/test/test_win7/disconnected.cpp @@ -94,6 +94,8 @@ TEST_CASE("disconnected,3") }); WaitForSingleObject(signal.get(), INFINITE); + // Give some time for to_hresult() to complete. + Sleep(500); } TEST_CASE("disconnected,4") @@ -123,4 +125,6 @@ TEST_CASE("disconnected,5") }); WaitForSingleObject(signal.get(), INFINITE); + // Give some time for to_hresult() to complete. + Sleep(500); }