From 864cc755b07f5f06e4a02b0e188e41e5d4544289 Mon Sep 17 00:00:00 2001 From: Vlada Kanivets Date: Mon, 18 Aug 2025 14:30:00 +0200 Subject: [PATCH 1/2] add tests --- include/kf/ConditionVariable.h | 12 +- test/CMakeLists.txt | 1 + test/ConditionVariableTest.cpp | 292 +++++++++++++++++++++++++++++++++ 3 files changed, 301 insertions(+), 4 deletions(-) create mode 100644 test/ConditionVariableTest.cpp diff --git a/include/kf/ConditionVariable.h b/include/kf/ConditionVariable.h index 0e79e20..af2fe49 100644 --- a/include/kf/ConditionVariable.h +++ b/include/kf/ConditionVariable.h @@ -1,5 +1,5 @@ #pragma once - +#include #include "EResource.h" #include "Semaphore.h" @@ -39,13 +39,17 @@ namespace kf { ++m_waitersCount; - external.release(); - auto status = NT_SUCCESS(m_semaphore.wait(timeout)) ? Status::Success : Status::Timeout; + if (external.isAcquiredExclusive()) + { + external.release(); + } + auto status = m_semaphore.wait(timeout); + auto result = (NT_SUCCESS(status) && status != STATUS_TIMEOUT) ? Status::Success : Status::Timeout; external.acquireExclusive(); --m_waitersCount; - return status; + return result; } template diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 6dccd52..21a5bae 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -59,6 +59,7 @@ wdk_add_driver(kf-test WINVER NTDDI_WIN10 STL AutoSpinLockTest.cpp EResourceSharedLockTest.cpp RecursiveAutoSpinLockTest.cpp + ConditionVariableTest.cpp ) target_link_libraries(kf-test kf::kf kmtest::kmtest) diff --git a/test/ConditionVariableTest.cpp b/test/ConditionVariableTest.cpp new file mode 100644 index 0000000..01d5614 --- /dev/null +++ b/test/ConditionVariableTest.cpp @@ -0,0 +1,292 @@ +#include "pch.h" +#include +#include +#include + +namespace +{ + struct Context + { + kf::EResource* resource = nullptr; + kf::ConditionVariable* cv = nullptr; + LONG counter = 0; + }; + + void delay() + { + LARGE_INTEGER interval; + interval.QuadPart = -10'000; // 1ms + KeDelayExecutionThread(KernelMode, FALSE, &interval); + } +} + +SCENARIO("kf::ConditionVariable") +{ + GIVEN("ConditionVariable without timeout and 1 thread") + { + WHEN("waitFor: timeout is nullptr and cv is notified") + { + kf::EResource resource; + kf::ConditionVariable cv; + kf::Thread thread; + thread.start([](void* context) { + auto cv = static_cast(context); + delay(); + cv->notifyOne(); + }, &cv); + + auto status = cv.waitFor(resource, nullptr); + THEN("waitFor should return success") + { + REQUIRE(status == kf::ConditionVariable::Status::Success); + } + } + + WHEN("waitFor: timeout is set to 0") + { + kf::EResource resource; + kf::ConditionVariable cv; + LARGE_INTEGER timeout{ 0 }; + auto status = cv.waitFor(resource, &timeout); + THEN("waitFor should return Status::Timeout") + { + REQUIRE(status == kf::ConditionVariable::Status::Timeout); + } + } + + WHEN("waitFor: timeout is set to 1ms") + { + kf::EResource resource; + kf::ConditionVariable cv; + LARGE_INTEGER timeout; + timeout.QuadPart = -10'000; // 1ms + auto status = cv.waitFor(resource, &timeout); + THEN("waitFor should return Status::Timeout") + { + REQUIRE(status == kf::ConditionVariable::Status::Timeout); + } + } + } + + GIVEN("ConditionVariable wait with predicate immediately true") + { + kf::EResource resource; + kf::ConditionVariable cv; + LONG counter = 1; + + cv.wait(resource, [&]() { return counter > 0; }); + THEN("Wait should return immediately") + { + REQUIRE(counter == 1); + } + } + + GIVEN("ConditionVariable without timeout and 2 Threads waiting for condition variable") + { + kf::EResource resource; + kf::ConditionVariable cv; + Context ctx{ &resource, &cv }; + kf::Thread thread1; + kf::Thread thread2; + + thread1.start([](void* context) { + auto ctx = static_cast(context); + ctx->cv->wait(*ctx->resource, [&]() { + return ctx->counter > 0; }); + ctx->counter++; + ctx->resource->release(); + }, &ctx); + + thread2.start([](void* context) { + auto ctx = static_cast(context); + ctx->cv->wait(*ctx->resource, [&]() { + return ctx->counter > 0; }); + ctx->counter++; + ctx->resource->release(); + }, &ctx); + delay(); + + WHEN("Predicate is true and called notifyAll") + { + ctx.counter++; + cv.notifyAll(); + thread1.join(); + thread2.join(); + + THEN("Both threads should wake up and increment counter") + { + REQUIRE(ctx.counter == 3); + } + } + } + + GIVEN("ConditionVariable without timeout and 1 thread waiting for predicate") + { + kf::EResource resource; + kf::ConditionVariable cv; + Context ctx{ &resource, &cv }; + kf::Thread thread; + + WHEN("Thread is notified") + { + thread.start([](void* context) { + auto ctx = static_cast(context); + ctx->cv->wait(*ctx->resource, [&]() { return ctx->counter > 0; }); + ctx->counter++; + ctx->resource->release(); + }, &ctx); + + delay(); + + THEN("Thread should be waiting for predicate") + { + REQUIRE(ctx.counter == 0); + } + + ctx.counter++; + cv.notifyOne(); + thread.join(); + + THEN("Predicate become true and thread should wake up") + { + REQUIRE(ctx.counter == 2); + } + } + } + + GIVEN("ConditionVariable with 2 waiting threads and notifyOne is called") + { + kf::EResource resource; + kf::ConditionVariable cv; + Context ctx{ &resource, &cv }; + kf::Thread thread1; + kf::Thread thread2; + + thread1.start([](void* context) { + auto ctx = static_cast(context); + ctx->cv->wait(*ctx->resource, [&]() { return ctx->counter > 0; }); + ctx->counter++; + ctx->resource->release(); + }, &ctx); + + thread2.start([](void* context) { + auto ctx = static_cast(context); + ctx->cv->wait(*ctx->resource, [&]() { return ctx->counter > 0; }); + ctx->counter++; + ctx->resource->release(); + }, &ctx); + + delay(); + ctx.counter++; + cv.notifyOne(); + delay(); + + THEN("Only one thread should wake up and increment counter") + { + REQUIRE(ctx.counter == 2); + } + + cv.notifyOne(); + thread1.join(); + thread2.join(); + + THEN("After second notifyOne both threads should finish") + { + REQUIRE(ctx.counter == 3); + } + } + + GIVEN("ConditionVariable::waitFor with predicate and timeout") + { + WHEN("Timeout is nullptr and predicate is true") + { + kf::EResource resource; + kf::ConditionVariable cv; + kf::Thread thread; + int counter = 1; + thread.start([](void* context) { + auto cv = static_cast(context); + delay(); + cv->notifyOne(); + }, &cv); + + THEN("waitFor should return true") + { + REQUIRE(cv.waitFor(resource, nullptr, [&]() { return counter > 0; })); + } + } + + WHEN("Timeout is set to 0 and predicate is false") + { + kf::EResource resource; + kf::ConditionVariable cv; + LARGE_INTEGER timeout{ 0 }; + int counter = 0; + THEN("waitFor should return false") + { + REQUIRE(!cv.waitFor(resource, &timeout, [&]() { return counter > 0; })); + } + } + + WHEN("Timeout is set to 1ms and predicate is false") + { + kf::EResource resource; + kf::ConditionVariable cv; + LARGE_INTEGER timeout; + timeout.QuadPart = -10'000; // 1ms + int counter = 0; + THEN("waitFor should return false") + { + REQUIRE(!cv.waitFor(resource, &timeout, [&]() { return counter > 0; })); + } + } + } + + GIVEN("ConditionVariable with no waiters") + { + kf::EResource resource; + kf::ConditionVariable cv; + + WHEN("notifyAll is called without waiters") + { + cv.notifyAll(); + THEN("Nothing should happen") + { + } + } + + WHEN("notifyOne is called without waiters") + { + cv.notifyOne(); + THEN("Nothing should happen") + { + } + } + } + + GIVEN("ConditionVariable with 1 thread and multiple notifyOne calls") + { + kf::EResource resource; + kf::ConditionVariable cv; + Context ctx{ &resource, &cv }; + kf::Thread thread; + + thread.start([](void* context) { + auto ctx = static_cast(context); + ctx->cv->wait(*ctx->resource, [&]() { return ctx->counter > 0; }); + ctx->counter++; + ctx->resource->release(); + }, &ctx); + + delay(); + ctx.counter++; + cv.notifyOne(); + cv.notifyOne(); + thread.join(); + + THEN("Thread should wake only once and increment counter") + { + REQUIRE(ctx.counter == 2); + } + } +} From ed2bb9bfca30be90f96db9c592e929d3f749843a Mon Sep 17 00:00:00 2001 From: Vlada Kanivets Date: Tue, 19 Aug 2025 14:31:38 +0200 Subject: [PATCH 2/2] add _invoke_watson stub --- test/pch.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/pch.h b/test/pch.h index b5c57a3..64f5805 100644 --- a/test/pch.h +++ b/test/pch.h @@ -21,6 +21,24 @@ extern "C" inline int _CrtDbgReport( KeBugCheckEx(KERNEL_SECURITY_CHECK_FAILURE, 0, 0, 0, 0); } +inline __declspec(noreturn) +_ACRTIMP void __cdecl _invoke_watson( + _In_opt_z_ wchar_t const* _Expression, + _In_opt_z_ wchar_t const* _FunctionName, + _In_opt_z_ wchar_t const* _FileName, + _In_ unsigned int _LineNo, + _In_ uintptr_t _Reserved) +{ + UNREFERENCED_PARAMETER(_Expression); + UNREFERENCED_PARAMETER(_FunctionName); + UNREFERENCED_PARAMETER(_FileName); + UNREFERENCED_PARAMETER(_LineNo); + UNREFERENCED_PARAMETER(_Reserved); + +#pragma warning(suppress: 28159) // Consider using 'error logging or driver shutdown' instead of 'KeBugCheckEx' + KeBugCheckEx(KERNEL_SECURITY_CHECK_FAILURE, 0, 0, 0, 0); +} + namespace std { [[noreturn]] inline void __cdecl _Xinvalid_argument(_In_z_ const char* /*What*/)