From f5e77385c579f7b9df0b7b3fafc944e2e9e00be8 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 7 Sep 2024 10:54:33 -0700 Subject: [PATCH] test: reduce the allocation size in test-worker-arraybuffer-zerofill Test has been flaky with timeouts in CI. This is possibly due to the repeated large allocations on the main thread. This commit reduces the allocation size and makes a number of other cleanups. The main goal is to hopefully make this test more reliable / not-flaky. Also move the test to sequential. The frequent large allocations could be causing the test to be flaky if run parallel to other tests. PR-URL: https://github.com/nodejs/node/pull/54839 Refs: https://github.com/nodejs/node/issues/52274 Reviewed-By: Yagiz Nizipli Reviewed-By: Matteo Collina --- .../test-worker-arraybuffer-zerofill.js | 33 -------------- .../test-worker-arraybuffer-zerofill.js | 43 +++++++++++++++++++ 2 files changed, 43 insertions(+), 33 deletions(-) delete mode 100644 test/parallel/test-worker-arraybuffer-zerofill.js create mode 100644 test/sequential/test-worker-arraybuffer-zerofill.js diff --git a/test/parallel/test-worker-arraybuffer-zerofill.js b/test/parallel/test-worker-arraybuffer-zerofill.js deleted file mode 100644 index 3dcf4c006ebcd9..00000000000000 --- a/test/parallel/test-worker-arraybuffer-zerofill.js +++ /dev/null @@ -1,33 +0,0 @@ -'use strict'; -require('../common'); -const assert = require('assert'); -const { Worker } = require('worker_threads'); - -// Make sure that allocating uninitialized ArrayBuffers in one thread does not -// affect the zero-initialization in other threads. - -const w = new Worker(` -const { parentPort } = require('worker_threads'); - -function post() { - const uint32array = new Uint32Array(64); - parentPort.postMessage(uint32array.reduce((a, b) => a + b)); -} - -setInterval(post, 0); -`, { eval: true }); - -function allocBuffers() { - Buffer.allocUnsafe(32 * 1024 * 1024); -} - -const interval = setInterval(allocBuffers, 0); - -let messages = 0; -w.on('message', (sum) => { - assert.strictEqual(sum, 0); - if (messages++ === 100) { - clearInterval(interval); - w.terminate(); - } -}); diff --git a/test/sequential/test-worker-arraybuffer-zerofill.js b/test/sequential/test-worker-arraybuffer-zerofill.js new file mode 100644 index 00000000000000..b30f3e513c6ec6 --- /dev/null +++ b/test/sequential/test-worker-arraybuffer-zerofill.js @@ -0,0 +1,43 @@ +'use strict'; +require('../common'); +const Countdown = require('../common/countdown'); +const assert = require('assert'); +const { Worker } = require('worker_threads'); +const { describe, it, mock } = require('node:test'); + +describe('Allocating uninitialized ArrayBuffers ...', () => { + it('...should not affect zero-fill in other threads', () => { + const w = new Worker(` + const { parentPort } = require('worker_threads'); + + function post() { + const uint32array = new Uint32Array(64); + parentPort.postMessage(uint32array.reduce((a, b) => a + b)); + } + + setInterval(post, 0); + `, { eval: true }); + + const fn = mock.fn(() => { + // Continuously allocate memory in the main thread. The allocUnsafe + // here sets a scope internally that indicates that the memory should + // not be initialized. While this is happening, the other thread is + // also allocating buffers that must remain zero-filled. The purpose + // of this test is to ensure that the scope used to determine whether + // to zero-fill or not does not impact the other thread. + setInterval(() => Buffer.allocUnsafe(32 * 1024 * 1024), 0).unref(); + }); + + w.on('online', fn); + + const countdown = new Countdown(100, () => { + w.terminate(); + assert(fn.mock.calls.length > 0); + }); + + w.on('message', (sum) => { + assert.strictEqual(sum, 0); + if (countdown.remaining) countdown.dec(); + }); + }); +});