fix: do not throw when fs is not extensible

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)
     }
   }
 

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()
+  })
+})

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')