Skip to content

Commit 5a26d3e

Browse files
author
Gabriel Schulhof
committed
remove race from asyncprogressqueueworker
1 parent 5ba003c commit 5a26d3e

File tree

2 files changed

+14
-21
lines changed

2 files changed

+14
-21
lines changed

test/asyncprogressqueueworker.cc

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@ class TestWorker : public AsyncProgressQueueWorker<ProgressData> {
4040
static void CancelWork(const CallbackInfo& info) {
4141
auto wrap = info[0].As<Napi::External<TestWorker>>();
4242
auto worker = wrap.Data();
43-
// We cannot cancel a worker if it got started. So we have to do a quick cancel.
44-
worker->Queue();
4543
worker->Cancel();
4644
}
4745

test/asyncprogressqueueworker.js

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ module.exports = test(require(`./build/${buildType}/binding.node`))
1010
async function test({ asyncprogressqueueworker }) {
1111
await success(asyncprogressqueueworker);
1212
await fail(asyncprogressqueueworker);
13-
await cancel(asyncprogressqueueworker);
13+
cancel(asyncprogressqueueworker);
1414
}
1515

1616
function success(binding) {
@@ -49,22 +49,17 @@ function fail(binding) {
4949
}
5050

5151
function cancel(binding) {
52-
return new Promise((resolve, reject) => {
53-
// make sure the work we are going to cancel will not be
54-
// able to start by using all the threads in the pool.
55-
for (let i = 0; i < os.cpus().length; ++i) {
56-
const worker = binding.createWork(-1, () => {}, () => {});
57-
binding.queueWork(worker);
58-
}
59-
const worker = binding.createWork(-1,
60-
() => {
61-
assert.fail('unexpected callback');
62-
},
63-
() => {
64-
assert.fail('unexpected progress report');
65-
}
66-
);
67-
binding.cancelWork(worker);
68-
resolve();
69-
});
52+
// We cannot cancel work once it gets started, so we have no choice here but
53+
// to create the work and then cancel it without queueing it. Thus we cannot
54+
// reliably test create -> queue -> cancel.
55+
//
56+
// We could queue it and immediately cancel, and we could even attempt to
57+
// saturate the thread pool with work before we do that but that would still
58+
// be racy and, on some platforms and under certain circumstances the work
59+
// might still get queued and therefore be unable to be cancelled, resulting
60+
// in this test being flaky.
61+
const worker = binding.createWork(-1,
62+
() => reject(new Error('unexpected callback')),
63+
() => reject(new Error('unexpected progress report')));
64+
binding.cancelWork(worker);
7065
}

0 commit comments

Comments
 (0)