From 0bb70b0a02c2e6d5e6de61f9f9619f379b09a92e Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 27 Jul 2020 18:19:33 +0200 Subject: [PATCH] test: replace flaky pummel regression tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These tests were written a long time ago, and use the allocation of large amounts of unused memory as a way to detect use-after-free problems with Buffers. As a result, the tests are resource-intensive and may crash because of that. Replace them with a more modern test. We don’t explicitly try to *detect* use-after-free conditions, and instead rely on e.g. ASAN (or the process just crashing hard) to do that for us. Fixes: https://github.com/nodejs/node/issues/34527 PR-URL: https://github.com/nodejs/node/pull/34530 Reviewed-By: Rich Trott Reviewed-By: Andrey Pechkurov Reviewed-By: Jiawen Geng --- test/parallel/test-fs-write-reuse-callback.js | 39 +++++++ test/pummel/test-regress-GH-814.js | 90 --------------- test/pummel/test-regress-GH-814_2.js | 105 ------------------ 3 files changed, 39 insertions(+), 195 deletions(-) create mode 100644 test/parallel/test-fs-write-reuse-callback.js delete mode 100644 test/pummel/test-regress-GH-814.js delete mode 100644 test/pummel/test-regress-GH-814_2.js diff --git a/test/parallel/test-fs-write-reuse-callback.js b/test/parallel/test-fs-write-reuse-callback.js new file mode 100644 index 00000000000000..13a33a7545263b --- /dev/null +++ b/test/parallel/test-fs-write-reuse-callback.js @@ -0,0 +1,39 @@ +// Flags: --expose-gc +'use strict'; +const common = require('../common'); +const tmpdir = require('../common/tmpdir'); +const assert = require('assert'); +const path = require('path'); + +// Regression test for https://github.com/nodejs/node-v0.x-archive/issues/814: +// Make sure that Buffers passed to fs.write() are not garbage-collected +// even when the callback is being reused. + +const fs = require('fs'); + +tmpdir.refresh(); +const filename = path.join(tmpdir.path, 'test.txt'); +const fd = fs.openSync(filename, 'w'); + +const size = 16 * 1024; +const writes = 1000; +let done = 0; + +const ondone = common.mustCall((err) => { + assert.ifError(err); + if (++done < writes) { + if (done % 25 === 0) global.gc(); + setImmediate(write); + } else { + assert.strictEqual( + fs.readFileSync(filename, 'utf8'), + 'x'.repeat(writes * size)); + fs.closeSync(fd); + } +}, writes); + +write(); +function write() { + const buf = Buffer.alloc(size, 'x'); + fs.write(fd, buf, 0, buf.size, -1, ondone); +} diff --git a/test/pummel/test-regress-GH-814.js b/test/pummel/test-regress-GH-814.js deleted file mode 100644 index 323163225738b4..00000000000000 --- a/test/pummel/test-regress-GH-814.js +++ /dev/null @@ -1,90 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -// Refs: https://github.com/nodejs/node-v0.x-archive/issues/814 - -'use strict'; -// Flags: --expose_gc - -require('../common'); -const assert = require('assert'); - -const tmpdir = require('../common/tmpdir'); - -function newBuffer(size, value) { - const buffer = Buffer.allocUnsafe(size); - while (size--) { - buffer[size] = value; - } - buffer[buffer.length - 1] = 0x0a; - return buffer; -} - -const fs = require('fs'); - -tmpdir.refresh(); -const testFileName = require('path').join(tmpdir.path, 'GH-814_testFile.txt'); -const testFileFD = fs.openSync(testFileName, 'w'); -console.log(testFileName); - - -const kBufSize = 128 * 1024; -let PASS = true; -const neverWrittenBuffer = newBuffer(kBufSize, 0x2e); // 0x2e === '.' -const bufPool = []; - - -const tail = require('child_process').spawn('tail', ['-f', testFileName]); -tail.stdout.on('data', tailCB); - -function tailCB(data) { - PASS = !data.toString().includes('.'); -} - - -const timeToQuit = Date.now() + 8e3; // Test during no more than this seconds. -(function main() { - - if (PASS) { - fs.write(testFileFD, newBuffer(kBufSize, 0x61), 0, kBufSize, -1, cb); - global.gc(); - const nuBuf = Buffer.allocUnsafe(kBufSize); - neverWrittenBuffer.copy(nuBuf); - if (bufPool.push(nuBuf) > 100) { - bufPool.length = 0; - } - } else { - throw new Error("Buffer GC'ed test -> FAIL"); - } - - if (Date.now() < timeToQuit) { - process.nextTick(main); - } else { - tail.kill(); - console.log("Buffer GC'ed test -> PASS (OK)"); - } - -})(); - - -function cb(err, written) { - assert.ifError(err); -} diff --git a/test/pummel/test-regress-GH-814_2.js b/test/pummel/test-regress-GH-814_2.js deleted file mode 100644 index 6806a7fc45143c..00000000000000 --- a/test/pummel/test-regress-GH-814_2.js +++ /dev/null @@ -1,105 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -// Refs: https://github.com/nodejs/node-v0.x-archive/issues/814 - -'use strict'; -// Flags: --expose_gc - -require('../common'); -const assert = require('assert'); - -const fs = require('fs'); -const tmpdir = require('../common/tmpdir'); - -tmpdir.refresh(); -const testFileName = require('path').join(tmpdir.path, 'GH-814_test.txt'); -const testFD = fs.openSync(testFileName, 'w'); -console.error(`${testFileName}\n`); - - -const tailProc = require('child_process').spawn('tail', ['-f', testFileName]); -tailProc.stdout.on('data', tailCB); - -function tailCB(data) { - PASS = !data.toString().includes('.'); - - if (!PASS) { - console.error('[FAIL]\n DATA -> '); - console.error(data); - console.error('\n'); - throw new Error('Buffers GC test -> FAIL'); - } -} - - -let PASS = true; -const bufPool = []; -const kBufSize = 16 * 1024 * 1024; -const neverWrittenBuffer = newBuffer(kBufSize, 0x2e); // 0x2e === '.' - -const timeToQuit = Date.now() + 5e3; // Test should last no more than this. -writer(); - -function writer() { - - if (PASS) { - if (Date.now() > timeToQuit) { - setTimeout(function() { - process.kill(tailProc.pid); - console.error('\nBuffers GC test -> PASS (OK)\n'); - }, 555); - } else { - fs.write(testFD, newBuffer(kBufSize, 0x61), 0, kBufSize, -1, writerCB); - global.gc(); - global.gc(); - global.gc(); - global.gc(); - global.gc(); - global.gc(); - const nuBuf = Buffer.allocUnsafe(kBufSize); - neverWrittenBuffer.copy(nuBuf); - if (bufPool.push(nuBuf) > 100) { - bufPool.length = 0; - } - process.nextTick(writer); - } - } - -} - -function writerCB(err, written) { - assert.ifError(err); -} - - -// ******************* UTILITIES - - -function newBuffer(size, value) { - const buffer = Buffer.allocUnsafe(size); - while (size--) { - buffer[size] = value; - } - buffer[buffer.length - 1] = 0x0d; - buffer[buffer.length - 1] = 0x0a; - return buffer; -}