test(sema): make contended semaphore poll/signal test deterministic (#176)

The "Semaphore poll/signal remains stable under host-thread contention"
test created the semaphore full (init == max == 1), so SignalSema could
only succeed after PollSema had already freed a slot. With no start
barrier, under host-thread contention the signaler thread could run all
64 of its iterations before the poller's first timeslice, making every
SignalSema legitimately return KE_SEMA_OVF and leaving signalOkCount at
0 — a false failure of "contended SignalSema should observe successful
releases". An earlier investigation of the unfixed test observed this
twice in 24 contended (8-way-parallel) full-suite runs and never in 20
serial runs; CI has independently hit the same assertion on the
unrelated draft PR #174, confirming the trigger is host scheduling, not
the code under review.

Seed the semaphore with headroom (init=1, max=2) so the first PollSema
and the first SignalSema each succeed regardless of scheduling order,
and add a start barrier so both workers start together, maximizing the
opportunity to interleave. Widen the final-count range check to the new
max. The semaphore implementation is unchanged; both threads still
contend concurrently on the same per-semaphore mutex.
This commit is contained in:
Shane Michael Mathews (Personal Account)
2026-07-21 10:09:38 -04:00
committed by GitHub
parent ee149581aa
commit 1176609890
+21 -2
View File
@@ -1734,9 +1734,13 @@ void register_ps2_runtime_expansion_tests()
std::vector<uint8_t> rdram(PS2_RAM_SIZE, 0u);
constexpr uint32_t kParamAddr = 0x2000u;
// init=1 < max=2 headroom makes both first calls succeed
// regardless of scheduling: poller is the sole decrementer
// (count>=1 at first poll), signaler the sole incrementer
// (count<max at first signal). Keep init<max.
const uint32_t semaParam[6] = {
0u, // count
1u, // max_count
2u, // max_count
1u, // init_count
0u, // wait_threads
0u, // attr
@@ -1754,11 +1758,25 @@ void register_ps2_runtime_expansion_tests()
std::atomic<int32_t> signalOkCount{0};
std::atomic<bool> pollerThrew{false};
std::atomic<bool> signalerThrew{false};
std::atomic<int32_t> readyCount{0};
// Release both workers together so their 64-iteration loops start at
// the same instant, maximizing the opportunity to interleave instead
// of one thread running to completion before the other is scheduled.
const auto waitForStart = [&]()
{
readyCount.fetch_add(1, std::memory_order_acq_rel);
while (readyCount.load(std::memory_order_acquire) < 2)
{
std::this_thread::yield();
}
};
std::thread poller([&]()
{
try
{
waitForStart();
for (int i = 0; i < 64; ++i)
{
R5900Context pollCtx{};
@@ -1780,6 +1798,7 @@ void register_ps2_runtime_expansion_tests()
{
try
{
waitForStart();
for (int i = 0; i < 64; ++i)
{
R5900Context signalCtx{};
@@ -1824,7 +1843,7 @@ void register_ps2_runtime_expansion_tests()
int32_t finalCount = 0;
std::memcpy(&finalCount, rdram.data() + kStatusAddr + 0u, sizeof(finalCount));
t.IsTrue(finalCount >= 0 && finalCount <= 1, "semaphore count should remain within [0, max_count]");
t.IsTrue(finalCount >= 0 && finalCount <= 2, "semaphore count should remain within [0, max_count]");
runtime.requestStop();
notifyRuntimeStop();