Skip to content

Commit 8ef68e6

Browse files
joaocgreisTrott
joaocgreis
authored and
Trott
committed
test: clean tmpdir on process exit
PR-URL: #28858 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
1 parent 0376b5b commit 8ef68e6

File tree

48 files changed

+212
-66
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+212
-66
lines changed

‎test/addons/load-long-path/test.js‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ if (common.isWindows && (process.env.PROCESSOR_ARCHITEW6432 !== undefined))
66
const fs = require('fs');
77
const path = require('path');
88
const assert = require('assert');
9+
const { fork } = require('child_process');
910

1011
const tmpdir = require('../../common/tmpdir');
11-
tmpdir.refresh();
1212

1313
// Make a path that is more than 260 chars long.
1414
// Any given folder cannot have a name longer than 260 characters,
@@ -17,7 +17,6 @@ let addonDestinationDir = path.resolve(tmpdir.path);
1717

1818
for (let i = 0; i < 10; i++) {
1919
addonDestinationDir = path.join(addonDestinationDir, 'x'.repeat(30));
20-
fs.mkdirSync(addonDestinationDir);
2120
}
2221

2322
const addonPath = path.join(__dirname,
@@ -26,11 +25,29 @@ const addonPath = path.join(__dirname,
2625
'binding.node');
2726
const addonDestinationPath = path.join(addonDestinationDir, 'binding.node');
2827

28+
// Loading an addon keeps the file open until the process terminates. Load
29+
// the addon in a child process so that when the parent terminates the file
30+
// is already closed and the tmpdir can be cleaned up.
31+
32+
// Child
33+
if (process.argv[2] === 'child') {
34+
// Attempt to load at long path destination
35+
const addon = require(addonDestinationPath);
36+
assert.notStrictEqual(addon, null);
37+
assert.strictEqual(addon.hello(), 'world');
38+
return;
39+
}
40+
41+
// Parent
42+
tmpdir.refresh();
43+
2944
// Copy binary to long path destination
45+
fs.mkdirSync(addonDestinationDir, { recursive: true });
3046
const contents = fs.readFileSync(addonPath);
3147
fs.writeFileSync(addonDestinationPath, contents);
3248

33-
// Attempt to load at long path destination
34-
const addon = require(addonDestinationPath);
35-
assert.notStrictEqual(addon, null);
36-
assert.strictEqual(addon.hello(), 'world');
49+
// Run test
50+
const child = fork(__filename, ['child'], { stdio: 'inherit' });
51+
child.on('exit', common.mustCall(function(code) {
52+
assert.strictEqual(code, 0);
53+
}));

‎test/common/README.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -906,6 +906,13 @@ The realpath of the testing temporary directory.
906906

907907
Deletes and recreates the testing temporary directory.
908908

909+
The first time `refresh()` runs, it adds a listener to process `'exit'` that
910+
cleans the temporary directory. Thus, every file under `tmpdir.path` needs to
911+
be closed before the test completes. A good way to do this is to add a
912+
listener to process `'beforeExit'`. If a file needs to be left open until
913+
Node.js completes, use a child process and call `refresh()` only in the
914+
parent.
915+
909916
## WPT Module
910917

911918
### harness

‎test/common/tmpdir.js‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const { execSync } = require('child_process');
55
const fs = require('fs');
66
const path = require('path');
77
const { debuglog } = require('util');
8+
const { isMainThread } = require('worker_threads');
89

910
const debug = debuglog('test/tmpdir');
1011

@@ -61,6 +62,9 @@ function rimrafSync(pathname, { spawn = true } = {}) {
6162
}
6263
rmdirSync(pathname, e);
6364
}
65+
66+
if (fs.existsSync(pathname))
67+
throw new Error(`Unable to rimraf ${pathname}`);
6468
}
6569

6670
function rmdirSync(p, originalEr) {
@@ -80,7 +84,9 @@ function rmdirSync(p, originalEr) {
8084
}
8185
});
8286
fs.rmdirSync(p);
87+
return;
8388
}
89+
throw e;
8490
}
8591
}
8692

@@ -93,9 +99,42 @@ const tmpdirName = '.tmp.' +
9399
(process.env.TEST_SERIAL_ID || process.env.TEST_THREAD_ID || '0');
94100
const tmpPath = path.join(testRoot, tmpdirName);
95101

102+
let firstRefresh = true;
96103
function refresh(opts = {}) {
97104
rimrafSync(this.path, opts);
98105
fs.mkdirSync(this.path);
106+
107+
if (firstRefresh) {
108+
firstRefresh = false;
109+
// Clean only when a test uses refresh. This allows for child processes to
110+
// use the tmpdir and only the parent will clean on exit.
111+
process.on('exit', onexit);
112+
}
113+
}
114+
115+
function onexit() {
116+
// Change directory to avoid possible EBUSY
117+
if (isMainThread)
118+
process.chdir(testRoot);
119+
120+
try {
121+
rimrafSync(tmpPath, { spawn: false });
122+
} catch (e) {
123+
console.error('Can\'t clean tmpdir:', tmpPath);
124+
125+
const files = fs.readdirSync(tmpPath);
126+
console.error('Files blocking:', files);
127+
128+
if (files.some((f) => f.startsWith('.nfs'))) {
129+
// Warn about NFS "silly rename"
130+
console.error('Note: ".nfs*" might be files that were open and ' +
131+
'unlinked but not closed.');
132+
console.error('See http://nfs.sourceforge.net/#faq_d2 for details.');
133+
}
134+
135+
console.error();
136+
throw e;
137+
}
99138
}
100139

101140
module.exports = {

‎test/parallel/test-child-process-server-close.js‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,29 @@
11
'use strict';
22

33
const common = require('../common');
4-
const { spawn } = require('child_process');
4+
const assert = require('assert');
5+
const { fork, spawn } = require('child_process');
56
const net = require('net');
67

78
const tmpdir = require('../common/tmpdir');
8-
tmpdir.refresh();
99

10+
// Run in a child process because the PIPE file descriptor stays open until
11+
// Node.js completes, blocking the tmpdir and preventing cleanup.
12+
13+
if (process.argv[2] !== 'child') {
14+
// Parent
15+
tmpdir.refresh();
16+
17+
// Run test
18+
const child = fork(__filename, ['child'], { stdio: 'inherit' });
19+
child.on('exit', common.mustCall(function(code) {
20+
assert.strictEqual(code, 0);
21+
}));
22+
23+
return;
24+
}
25+
26+
// Child
1027
const server = net.createServer((conn) => {
1128
spawn(process.execPath, ['-v'], {
1229
stdio: ['ignore', conn, 'ignore']

‎test/parallel/test-fs-append-file-sync.js‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,3 @@ const fileData5 = fs.readFileSync(filename5);
9999

100100
assert.strictEqual(Buffer.byteLength(data) + currentFileData.length,
101101
fileData5.length);
102-
103-
// Exit logic for cleanup.
104-
105-
process.on('exit', function() {
106-
fs.unlinkSync(filename);
107-
fs.unlinkSync(filename2);
108-
fs.unlinkSync(filename3);
109-
fs.unlinkSync(filename4);
110-
fs.unlinkSync(filename5);
111-
});

‎test/parallel/test-fs-buffertype-writesync.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,5 @@ v.forEach((value) => {
1919
const fd = fs.openSync(filePath, 'w');
2020
fs.writeSync(fd, value);
2121
assert.strictEqual(fs.readFileSync(filePath).toString(), String(value));
22+
fs.closeSync(fd);
2223
});

‎test/parallel/test-fs-fsync.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ fs.open(fileTemp, 'a', 0o777, common.mustCall(function(err, fd) {
4646
assert.ifError(err);
4747
fs.fsync(fd, common.mustCall(function(err) {
4848
assert.ifError(err);
49+
fs.closeSync(fd);
4950
}));
5051
}));
5152
}));

‎test/parallel/test-fs-long-path.js‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,3 @@ fs.writeFile(fullPath, 'ok', common.mustCall(function(err) {
4949
assert.ifError(err);
5050
}));
5151
}));
52-
53-
process.on('exit', function() {
54-
fs.unlinkSync(fullPath);
55-
});

‎test/parallel/test-fs-open-flags.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,5 +94,8 @@ if (common.isLinux || common.isOSX) {
9494
tmpdir.refresh();
9595
const file = path.join(tmpdir.path, 'a.js');
9696
fs.copyFileSync(fixtures.path('a.js'), file);
97-
fs.open(file, O_DSYNC, common.mustCall(assert.ifError));
97+
fs.open(file, O_DSYNC, common.mustCall((err, fd) => {
98+
assert.ifError(err);
99+
fs.closeSync(fd);
100+
}));
98101
}

‎test/parallel/test-fs-options-immutable.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,6 @@ if (common.canCreateSymLink()) {
7070
{
7171
const fileName = path.resolve(tmpdir.path, 'streams');
7272
fs.WriteStream(fileName, options).once('open', common.mustCall(() => {
73-
fs.ReadStream(fileName, options);
74-
}));
73+
fs.ReadStream(fileName, options).destroy();
74+
})).end();
7575
}

0 commit comments

Comments
 (0)