Skip to content

Commit 40b7302

Browse files
committed
fs.readFile: don't make the callback before the fd is closed
On Windows it is not possible to unlink() the read file in the callback. This fixes #3051. A test is included.
1 parent f178f2a commit 40b7302

File tree

2 files changed

+55
-2
lines changed

2 files changed

+55
-2
lines changed

lib/fs.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,18 +78,23 @@ fs.readFile = function(path, encoding_) {
7878
var readStream = fs.createReadStream(path);
7979
var buffers = [];
8080
var nread = 0;
81+
var error;
8182

8283
readStream.on('data', function(chunk) {
8384
buffers.push(chunk);
8485
nread += chunk.length;
8586
});
8687

8788
readStream.on('error', function(er) {
88-
callback(er);
89+
error = er;
8990
readStream.destroy();
9091
});
9192

92-
readStream.on('end', function() {
93+
readStream.on('close', function() {
94+
if (error) {
95+
return callback(error);
96+
}
97+
9398
// copy all the buffers into one
9499
var buffer;
95100
switch (buffers.length) {
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright Joyent, Inc. and other Node contributors.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a
4+
// copy of this software and associated documentation files (the
5+
// "Software"), to deal in the Software without restriction, including
6+
// without limitation the rights to use, copy, modify, merge, publish,
7+
// distribute, sublicense, and/or sell copies of the Software, and to permit
8+
// persons to whom the Software is furnished to do so, subject to the
9+
// following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included
12+
// in all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
15+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
17+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
18+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
19+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
20+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
22+
var assert = require('assert'),
23+
common = require('../common'),
24+
fs = require('fs'),
25+
path = require('path'),
26+
dirName = path.resolve(common.fixturesDir, 'test-readfile-unlink'),
27+
fileName = path.resolve(dirName, 'test.bin');
28+
29+
var buf = new Buffer(512 * 1024);
30+
buf.fill(42);
31+
32+
try {
33+
fs.mkdirSync(dirName);
34+
} catch (e) {
35+
// Ignore if the directory already exists.
36+
if (e.code != 'EEXIST') throw e;
37+
}
38+
39+
fs.writeFileSync(fileName, buf);
40+
41+
fs.readFile(fileName, function(err, data) {
42+
assert.ifError(err);
43+
assert(data.length == buf.length);
44+
assert.strictEqual(buf[0], 42);
45+
46+
fs.unlinkSync(fileName);
47+
fs.rmdirSync(dirName);
48+
});

0 commit comments

Comments
 (0)