diff --git a/generate/templates/templates/nodegit.js b/generate/templates/templates/nodegit.js index c5fd769eb..b21e0f6a2 100644 --- a/generate/templates/templates/nodegit.js +++ b/generate/templates/templates/nodegit.js @@ -9,7 +9,20 @@ try { var rawApi = require("node-gyp-build")(path.join(__dirname, "..")); -var promisify = fn => fn && util.promisify(fn); // jshint ignore:line +// The native binding is a process-wide singleton, so when this file is evaluated more than +// once in the same process the methods on the shared classes are no longer the raw +// callback-style natives: they are the promise-returning wrappers installed by an earlier +// evaluation (util.promisify below, or an extension such as lookupWrapper). Re-promisifying +// those makes every call emit a DEP0174 deprecation warning and leak a pending promise per +// extra layer, so only ever wrap the raw natives — anything else is already promise-based +// and is returned untouched. +var promisify = fn => { // jshint ignore:line + if (typeof fn !== "function" || + !Function.prototype.toString.call(fn).includes("[native code]")) { + return fn; + } + return util.promisify(fn); +}; // For disccussion on why `cloneDeep` is required, see: // https://github.com/facebook/jest/issues/3552 diff --git a/test/tests/reload.js b/test/tests/reload.js new file mode 100644 index 000000000..7f289d3b8 --- /dev/null +++ b/test/tests/reload.js @@ -0,0 +1,28 @@ +var path = require("path"); +var childProcess = require("child_process"); +var local = path.join.bind(path, __dirname); + +describe("Reload", function() { + // lib/nodegit.js can be evaluated more than once in a single process while the native + // binding stays a process-wide singleton. Re-evaluations must not re-promisify the shared + // native methods: each extra util.promisify layer emits a DEP0174 deprecation warning on + // every call and leaks a pending promise. Run in a child process so this suite's own + // module cache is untouched. + it("does not re-promisify shared natives when re-evaluated in one process", + function(done) { + this.timeout(30000); + + var fixture = local("../utils/reload_fixture.js"); + childProcess.execFile(process.execPath, [fixture], + function(error, stdout, stderr) { + if (error) { + done(new Error( + "reload fixture exited with code " + error.code + "\n" + stdout + stderr + )); + return; + } + + done(); + }); + }); +}); diff --git a/test/utils/reload_fixture.js b/test/utils/reload_fixture.js new file mode 100644 index 000000000..4cca61bd7 --- /dev/null +++ b/test/utils/reload_fixture.js @@ -0,0 +1,63 @@ +// Evaluates lib/nodegit.js twice in one process (fresh JS module registry, shared native +// binding), then exercises methods that earlier versions would re-promisify. Exits +// non-zero if any DEP0174 deprecation warning is emitted. +var path = require("path"); + +var libDir = path.resolve(__dirname, "..", "..", "lib"); +var nodegitPath = path.join(libDir, "nodegit.js"); +var projectRepoPath = path.resolve(__dirname, "..", ".."); + +var warningCount = 0; +process.on("warning", function(warning) { + if (warning.code === "DEP0174") { + warningCount++; + } +}); + +// The native .node binding stays cached (it is a process-wide singleton no matter what); +// only the package's JS modules are re-evaluated. +function purgePackageJsFromCache() { + Object.keys(require.cache).forEach(function(key) { + var isNativeModule = key.slice(-5) === ".node"; + var isPackageJs = key.indexOf(libDir) === 0 || + key.indexOf("node-gyp-build") !== -1; + + if (!isNativeModule && isPackageJs) { + delete require.cache[key]; + } + }); +} + +var first = require(nodegitPath); + +purgePackageJsFromCache(); +var second = require(nodegitPath); + +if (second === first) { + console.error("fixture failed to re-evaluate nodegit"); + process.exit(2); +} + +// A promisify layer wrapped around an already promise-returning function emits DEP0174 at +// call time, which is what we detect. Commit.lookup goes through the lookupWrapper +// extension, the layer that historically got re-promisified. +second.Repository.open(projectRepoPath) + .then(function(repo) { + return repo.getHeadCommit().then(function(commit) { + return second.Commit.lookup(repo, commit.id()); + }); + }) + .catch(function(error) { + console.error("fixture could not exercise lookups: " + error); + process.exit(2); + }) + .then(function() { + setTimeout(function() { + if (warningCount > 0) { + console.error("DEP0174 warnings after re-evaluation: " + warningCount); + process.exit(1); + } + + process.exit(0); + }, 100); + });