Skip to content

Commit 5790b6a

Browse files
[SYCL] fix for flaky ~event failure in Win unit tests (#19762)
Windows shutdown/teardown doesn't have the exact same timing as on Linux, and even more in the the unit tests which are statically compiled ( so no DllMain() call, which means no space between the calls to `shutdown_early()` and `shutdown_late()` ). Here we remove a race between the platform deletion in the mock and the win shutdown. This fixes a flaky failure we are seeing in a couple of the unit tests. --------- Signed-off-by: Chris Perkins <[email protected]>
1 parent 47a172b commit 5790b6a

File tree

2 files changed

+20
-8
lines changed

2 files changed

+20
-8
lines changed

sycl/unittests/context_device/DeviceRefCounter.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
66
//
77
//===----------------------------------------------------------------------===//
8+
#include <detail/global_handler.hpp>
89
#include <gtest/gtest.h>
910
#include <helpers/UrMock.hpp>
1011
#include <sycl/sycl.hpp>
@@ -31,6 +32,7 @@ static ur_result_t redefinedDeviceReleaseAfter(void *) {
3132
TEST(DevRefCounter, DevRefCounter) {
3233
{
3334
sycl::unittest::UrMock<> Mock;
35+
EXPECT_EQ(DevRefCounter, 0);
3436

3537
mock::getCallbacks().set_after_callback("urDeviceGet",
3638
&redefinedDevicesGetAfter);
@@ -41,6 +43,12 @@ TEST(DevRefCounter, DevRefCounter) {
4143
sycl::platform Plt = sycl::platform();
4244

4345
Plt.get_devices();
46+
EXPECT_NE(DevRefCounter, 0);
47+
// This is the behavior that SYCL performs at shutdown, but there
48+
// are timing differences Lin/Win and shared/static that make
49+
// it not map correctly into our mock.
50+
// So for this test, we just do it.
51+
sycl::detail::GlobalHandler::instance().getPlatformCache().clear();
4452
}
4553
EXPECT_EQ(DevRefCounter, 0);
4654
}

sycl/unittests/helpers/UrMock.hpp

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,16 @@ template <sycl::backend Backend = backend::opencl> class UrMock {
596596

597597
sycl::detail::ur::initializeUr(UrLoaderConfig);
598598
urLoaderConfigRelease(UrLoaderConfig);
599+
600+
// We clear platform cache for each test run, so that tests can have a
601+
// different backend. This forces platforms to be reconstructed (and thus
602+
// queries about UR backend info to be called again) This also erases each
603+
// platform's devices (normally done in the library shutdown) so that
604+
// platforms/devices' lifetimes could work in unittests scenario. In SYCL,
605+
// this is normally done at shutdown, but between Win/Lin and static/shared
606+
// differences, there is no correct parallel in the ~UrMock destructor.
607+
// Instead we do it here. Simple and clean.
608+
detail::GlobalHandler::instance().getPlatformCache().clear();
599609
}
600610

601611
UrMock(UrMock<Backend> &&Other) = delete;
@@ -606,14 +616,8 @@ template <sycl::backend Backend = backend::opencl> class UrMock {
606616
// these between tests
607617
detail::GlobalHandler::instance().prepareSchedulerToRelease(true);
608618
detail::GlobalHandler::instance().releaseDefaultContexts();
609-
// clear platform cache in case subsequent tests want a different backend,
610-
// this forces platforms to be reconstructed (and thus queries about UR
611-
// backend info to be called again)
612-
//
613-
// This also erases each platform's devices (normally done in the library
614-
// shutdown) so that platforms/devices' lifetimes could work in unittests
615-
// scenario.
616-
detail::GlobalHandler::instance().getPlatformCache().clear();
619+
// the platform cache is cleared at the BEGINING of the mock.
620+
617621
mock::getCallbacks().resetCallbacks();
618622
}
619623

0 commit comments

Comments
 (0)