Tests on worker thread terminate while doing a checkout

AlexaXs · AlexaXs · commit caec8195155d · 2022-02-17T09:45:17.000+01:00
Checkout leverages libgit2 threads and when applying filters it runs JS callbacks. These tests check that when running checkout on a worker thread and this is terminated, it exists gracefully without memory leaks.
diff --git a/test/tests/filter.js b/test/tests/filter.js
@@ -335,7 +335,7 @@ describe("Filter", function() {
 
     it("can run sync callback on checkout without deadlocking", function() { // jshint ignore:line
       var test = this;
-      var syncCallbackResult = true;
+      var syncCallbackResult = 1;
 
       return Registry.register(filterName, {
         apply: function() {
@@ -365,9 +365,10 @@ describe("Filter", function() {
         });
     });
 
+    // Temporary workaround for LFS checkout. Test skipped.
+    // To activate when reverting workaround.
     // 'Checkout.head' and 'Submodule.lookup' do work with the repo locked.
     // They should work together without deadlocking.
-    // Temporary workaround for LFS checkout. Test skipped to be reverted.
     it.skip("can run async callback on checkout without deadlocking", function() { // jshint ignore:line
       var test = this;
       var submoduleNameIn = "vendor/libgit2";
diff --git a/test/tests/worker.js b/test/tests/worker.js
@@ -2,7 +2,9 @@ const path = require("path");
 const assert = require("assert");
 const fse = require("fs-extra");
 const local = path.join.bind(path, __dirname);
+const NodeGit = require("../../");
 
+let filterName = "psuedo_filter";
 let Worker;
 
 try {
@@ -24,6 +26,15 @@ if (Worker) {
       });
     });
 
+    afterEach(function() {
+      return NodeGit.FilterRegistry.unregister(filterName)
+        .catch(function(error) {
+          if (error === NodeGit.Error.CODE.ERROR) {
+            throw new Error("Cannot unregister filter");
+          }
+        });
+    });
+
     it("can perform basic functionality via worker thread", function(done) {
       const workerPath = local("../utils/worker.js");
       const worker = new Worker(workerPath, {
@@ -127,5 +138,78 @@ if (Worker) {
         }
       });
     });
+
+    // This tests that while calling filter's apply callbacks and the worker
+    // is terminated, node exits gracefully. To make sure we terminate the
+    // worker during a checkout, continuous checkouts will be running in a loop.
+    it("can kill worker thread while doing a checkout and exit gracefully", function(done) { // jshint ignore:line
+      const workerPath = local("../utils/worker_checkout.js");
+      const worker = new Worker(workerPath, {
+        workerData: {
+          clonePath,
+          url: "https://github.com/nodegit/test.git"
+        }
+      });
+      worker.on("message", (message) => {
+        switch (message) {
+          case "init":
+            // give enough time for the worker to start applying the filter
+            // during continuous checkouts
+            setTimeout(() => { worker.terminate(); }, 10000);
+            break;
+          case "success":
+            assert.fail();
+            break;
+          case "failure":
+            assert.fail();
+            break;
+        }
+      });
+      worker.on("error", () => assert.fail());
+      worker.on("exit", (code) => {
+        if (code == 1) {
+          done();
+        } else {
+          assert.fail();
+        }
+      });
+    });
+
+    // This tests that after calling filter's apply callbacks and the worker
+    // is terminated, there will be no memory leaks.
+    it("can track objects to free on context shutdown after multiple checkouts", function(done) { // jshint ignore:line
+      let testOk;
+      const workerPath = local("../utils/worker_context_aware_checkout.js");
+      const worker = new Worker(workerPath, {
+        workerData: {
+          clonePath,
+          url: "https://github.com/nodegit/test.git"
+        }
+      });
+      worker.on("message", (message) => {
+        switch (message) {
+          case "numbersMatch":
+            testOk = true;
+            worker.terminate();
+            break;
+          case "numbersDoNotMatch":
+            testOk = false;
+            worker.terminate();
+            break;
+          case "failure":
+            assert.fail();
+            break;
+        }
+      });
+      worker.on("error", () => assert.fail());
+      worker.on("exit", (code) => {
+        if (code === 1 && testOk === true) {
+          done();
+        }
+        else {
+          assert.fail();
+        }
+      });
+    });
   });
 }
diff --git a/test/utils/loop_checkout.js b/test/utils/loop_checkout.js
@@ -0,0 +1,91 @@
+const fse = require("fs-extra");
+const path = require("path");
+const NodeGit = require("../../");
+
+const getDirExtFiles = function(dir, ext, done) {
+  let results = [];
+  fse.readdir(dir, function(err, list) {
+    if (err) {
+      return done(err);
+    }
+    let i = 0;
+    (function next() {
+      let file = list[i++];
+      if (!file) {
+        return done(null, results);
+      }
+      file = path.resolve(dir, file);
+      fse.stat(file, function(err, stat) {
+        if (stat && stat.isDirectory()) {
+          getDirExtFiles(file, ext, function(err, res) {
+            results = results.concat(res);
+            next();
+          });
+        } else {
+          if (path.extname(file) == ".".concat(ext)) {
+            results.push(file);
+          }
+          next();
+        }
+      });
+    })();
+  });
+};
+
+const getDirFilesToChange = function(dir, ext) {
+  return new Promise(function(resolve, reject) {
+    getDirExtFiles(dir, ext, function(err, results) {
+      if (err) {
+        reject(err);
+      }
+      resolve(results);
+    });
+  });
+};
+
+// Changes the content of files with extension 'ext'
+// in directory 'dir' recursively.
+// Returns relative file paths
+const changeDirExtFiles = function (dir, ext, newText) {
+  let filesChanged = [];
+  return getDirFilesToChange(dir, ext)
+  .then(function(filesWithExt) {
+    filesWithExt.forEach(function(file) {
+      fse.writeFile(
+        file,
+        newText,
+        { encoding: "utf-8" }
+      );
+      filesChanged.push(path.relative(dir, file));
+    });
+    return filesChanged;
+  })
+  .catch(function(err) {
+    throw new Error("Error getting files with extension .".concat(ext));
+  });
+};
+
+// 'times' to limit the number of iterations in the loop.
+// 0 means no limit.
+const loopingCheckoutHead = async function(repoPath, repo, times) {
+  const text0 = "Text0: changing content to trigger checkout";
+  const text1 = "Text1: changing content to trigger checkout";
+
+  let iteration = 0;
+  for (let i = 0; true; i = ++i%2) {
+    const newText = (i == 0) ? text0 : text1;
+    const jsRelativeFilePahts = await changeDirExtFiles(repoPath, "js", newText);  // jshint ignore:line
+    let checkoutOpts = {
+      checkoutStrategy: NodeGit.Checkout.STRATEGY.FORCE,
+      paths: jsRelativeFilePahts
+    };
+    await NodeGit.Checkout.head(repo, checkoutOpts);
+
+    if (++iteration == times) {
+      break;
+    }
+  }
+  return;
+};
+
+module.exports = loopingCheckoutHead;
diff --git a/test/utils/worker_checkout.js b/test/utils/worker_checkout.js
@@ -0,0 +1,51 @@
+const {
+  isMainThread,
+  parentPort,
+  workerData
+} = require("worker_threads");
+const assert = require("assert");
+const NodeGit = require("../../");
+const loopingCheckoutHead = require("./loop_checkout.js");
+
+if (isMainThread) {
+  throw new Error("Must be run via worker thread");
+}
+
+parentPort.postMessage("init");
+
+const { clonePath, url } = workerData;
+const cloneOpts = {
+    fetchOpts: {
+      callbacks: {
+        certificateCheck: () => 0
+    }
+  }
+};
+
+let repository;
+let filterName = "psuedo_filter";
+let applyCallbackResult = 1;
+
+return NodeGit.Clone(url, clonePath, cloneOpts)
+.then(function(_repository) {
+  repository = _repository;
+  assert.ok(repository instanceof NodeGit.Repository);
+  return NodeGit.FilterRegistry.register(filterName, {
+    apply: function() {
+      applyCallbackResult = 0;
+    },
+    check: function() {
+      return NodeGit.Error.CODE.OK;
+    }
+  }, 0);
+})
+.then(function(result) {
+  assert.strictEqual(result, NodeGit.Error.CODE.OK);
+  return loopingCheckoutHead(clonePath, repository, 0);
+}).then(function() {
+  assert.strictEqual(applyCallbackResult, 0);
+  parentPort.postMessage("success");
+})
+.catch((err) => { 
+  parentPort.postMessage("failure");
+});
diff --git a/test/utils/worker_context_aware.js b/test/utils/worker_context_aware.js
@@ -41,7 +41,7 @@ return NodeGit.Clone(url, clonePath, opts)
       garbageCollect();
 
       // Count total of objects left after being created/destroyed
-      const selfFreeingCount =
+      const freeingCount =
         NodeGit.Cert.getNonSelfFreeingConstructedCount() +
         NodeGit.Repository.getSelfFreeingInstanceCount() +
         NodeGit.Commit.getSelfFreeingInstanceCount() +
@@ -50,7 +50,7 @@ return NodeGit.Clone(url, clonePath, opts)
 
       const numberOfTrackedObjects = NodeGit.getNumberOfTrackedObjects();
 
-      if (selfFreeingCount === numberOfTrackedObjects) {
+      if (freeingCount === numberOfTrackedObjects) {
         parentPort.postMessage("numbersMatch");
       }
       else {
diff --git a/test/utils/worker_context_aware_checkout.js b/test/utils/worker_context_aware_checkout.js
@@ -0,0 +1,66 @@
+const {
+  isMainThread,
+  parentPort,
+  workerData
+} = require("worker_threads");
+const garbageCollect = require("./garbage_collect.js");
+const assert = require("assert");
+const NodeGit = require("../../");
+const loopingCheckoutHead = require("./loop_checkout.js");
+const { promisify } = require("util");
+
+if (isMainThread) {
+  throw new Error("Must be run via worker thread");
+}
+
+const { clonePath, url } = workerData;
+const cloneOpts = {
+    fetchOpts: {
+      callbacks: {
+        certificateCheck: () => 0
+    }
+  }
+};
+
+let repository;
+let filterName = "psuedo_filter";
+let applyCallbackResult = 1;
+
+return NodeGit.Clone(url, clonePath, cloneOpts)
+.then(function(_repository) {
+  repository = _repository;
+  assert.ok(repository instanceof NodeGit.Repository);
+  return NodeGit.FilterRegistry.register(filterName, {
+    apply: function() {
+      applyCallbackResult = 0;
+    },
+    check: function() {
+      return NodeGit.Error.CODE.OK;
+    }
+  }, 0);
+})
+.then(function(result) {
+  assert.strictEqual(result, NodeGit.Error.CODE.OK);
+  return loopingCheckoutHead(clonePath, repository, 10);
+}).then(function() {
+  assert.strictEqual(applyCallbackResult, 0);
+  // Tracked objects must work too when the Garbage Collector is triggered
+  garbageCollect();
+
+  // Count total of objects left after being created/destroyed
+  const freeingCount =
+    NodeGit.Cert.getNonSelfFreeingConstructedCount() +
+    NodeGit.FilterSource.getNonSelfFreeingConstructedCount() +
+    NodeGit.Buf.getNonSelfFreeingConstructedCount() +
+    NodeGit.Repository.getSelfFreeingInstanceCount();
+
+  const numberOfTrackedObjects = NodeGit.getNumberOfTrackedObjects();
+
+  if (freeingCount === numberOfTrackedObjects) {
+    parentPort.postMessage("numbersMatch");
+  }
+  else {
+    parentPort.postMessage("numbersDoNotMatch");
+  }
+  return promisify(setTimeout)(50000);
+}).catch((err) => parentPort.postMessage("failure"));
