From aa4555f063f427db398583b3aaf0fbf8b59e5d9f Mon Sep 17 00:00:00 2001 From: "Peter A. Bigot" Date: Mon, 10 Oct 2016 05:10:38 -0500 Subject: [PATCH 1/2] add options to control descriptor management This commit adds two options to control how tmp manages the file descriptor that results from creating the temporary file. Fixes #88 Fixes #91 Signed-off-by: Peter A. Bigot --- README.md | 41 +++++++++++++++++++++++++++++++++++ lib/tmp.js | 12 ++++++++++- test/base.js | 5 +++++ test/file-test.js | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 804bf33..1e0798e 100644 --- a/README.md +++ b/README.md @@ -164,6 +164,47 @@ console.log("File: ", tmpobj.name); console.log("Filedescriptor: ", tmpobj.fd); ``` +### Controlling the Descriptor + +As a side effect of creating a unique file `tmp` gets a file descriptor that is +returned to the user as the `fd` parameter. The descriptor may be used by the +application and is closed when the `removeCallback` is invoked. + +In some use cases the application does not need the descriptor, needs to close it +without removing the file, or needs to remove the file without closing the +descriptor. Two options control how the descriptor is managed: + +* `discardDescriptor` - if `true` causes `tmp` to close the descriptor after the file + is created. In this case the `fd` parameter is undefined. +* `detachDescriptor` - if `true` causes `tmp` to return the descriptor in the `fd` + parameter, but it is the application's responsibility to close it when it is no + longer needed. + +```javascript +var tmp = require('tmp'); + +tmp.file({discardDescriptor: true}, + function _tempFileCreated(err, path, fd, cleanupCallback) { + if (err) throw err; + // fd will be undefined, allowing application to use fs.createReadStream(path) + // without holding an unused descriptor open. +}); +``` + +```javascript +var tmp = require('tmp'); + +tmp.file({detachDescriptor: true}, + function _tempFileCreated(err, path, fd, cleanupCallback) { + if (err) throw err; + + cleanupCallback(); + // Application can store data through fd here; the space used will automatically + // be reclaimed by the operating system when the descriptor is closed or program + // terminates. +}); +``` + ### Asynchronous directory creation Creates a directory with mode `0755`, prefix will be `myTmpDir_`. diff --git a/lib/tmp.js b/lib/tmp.js index e968214..39bf070 100644 --- a/lib/tmp.js +++ b/lib/tmp.js @@ -220,6 +220,14 @@ function _createTmpFile(options, callback) { fs.open(name, CREATE_FLAGS, opts.mode || FILE_MODE, function _fileCreated(err, fd) { if (err) return cb(err); + if (opts.discardDescriptor) { + return fs.close(fd, function _invokeCallback(err) { + cb(null, name, undefined, _prepareTmpFileRemoveCallback(name, -1, opts)); + }); + } + if (opts.detachDescriptor) { + return cb(null, name, fd, _prepareTmpFileRemoveCallback(name, -1, opts)); + } cb(null, name, fd, _prepareTmpFileRemoveCallback(name, fd, opts)); }); }); @@ -345,7 +353,9 @@ function _createTmpDirSync(options) { function _prepareTmpFileRemoveCallback(name, fd, opts) { var removeCallback = _prepareRemoveCallback(function _removeCallback(fdPath) { try { - fs.closeSync(fdPath[0]); + if (0 <= fdPath[0]) { + fs.closeSync(fdPath[0]); + } } catch (e) { // under some node/windows related circumstances, a temporary file diff --git a/test/base.js b/test/base.js index a77f3a5..ef916ff 100644 --- a/test/base.js +++ b/test/base.js @@ -86,6 +86,10 @@ function _testGracefulSync(type, graceful, cb) { _spawnTestWithoutError('graceful-sync.js', [ type, graceful ], cb); } +function _assertNoDescriptor(err, name, fd) { + assert.strictEqual(fd, undefined); +} + function _assertName(err, name) { assert.isString(name); assert.isNotZero(name.length, 'an empty string is not a valid name'); @@ -141,6 +145,7 @@ module.exports.testGraceful = _testGraceful; module.exports.testGracefulSync = _testGracefulSync; module.exports.assertName = _assertName; module.exports.assertNameSync = _assertNameSync; +module.exports.assertNoDescriptor = _assertNoDescriptor; module.exports.testName = _testName; module.exports.testNameSync = _testNameSync; module.exports.testUnsafeCleanup = _testUnsafeCleanup; diff --git a/test/file-test.js b/test/file-test.js index b710859..dde3d09 100644 --- a/test/file-test.js +++ b/test/file-test.js @@ -32,6 +32,35 @@ function _testFile(mode, fdTest) { }; } +function _testFileNoDescriptor(mode) { + return function _testFileNoDescriptor(err, name, fd) { + assert.ok(existsSync(name), 'should exist'); + + var stat = fs.statSync(name); + assert.equal(stat.size, 0, 'should have zero size'); + assert.ok(stat.isFile(), 'should be a file'); + + Test.testStat(stat, mode); + + assert.strictEqual(fd, undefined); + }; +} + +function _testFileAfterDetachRemove(mode) { + return function _testFileAfterDetachRemove(err, name, fd) { + assert.ok(!existsSync(name), 'File should be removed'); + + var fstat = fs.fstatSync(fd); + assert.equal(fstat.size, 0, 'should have zero size'); + assert.ok(fstat.isFile(), 'should be a file'); + Test.testStat(fstat, mode); + + var data = new Buffer('something'); + assert.equal(fs.writeSync(fd, data, 0, data.length, 0), data.length, 'should be writable'); + assert.ok(!fs.closeSync(fd), 'should not return with error'); + }; +} + vows.describe('File creation').addBatch({ 'when using without parameters': { topic: function () { @@ -93,6 +122,31 @@ vows.describe('File creation').addBatch({ } }, + 'when using discardDescriptor': { + topic: function () { + tmp.file({ discardDescriptor: true }, this.callback); + }, + + 'should not return with an error': assert.isNull, + 'should return with a name': Test.assertName, + 'should not return with a descriptor': Test.assertNoDescriptor, + 'should be a file': _testFileNoDescriptor(0100600), + }, + + 'when using detachDescriptor': { + topic: function () { + var cb = this.callback; + tmp.file({ detachDescriptor: true }, function (err, name, fd, removeCallback) { + removeCallback(); + return cb(err, name, fd); + }); + }, + + 'should not return with an error': assert.isNull, + 'should return with a name': Test.assertName, + 'should have working descriptor after removeCallback': _testFileAfterDetachRemove(0100600), + }, + 'when using multiple options': { topic: function () { tmp.file({ prefix: 'foo', postfix: 'bar', mode: 0640 }, this.callback); From 068a2a48d7745fef799904a71a39aff424f0d3b7 Mon Sep 17 00:00:00 2001 From: "Peter A. Bigot" Date: Wed, 12 Oct 2016 04:36:15 -0500 Subject: [PATCH 2/2] treat failure to discard descriptor as an error Addresses comment in pull request #94 Signed-off-by: Peter A. Bigot --- lib/tmp.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/tmp.js b/lib/tmp.js index 39bf070..8bbb53b 100644 --- a/lib/tmp.js +++ b/lib/tmp.js @@ -221,7 +221,19 @@ function _createTmpFile(options, callback) { if (err) return cb(err); if (opts.discardDescriptor) { - return fs.close(fd, function _invokeCallback(err) { + return fs.close(fd, function _discardCallback(err) { + if (err) { + // Low probability, and the file exists, so this could be + // ignored. If it isn't we certainly need to unlink the + // file, and if that fails too its error is more + // important. + try { + fs.unlinkSync(name); + } catch (e) { + err = e; + } + return cb(err); + } cb(null, name, undefined, _prepareTmpFileRemoveCallback(name, -1, opts)); }); }