Skip to content

Commit

Permalink
Merge pull request #1539 from github/aeisenberg/unref-delay
Browse files Browse the repository at this point in the history
Avoid unref-ing timer while awaiting status upload
  • Loading branch information
aeisenberg committed Feb 13, 2023
2 parents a25536b + a2487fb commit e00cd12
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 16 deletions.
4 changes: 3 additions & 1 deletion lib/upload-lib.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/upload-lib.js.map

Large diffs are not rendered by default.

22 changes: 16 additions & 6 deletions lib/util.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/util.js.map

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion src/upload-lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,9 @@ export async function waitForProcessing(
util.assertNever(status);
}

await util.delay(STATUS_CHECK_FREQUENCY_MILLISECONDS);
await util.delay(STATUS_CHECK_FREQUENCY_MILLISECONDS, {
allowProcessExit: false,
});
}
} finally {
logger.endGroup();
Expand Down
25 changes: 19 additions & 6 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -548,10 +548,23 @@ export async function bundleDb(
return databaseBundlePath;
}

export async function delay(milliseconds: number) {
// Immediately `unref` the timer such that it only prevents the process from exiting if the
// surrounding promise is being awaited.
return new Promise((resolve) => setTimeout(resolve, milliseconds).unref());
/**
* @param milliseconds time to delay
* @param opts options
* @param opts.allowProcessExit if true, the timer will not prevent the process from exiting
*/
export async function delay(
milliseconds: number,
{ allowProcessExit }: { allowProcessExit: boolean }
) {
return new Promise((resolve) => {
const timer = setTimeout(resolve, milliseconds);
if (allowProcessExit) {
// Immediately `unref` the timer such that it only prevents the process from exiting if the
// surrounding promise is being awaited.
timer.unref();
}
});
}

export function isGoodVersion(versionSpec: string) {
Expand Down Expand Up @@ -748,7 +761,7 @@ export async function withTimeout<T>(
return result;
};
const timeoutTask = async () => {
await delay(timeoutMs);
await delay(timeoutMs, { allowProcessExit: true });
if (!finished) {
// Workaround: While the promise racing below will allow the main code
// to continue, the process won't normally exit until the asynchronous
Expand All @@ -773,7 +786,7 @@ export async function checkForTimeout() {
core.info(
"A timeout occurred, force exiting the process after 30 seconds to prevent hanging."
);
await delay(30_000);
await delay(30_000, { allowProcessExit: true });
process.exit();
}
}
Expand Down

0 comments on commit e00cd12

Please sign in to comment.