Skip to content

Commit

Permalink
Create/use global shared retry queue.
Browse files Browse the repository at this point in the history
Importing a fresh copy of graceful-fs then using `require('fs').close`
would only initiate retry of the queue from the first copy of
graceful-fs, so needed retries associated with subsequent instances of
graceful-fs would not be executed.
  • Loading branch information
coreyfarrell committed Aug 5, 2019
1 parent 841bf6d commit ea9e6bc
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 46 deletions.
82 changes: 41 additions & 41 deletions graceful-fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ var polyfills = require('./polyfills.js')
var legacy = require('./legacy-streams.js')
var clone = require('./clone.js')

var queue = []

var util = require('util')

var gracefulQueue = Symbol.for('graceful-fs.queue')

function noop () {}

var debug = noop
Expand All @@ -19,11 +19,44 @@ else if (/\bgfs4\b/i.test(process.env.NODE_DEBUG || ''))
console.error(m)
}

if (/\bgfs4\b/i.test(process.env.NODE_DEBUG || '')) {
process.on('exit', function() {
debug(queue)
require('assert').equal(queue.length, 0)
// Once time initialization
if (!global[gracefulQueue]) {
// This queue can be shared by multiple loaded instances
var queue = []
Object.defineProperty(global, gracefulQueue, {
get: function() {
return queue
}
})

// Patch fs.close/closeSync to shared queue version, because we need
// to retry() whenever a close happens *anywhere* in the program.
// This is essential when multiple graceful-fs instances are
// in play at the same time.
fs.close = (function (fs$close) { return function (fd, cb) {
return fs$close.call(fs, fd, function (err) {
// This function uses the graceful-fs shared queue
if (!err) {
retry()
}

if (typeof cb === 'function')
cb.apply(this, arguments)
})
}})(fs.close)

fs.closeSync = (function (fs$closeSync) { return function (fd) {
// This function uses the graceful-fs shared queue
fs$closeSync.apply(fs, arguments)
retry()
}})(fs.closeSync)

if (/\bgfs4\b/i.test(process.env.NODE_DEBUG || '')) {
process.on('exit', function() {
debug(global[gracefulQueue])
require('assert').equal(global[gracefulQueue].length, 0)
})
}
}

module.exports = patch(clone(fs))
Expand All @@ -32,39 +65,6 @@ if (process.env.TEST_GRACEFUL_FS_GLOBAL_PATCH && !fs.__patched) {
fs.__patched = true;
}

// Always patch fs.close/closeSync, because we want to
// retry() whenever a close happens *anywhere* in the program.
// This is essential when multiple graceful-fs instances are
// in play at the same time.
module.exports.close = (function (fs$close) { return function (fd, cb) {
return fs$close.call(fs, fd, function (err) {
if (!err)
retry()

if (typeof cb === 'function')
cb.apply(this, arguments)
})
}})(fs.close)

module.exports.closeSync = (function (fs$closeSync) { return function (fd) {
// Note that graceful-fs also retries when fs.closeSync() fails.
// Looks like a bug to me, although it's probably a harmless one.
var rval = fs$closeSync.apply(fs, arguments)
retry()
return rval
}})(fs.closeSync)

// Only patch fs once, otherwise we'll run into a memory leak if
// graceful-fs is loaded multiple times, such as in test environments that
// reset the loaded modules between tests.
// We look for the string `graceful-fs` from the comment above. This
// way we are not adding any extra properties and it will detect if older
// versions of graceful-fs are installed.
if (!/\bgraceful-fs\b/.test(fs.closeSync.toString())) {
fs.closeSync = module.exports.closeSync;
fs.close = module.exports.close;
}

function patch (fs) {
// Everything that references the open() function needs to be in here
polyfills(fs)
Expand Down Expand Up @@ -267,11 +267,11 @@ function patch (fs) {

function enqueue (elem) {
debug('ENQUEUE', elem[0].name, elem[1])
queue.push(elem)
global[gracefulQueue].push(elem)
}

function retry () {
var elem = queue.shift()
var elem = global[gracefulQueue].shift()
if (elem) {
debug('RETRY', elem[0].name, elem[1])
elem[0].apply(null, elem[1])
Expand Down
22 changes: 17 additions & 5 deletions test/close.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,22 @@
var fs$close = require('fs').close;
var fs$closeSync = require('fs').closeSync;
var fs = require('../');
var fs = require('fs')
var path = require('path')
var gfsPath = path.resolve(__dirname, '..', 'graceful-fs.js')
var gfs = require(gfsPath)
var importFresh = require('import-fresh')
var fs$close = fs.close
var fs$closeSync = fs.closeSync
var test = require('tap').test

test('`close` is patched correctly', function(t) {
t.notEqual(fs.close, fs$close, 'patch close');
t.notEqual(fs.closeSync, fs$closeSync, 'patch closeSync');
t.match(fs$close.toString(), /graceful-fs shared queue/, 'patch fs.close');
t.match(fs$closeSync.toString(), /graceful-fs shared queue/, 'patch fs.closeSync');
t.match(gfs.close.toString(), /graceful-fs shared queue/, 'patch gfs.close');
t.match(gfs.closeSync.toString(), /graceful-fs shared queue/, 'patch gfs.closeSync');

var newGFS = importFresh(gfsPath)
t.equal(fs.close, fs$close)
t.equal(fs.closeSync, fs$closeSync)
t.equal(newGFS.close, fs$close)
t.equal(newGFS.closeSync, fs$closeSync)
t.end();
})

0 comments on commit ea9e6bc

Please sign in to comment.