diff --git a/graceful-fs.js b/graceful-fs.js index 8d5b89e..8cd088c 100644 --- a/graceful-fs.js +++ b/graceful-fs.js @@ -22,11 +22,29 @@ if (typeof Symbol === 'function' && typeof Symbol.for === 'function') { function noop () {} function publishQueue(context, queue) { - Object.defineProperty(context, gracefulQueue, { - get: function() { - return queue - } - }) + // ESM `node:fs` namespace objects are not extensible (#245). + // Keep the Symbol-on-fs / Symbol-on-global sharing path when possible. + if (!context || (typeof Object.isExtensible === 'function' && !Object.isExtensible(context))) { + return + } + try { + Object.defineProperty(context, gracefulQueue, { + get: function() { + return queue + } + }) + } catch (err) { + // ignore sealed / host objects that still reject the new property + } +} + +// Shared retry queue. Prefer the published Symbol on `fs` so multiple +// copies of this module see the same array. When `fs` is not extensible, +// fall back to global (already used for cross-version sharing) and then +// a module-local array so loading never throws. +var fallbackQueue = [] +function getQueue () { + return fs[gracefulQueue] || global[gracefulQueue] || fallbackQueue } var debug = noop @@ -42,7 +60,7 @@ else if (/\bgfs4\b/i.test(process.env.NODE_DEBUG || '')) // Once time initialization if (!fs[gracefulQueue]) { // This queue can be shared by multiple loaded instances - var queue = global[gracefulQueue] || [] + var queue = global[gracefulQueue] || fallbackQueue publishQueue(fs, queue) // Patch fs.close/closeSync to shared queue version, because we need @@ -83,14 +101,14 @@ if (!fs[gracefulQueue]) { if (/\bgfs4\b/i.test(process.env.NODE_DEBUG || '')) { process.on('exit', function() { - debug(fs[gracefulQueue]) - require('assert').equal(fs[gracefulQueue].length, 0) + debug(getQueue()) + require('assert').equal(getQueue().length, 0) }) } } if (!global[gracefulQueue]) { - publishQueue(global, fs[gracefulQueue]); + publishQueue(global, getQueue()); } module.exports = patch(clone(fs)) @@ -370,7 +388,7 @@ function patch (fs) { function enqueue (elem) { debug('ENQUEUE', elem[0].name, elem[1]) - fs[gracefulQueue].push(elem) + getQueue().push(elem) retry() } @@ -382,12 +400,13 @@ var retryTimer // delay between attempts so that we'll retry these jobs sooner function resetQueue () { var now = Date.now() - for (var i = 0; i < fs[gracefulQueue].length; ++i) { + var queue = getQueue() + for (var i = 0; i < queue.length; ++i) { // entries that are only a length of 2 are from an older version, don't // bother modifying those since they'll be retried anyway. - if (fs[gracefulQueue][i].length > 2) { - fs[gracefulQueue][i][3] = now // startTime - fs[gracefulQueue][i][4] = now // lastTime + if (queue[i].length > 2) { + queue[i][3] = now // startTime + queue[i][4] = now // lastTime } } // call retry to make sure we're actively processing the queue @@ -399,10 +418,11 @@ function retry () { clearTimeout(retryTimer) retryTimer = undefined - if (fs[gracefulQueue].length === 0) + var queue = getQueue() + if (queue.length === 0) return - var elem = fs[gracefulQueue].shift() + var elem = queue.shift() var fn = elem[0] var args = elem[1] // these items may be unset if they were added by an older graceful-fs @@ -437,7 +457,7 @@ function retry () { } else { // if we can't do this job yet, push it to the end of the queue // and let the next iteration check again - fs[gracefulQueue].push(elem) + queue.push(elem) } } diff --git a/test/non-extensible-fs.js b/test/non-extensible-fs.js new file mode 100644 index 0000000..cb05d00 --- /dev/null +++ b/test/non-extensible-fs.js @@ -0,0 +1,119 @@ +'use strict' + +// Regression for https://github.com/isaacs/node-graceful-fs/issues/245 +// +// In ESM, `import * as fs from 'node:fs'` is a non-extensible namespace +// object. graceful-fs used to throw: +// TypeError: Cannot define property Symbol(graceful-fs.queue), object is not extensible +// +// nyc (via package-hash) preloads node_modules/graceful-fs onto the real +// `fs` before tests run, so we load *this* tree's graceful-fs.js against a +// cloned, Object.preventExtensions fs-like object instead. + +var Module = require('module') +var path = require('path') +var clone = require('../clone.js') +var realFs = require('fs') +var test = require('tap').test + +var filename = path.resolve(__dirname, '..', 'graceful-fs.js') +var queueSymbol = typeof Symbol === 'function' && typeof Symbol.for === 'function' + ? Symbol.for('graceful-fs.queue') + : '___graceful-fs.queue' + +function loadGracefulFs (fakeFs) { + var prev = process.env.TEST_GRACEFUL_FS_GLOBAL_PATCH + delete process.env.TEST_GRACEFUL_FS_GLOBAL_PATCH + + var m = new Module(filename + '-non-extensible-' + Date.now() + '-' + Math.random(), module) + m.filename = filename + m.paths = Module._nodeModulePaths(path.dirname(filename)) + var protoRequire = Module.prototype.require + m.require = function (request) { + if (request === 'fs') + return fakeFs + return protoRequire.call(this, request) + } + + try { + m.load(filename) + return m.exports + } finally { + if (prev !== undefined) + process.env.TEST_GRACEFUL_FS_GLOBAL_PATCH = prev + } +} + +function nonExtensibleFs () { + var fakeFs = clone(realFs) + Object.preventExtensions(fakeFs) + return fakeFs +} + +test('loading must not throw when fs is not extensible', function (t) { + var fakeFs = nonExtensibleFs() + t.equal(Object.isExtensible(fakeFs), false) + + var gfs + t.doesNotThrow(function () { + gfs = loadGracefulFs(fakeFs) + }, 'loading graceful-fs does not throw') + + t.type(gfs.readFile, 'function') + t.type(gfs.openSync, 'function') + t.type(gfs.closeSync, 'function') + + t.equal(fakeFs[queueSymbol], undefined, + 'does not define the queue Symbol on a non-extensible fs') + t.ok(Array.isArray(global[queueSymbol]), + 'queue is stored off fs (on global) so copies can still share it') + + var fd = gfs.openSync(filename, 'r') + gfs.closeSync(fd) + + gfs.readFile(filename, 'utf8', function (err, data) { + t.error(err, 'readFile still works') + t.match(data, /not extensible/) + t.end() + }) +}) + +test('later loads share the same queue via global', function (t) { + var fakeFs = nonExtensibleFs() + loadGracefulFs(fakeFs) + var q1 = global[queueSymbol] + t.ok(Array.isArray(q1)) + + var gfs2 = loadGracefulFs(fakeFs) + t.type(gfs2.readFile, 'function') + t.equal(global[queueSymbol], q1, + 'second load keeps the shared queue instance') + t.equal(fakeFs[queueSymbol], undefined) + t.end() +}) + +test('EMFILE retries use the off-fs queue', function (t) { + var realNow = Date.now + var EMFILE = Object.assign(new Error('FAKE EMFILE'), { code: 'EMFILE' }) + var fakeFs = clone(realFs) + + t.teardown(function () { + Date.now = realNow + }) + + fakeFs.readFile = function (p, options, cb) { + process.nextTick(function () { + cb(EMFILE) + Date.now = function () { + return realNow() + 60000 + } + }) + } + Object.preventExtensions(fakeFs) + + var gfs = loadGracefulFs(fakeFs) + gfs.readFile('literally anything', function (err) { + t.equal(err.code, 'EMFILE', 'eventually got the EMFILE from the shared queue') + t.end() + }) +}) diff --git a/test/queue-published-on-fs.js b/test/queue-published-on-fs.js new file mode 100644 index 0000000..f55c033 --- /dev/null +++ b/test/queue-published-on-fs.js @@ -0,0 +1,21 @@ +'use strict' + +// Lock the historical CJS sharing path: when `fs` is extensible, the +// retry queue is published on `fs` via Symbol.for('graceful-fs.queue') +// so multiple graceful-fs copies see the same array (#245 fallback +// must not change this). + +var fs = require('fs') +var t = require('tap') + +var queueSymbol = typeof Symbol === 'function' && typeof Symbol.for === 'function' + ? Symbol.for('graceful-fs.queue') + : '___graceful-fs.queue' + +t.ok(Object.isExtensible(fs), 'CJS fs is extensible') + +var gfs = require('../') +t.type(gfs.readFile, 'function') +t.ok(Array.isArray(fs[queueSymbol]), 'queue published on fs when extensible') +t.ok(Array.isArray(global[queueSymbol]), 'queue also published on global') +t.equal(fs[queueSymbol], global[queueSymbol], 'fs and global share one queue array')