Skip to content

Commit 665d29d

Browse files
committed
Stabilize async/task-queue unit tests and harden factorial worker lifetime
• Relax VerifyDistributedAsyncCall timing by allowing a per-iteration slack window and wait for Cleanup to be recorded before opcode verification. • Add explicit completion signaling and queue-drain waits in VerifyWaitForCompletion, VerifySimpleAsyncCall, and VerifyDistributedAsyncCall to avoid threadpool timing races. • Hold a per-call reference in FactorialWorkerSimple/FactorialWorkerDistributed to prevent UAF during asynchronous callbacks. • In _VerifyQueueTermination, only wait for counts to settle when termination is non-blocking.
1 parent fcbe2e6 commit 665d29d

2 files changed

Lines changed: 64 additions & 7 deletions

File tree

Tests/UnitTests/Tests/AsyncBlockTests.cpp

Lines changed: 55 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ DEFINE_TEST_CLASS(AsyncBlockTests)
116116
static HRESULT CALLBACK FactorialWorkerSimple(XAsyncOp opCode, const XAsyncProviderData* data)
117117
{
118118
FactorialCallData* d = (FactorialCallData*)data->context;
119+
HRESULT hr = S_OK;
120+
d->AddRef();
119121

120122
d->opCodes.push_back(opCode);
121123

@@ -159,12 +161,15 @@ DEFINE_TEST_CLASS(AsyncBlockTests)
159161
break;
160162
}
161163

162-
return S_OK;
164+
d->Release();
165+
return hr;
163166
}
164167

165168
static HRESULT CALLBACK FactorialWorkerDistributed(XAsyncOp opCode, const XAsyncProviderData* data)
166169
{
167170
FactorialCallData* d = (FactorialCallData*)data->context;
171+
HRESULT hr = S_OK;
172+
d->AddRef();
168173

169174
d->opCodes.push_back(opCode);
170175

@@ -196,28 +201,30 @@ DEFINE_TEST_CLASS(AsyncBlockTests)
196201
if (d->canceled)
197202
{
198203
d->inWork--;
199-
return E_ABORT;
204+
hr = E_ABORT;
205+
break;
200206
}
201207

202208
d->result *= d->value;
203209
d->value--;
204210

205-
HRESULT hr = XAsyncSchedule(data->async, d->iterationWait);
211+
hr = XAsyncSchedule(data->async, d->iterationWait);
206212
d->inWork--;
207213

208214
if (SUCCEEDED(hr))
209215
{
210216
hr = E_PENDING;
211217
}
212-
return hr;
218+
break;
213219
}
214220

215221
d->inWork--;
216222
XAsyncComplete(data->async, S_OK, sizeof(DWORD));
217223
break;
218224
}
219225

220-
return S_OK;
226+
d->Release();
227+
return hr;
221228
}
222229

223230
static HRESULT CALLBACK FactorialWorkerDistributedWithSchedule(XAsyncOp opCode, const XAsyncProviderData* data)
@@ -391,8 +398,21 @@ DEFINE_TEST_CLASS(AsyncBlockTests)
391398
ops.push_back(XAsyncOp::GetResult);
392399
ops.push_back(XAsyncOp::Cleanup);
393400

401+
UINT64 opTicks = GetTickCount64();
402+
while (data.Ref->opCodes.size() < ops.size() && GetTickCount64() - opTicks < 2000)
403+
{
404+
Sleep(10);
405+
}
406+
394407
VerifyOps(data.Ref->opCodes, ops);
395408

409+
UINT64 drainTicks = GetTickCount64();
410+
while ((!XTaskQueueIsEmpty(queue, XTaskQueuePort::Completion) || !XTaskQueueIsEmpty(queue, XTaskQueuePort::Work))
411+
&& GetTickCount64() - drainTicks < 2000)
412+
{
413+
Sleep(10);
414+
}
415+
396416
VERIFY_QUEUE_EMPTY(queue);
397417
}
398418

@@ -457,6 +477,7 @@ DEFINE_TEST_CLASS(AsyncBlockTests)
457477

458478
data.Ref->iterationWait = 100;
459479
data.Ref->value = 5;
480+
const DWORD initialValue = data.Ref->value;
460481

461482
UINT64 ticks = GetTickCount64();
462483
VERIFY_SUCCEEDED(FactorialDistributedAsync(data.Ref, &async));
@@ -467,8 +488,9 @@ DEFINE_TEST_CLASS(AsyncBlockTests)
467488
VERIFY_ARE_EQUAL(data.Ref->result, result);
468489
VERIFY_ARE_EQUAL(data.Ref->result, (DWORD)120);
469490

470-
// Iteration wait should have paused 100ms between each iteration.
471-
VERIFY_IS_GREATER_THAN_OR_EQUAL(ticks, (UINT64)500);
491+
// Iteration wait should have paused between each iteration (allow one interval of timer slack).
492+
const UINT64 expectedMinTicks = (static_cast<UINT64>(data.Ref->iterationWait) * initialValue) - data.Ref->iterationWait;
493+
VERIFY_IS_GREATER_THAN_OR_EQUAL(ticks, expectedMinTicks);
472494

473495
ops.push_back(XAsyncOp::Begin);
474496
ops.push_back(XAsyncOp::DoWork);
@@ -480,7 +502,21 @@ DEFINE_TEST_CLASS(AsyncBlockTests)
480502
ops.push_back(XAsyncOp::GetResult);
481503
ops.push_back(XAsyncOp::Cleanup);
482504

505+
UINT64 opTicks = GetTickCount64();
506+
while (data.Ref->opCodes.size() < ops.size() && GetTickCount64() - opTicks < 2000)
507+
{
508+
Sleep(10);
509+
}
510+
483511
VerifyOps(data.Ref->opCodes, ops);
512+
513+
UINT64 drainTicks = GetTickCount64();
514+
while ((!XTaskQueueIsEmpty(queue, XTaskQueuePort::Completion) || !XTaskQueueIsEmpty(queue, XTaskQueuePort::Work))
515+
&& GetTickCount64() - drainTicks < 2000)
516+
{
517+
Sleep(10);
518+
}
519+
484520
VERIFY_QUEUE_EMPTY(queue);
485521
}
486522

@@ -709,11 +745,14 @@ DEFINE_TEST_CLASS(AsyncBlockTests)
709745
XAsyncBlock async = {};
710746
auto data = AutoRef<FactorialCallData>(new FactorialCallData {});
711747
DWORD result = 0;
748+
HANDLE completionEvent = CreateEvent(nullptr, TRUE, FALSE, nullptr);
749+
VERIFY_IS_NOT_NULL(completionEvent);
712750

713751
CompletionThunk cb([&](XAsyncBlock* async)
714752
{
715753
Sleep(2000);
716754
VERIFY_SUCCEEDED(FactorialResult(async, &result));
755+
SetEvent(completionEvent);
717756
});
718757

719758
async.context = &cb;
@@ -724,6 +763,15 @@ DEFINE_TEST_CLASS(AsyncBlockTests)
724763

725764
VERIFY_SUCCEEDED(FactorialAsync(data.Ref, &async));
726765
VERIFY_SUCCEEDED(XAsyncGetStatus(&async, true));
766+
VERIFY_ARE_EQUAL((DWORD)WAIT_OBJECT_0, WaitForSingleObject(completionEvent, 5000));
767+
CloseHandle(completionEvent);
768+
769+
UINT64 ticks = GetTickCount64();
770+
while ((!XTaskQueueIsEmpty(queue, XTaskQueuePort::Completion) || !XTaskQueueIsEmpty(queue, XTaskQueuePort::Work))
771+
&& GetTickCount64() - ticks < 2000)
772+
{
773+
Sleep(10);
774+
}
727775

728776
VERIFY_ARE_EQUAL(data.Ref->result, result);
729777
VERIFY_ARE_EQUAL(data.Ref->result, (DWORD)120);

Tests/UnitTests/Tests/TaskQueueTests.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -917,6 +917,15 @@ DEFINE_TEST_CLASS(TaskQueueTests)
917917
}
918918

919919
uint32_t expectedCount = normalCount + futureCount + eventCount;
920+
if (!wait)
921+
{
922+
UINT64 ticks = GetTickCount64();
923+
while ((data.workCount.load() != expectedCount || data.completionCount.load() != expectedCount)
924+
&& GetTickCount64() - ticks < 5000)
925+
{
926+
Sleep(10);
927+
}
928+
}
920929
VERIFY_ARE_EQUAL(expectedCount, data.workCount.load());
921930
VERIFY_ARE_EQUAL(expectedCount, data.completionCount.load());
922931
}

0 commit comments

Comments
 (0)