Browse Source

Retain buffers in fs.read/write()

Closes GH-814.
Closes GH-827.
v0.7.4-release
Jorge Chamorro Bieling 14 years ago
committed by Ryan Dahl
parent
commit
e7604b1ea7
  1. 14
      lib/fs.js
  2. 7
      src/node_file.cc
  3. 4
      test/common.js
  4. 71
      test/pummel/test-regress-GH-814.js
  5. 85
      test/pummel/test-regress-GH-814_2.js

14
lib/fs.js

@ -240,7 +240,12 @@ fs.read = function(fd, buffer, offset, length, position, callback) {
};
}
binding.read(fd, buffer, offset, length, position, callback || noop);
function wrapper(err, bytesRead) {
// Retain a reference to buffer so that it can't be GC'ed too soon.
callback && callback(err, bytesRead || 0, buffer);
}
binding.read(fd, buffer, offset, length, position, wrapper);
};
fs.readSync = function(fd, buffer, offset, length, position) {
@ -285,7 +290,12 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
return;
}
binding.write(fd, buffer, offset, length, position, callback || noop);
function wrapper(err, written) {
// Retain a reference to buffer so that it can't be GC'ed too soon.
callback && callback(err, written || 0, buffer);
}
binding.write(fd, buffer, offset, length, position, wrapper);
};
fs.writeSync = function(fd, buffer, offset, length, position) {

7
src/node_file.cc

@ -707,9 +707,6 @@ static Handle<Value> Write(const Arguments& args) {
Local<Value> cb = args[5];
if (cb->IsFunction()) {
// Grab a reference to buffer so it isn't GCed
Local<Object> cb_obj = cb->ToObject();
cb_obj->Set(buf_symbol, buffer_obj);
ASYNC_CALL(write, cb, fd, buf, len, pos)
} else {
@ -775,10 +772,6 @@ static Handle<Value> Read(const Arguments& args) {
cb = args[5];
if (cb->IsFunction()) {
// Grab a reference to buffer so it isn't GCed
// TODO: need test coverage
Local<Object> cb_obj = cb->ToObject();
cb_obj->Set(buf_symbol, buffer_obj);
ASYNC_CALL(read, cb, fd, buf, len, pos);
} else {

4
test/common.js

@ -60,6 +60,10 @@ process.on('exit', function() {
process,
global];
if (global.gc) {
knownGlobals.push(gc);
}
if (global.DTRACE_HTTP_SERVER_RESPONSE) {
knownGlobals.push(DTRACE_HTTP_SERVER_RESPONSE);
knownGlobals.push(DTRACE_HTTP_SERVER_REQUEST);

71
test/pummel/test-regress-GH-814.js

@ -0,0 +1,71 @@
// Flags: --expose_gc
function newBuffer(size, value) {
var buffer = new Buffer(size);
while (size--) {
buffer[size] = value;
}
//buffer[buffer.length-2]= 0x0d;
buffer[buffer.length - 1] = 0x0a;
return buffer;
}
var common = require('../common');
var fs = require('fs');
var testFileName = require('path').join(common.tmpDir, 'GH-814_testFile.txt');
var testFileFD = fs.openSync(testFileName, 'w');
console.log(testFileName);
var kBufSize = 128 * 1024;
var PASS = true;
var neverWrittenBuffer = newBuffer(kBufSize, 0x2e); //0x2e === '.'
var bufPool = [];
var tail = require('child_process').spawn('tail', ['-f', testFileName]);
tail.stdout.on('data', tailCB);
function tailCB(data) {
PASS = data.toString().indexOf('.') < 0;
}
var 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);
gc();
var nuBuf = new Buffer(kBufSize);
neverWrittenBuffer.copy(nuBuf);
if (bufPool.push(nuBuf) > 100) {
bufPool.length = 0;
}
}
else {
throw 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) {
if (err) {
throw err;
}
}

85
test/pummel/test-regress-GH-814_2.js

@ -0,0 +1,85 @@
// Flags: --expose_gc
var common = require('../common');
var fs = require('fs');
var testFileName = require('path').join(common.tmpDir, 'GH-814_test.txt');
var testFD = fs.openSync(testFileName, 'w');
console.error(testFileName + '\n');
var tailProc = require('child_process').spawn('tail', ['-f', testFileName]);
tailProc.stdout.on('data', tailCB);
function tailCB(data) {
PASS = data.toString().indexOf('.') < 0;
if (PASS) {
//console.error('i');
} else {
console.error('[FAIL]\n DATA -> ');
console.error(data);
console.error('\n');
throw Error('Buffers GC test -> FAIL');
}
}
var PASS = true;
var bufPool = [];
var kBufSize = 16 * 1024 * 1024;
var neverWrittenBuffer = newBuffer(kBufSize, 0x2e); //0x2e === '.'
var 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);
gc();
gc();
gc();
gc();
gc();
gc();
var nuBuf = new Buffer(kBufSize);
neverWrittenBuffer.copy(nuBuf);
if (bufPool.push(nuBuf) > 100) {
bufPool.length = 0;
}
process.nextTick(writer);
//console.error('o');
}
}
}
function writerCB(err, written) {
//console.error('cb.');
if (err) {
throw err;
}
}
// ******************* UTILITIES
function newBuffer(size, value) {
var buffer = new Buffer(size);
while (size--) {
buffer[size] = value;
}
buffer[buffer.length - 1] = 0x0d;
buffer[buffer.length - 1] = 0x0a;
return buffer;
}
Loading…
Cancel
Save