From 16f01e6289162219c7585da123e1ad5cc6c46063 Mon Sep 17 00:00:00 2001 From: Edoardo Pirovano Date: Wed, 9 Nov 2022 17:08:44 +0000 Subject: [PATCH] Force exit of process if a timeout has occurred --- src/analyze-action.ts | 2 ++ src/init-action.ts | 2 ++ src/util.ts | 33 +++++++++++++++++++++++++++++++-- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/analyze-action.ts b/src/analyze-action.ts index 7ca5a2bd5..9c660a3a8 100644 --- a/src/analyze-action.ts +++ b/src/analyze-action.ts @@ -27,6 +27,7 @@ import { getTotalCacheSize, uploadTrapCaches } from "./trap-caching"; import * as upload_lib from "./upload-lib"; import { UploadResult } from "./upload-lib"; import * as util from "./util"; +import { checkForTimeout } from "./util"; // eslint-disable-next-line import/no-commonjs const pkg = require("../package.json"); @@ -402,6 +403,7 @@ async function runWrapper() { core.setFailed(`analyze action failed: ${error}`); console.log(error); } + await checkForTimeout(); } void runWrapper(); diff --git a/src/init-action.ts b/src/init-action.ts index 1c24259d0..17c60bee4 100644 --- a/src/init-action.ts +++ b/src/init-action.ts @@ -29,6 +29,7 @@ import { parseRepositoryNwo } from "./repository"; import { getTotalCacheSize } from "./trap-caching"; import { checkActionVersion, + checkForTimeout, checkGitHubVersionInRange, codeQlVersionAbove, DEFAULT_DEBUG_ARTIFACT_NAME, @@ -339,6 +340,7 @@ async function runWrapper() { core.setFailed(`init action failed: ${error}`); console.log(error); } + await checkForTimeout(); } void runWrapper(); diff --git a/src/util.ts b/src/util.ts index 476ea8eaf..da345244d 100644 --- a/src/util.ts +++ b/src/util.ts @@ -860,11 +860,17 @@ export async function tryGetFolderBytes( /** * Run a promise for a given amount of time, and if it doesn't resolve within - * that time, call the provided callback and then return undefined. + * that time, call the provided callback and then return undefined. Due to the + * limitation outlined below, using this helper function is not recommended + * unless there is no other option for adding a timeout (e.g. the code that + * would need the timeout added is an external library). * * Important: This does NOT cancel the original promise, so that promise will * continue in the background even after the timeout has expired. If the * original promise hangs, then this will prevent the process terminating. + * If a timeout has occurred then the global hadTimeout variable will get set + * to true, and the caller is responsible for forcing the process to exit + * if this is the case by calling the `checkForTimeout` function. * * @param timeoutMs The timeout in milliseconds. * @param promise The promise to run. @@ -884,7 +890,14 @@ export async function withTimeout( }; const timeout: Promise = new Promise((resolve) => { setTimeout(() => { - if (!finished) onTimeout(); + if (!finished) { + // Workaround: While the promise racing below will allow the main code + // to continue, the process won't normally exit until the asynchronous + // task in the background has finished. We set this variable to force + // an exit at the end of our code. + globalThis.hadTimeout = true; + onTimeout(); + } resolve(undefined); }, timeoutMs); }); @@ -892,6 +905,22 @@ export async function withTimeout( return await Promise.race([mainTask(), timeout]); } +/** + * Check if the global hadTimeout variable has been set, and if so then + * exit the process to ensure any background tasks that are still running + * are killed. This should be called at the end of execution if the + * `withTimeout` function has been used. + */ +export async function checkForTimeout() { + if (globalThis.hadTimeout === true) { + core.info( + "A timeout occurred, force exiting the process after 30 seconds to prevent hanging." + ); + await delay(30_000); + process.exit(); + } +} + /** * This function implements a heuristic to determine whether the * runner we are on is hosted by GitHub. It does this by checking