From 64065141b6e4cd2007b035384d24b5a016f08e62 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Fri, 5 Apr 2024 12:35:47 +0200 Subject: [PATCH] Remove the timeout logic from the TTX driver code The original `test.py` code, see https://github.com/mozilla/pdf.js/blob/c2376e5cea1b3cee89d77e6a4e8b0d9b5b8591be/test/test.py, did not have any timeout logic for TTX, but it got introduced when `test.py` was ported from Python to JavaScript as `test.js` in https://github.com/mozilla/pdf.js/commit/c2376e5cea1b3cee89d77e6a4e8b0d9b5b8591be#diff-a561630bb56b82342bc66697aee2ad96efddcbc9d150665abd6fb7ecb7c0ab2f. However, I don't think we've ever actually seen TTX timing out. Moreover, back then we used a very old version of TTX and ran the font tests on the bots (where a hanging process would block other jobs and would require a manual action to fix), so this code was most likely only included defensively. Fortunately, nowadays it should not be necessary anymore because we use the most recent version of TTX (which either returns the result or errors out, but isn't known to hang on inputs) and we run the font tests on GitHub Actions which doesn't block other jobs anymore and also automatically times the job out for us in the unlikely event that a hang would ever occur. In short, we can safely remove this logic to simplify the code and to get rid of a callback. --- test/font/ttxdriver.mjs | 11 +++-------- test/test.mjs | 20 ++++---------------- 2 files changed, 7 insertions(+), 24 deletions(-) diff --git a/test/font/ttxdriver.mjs b/test/font/ttxdriver.mjs index c820d8ccf..3e51caa4b 100644 --- a/test/font/ttxdriver.mjs +++ b/test/font/ttxdriver.mjs @@ -21,14 +21,9 @@ import { spawn } from "child_process"; let ttxTaskId = Date.now(); -function runTtx(fontPath, registerOnCancel, callback) { +function runTtx(fontPath, callback) { const ttx = spawn("ttx", [fontPath], { stdio: "ignore" }); let ttxRunError; - registerOnCancel(function (reason) { - ttxRunError = reason; - callback(reason); - ttx.kill(); - }); ttx.on("error", function (errorTtx) { ttxRunError = errorTtx; callback( @@ -43,14 +38,14 @@ function runTtx(fontPath, registerOnCancel, callback) { }); } -function translateFont(content, registerOnCancel, callback) { +function translateFont(content, callback) { const buffer = Buffer.from(content, "base64"); const taskId = (ttxTaskId++).toString(); const fontPath = path.join(os.tmpdir(), `pdfjs-font-test-${taskId}.otf`); const resultPath = path.join(os.tmpdir(), `pdfjs-font-test-${taskId}.ttx`); fs.writeFileSync(fontPath, buffer); - runTtx(fontPath, registerOnCancel, function (err) { + runTtx(fontPath, function (err) { fs.unlinkSync(fontPath); if (err) { console.error(err); diff --git a/test/test.mjs b/test/test.mjs index 1080be89d..a6a1e76b4 100644 --- a/test/test.mjs +++ b/test/test.mjs @@ -842,22 +842,10 @@ function unitTestPostHandler(req, res) { }); req.on("end", function () { if (pathname === "/ttx") { - var onCancel = null, - ttxTimeout = 10000; - var timeoutId = setTimeout(function () { - onCancel?.("TTX timeout"); - }, ttxTimeout); - translateFont( - body, - function (fn) { - onCancel = fn; - }, - function (err, xml) { - clearTimeout(timeoutId); - res.writeHead(200, { "Content-Type": "text/xml" }); - res.end(err ? "" + err + "" : xml); - } - ); + translateFont(body, function (err, xml) { + res.writeHead(200, { "Content-Type": "text/xml" }); + res.end(err ? "" + err + "" : xml); + }); return; }