Skip to content

Commit

Permalink
fix: Fix hung launcher on shutdown of Safari (#38)
Browse files Browse the repository at this point in the history
In some cases (notably macOS Safari under GitHub Actions), a call to
forceKill() would be triggered during another call to forceKill().
This could cause the second Promise to go unresolved, leading to a
hang.  On GitHub, eventually, the workflow would be cancelled.

This fixes the nested calls to forceKill by making them both resolve
on the same event (the shutdown triggered by the first call).

This also adds a timeout for shutting down a WebDriver session.
Although this does not appear to be the root cause of the hang we
were experiencing in GitHub Actions workflows, it should be safer to
have this timeout.  If we can't stop a WebDriver session gracefully,
we will timeout after 5s and end the launcher anyway.

Closes #24
  • Loading branch information
joeyparrish committed Apr 20, 2022
1 parent 76a000e commit adc7898
Showing 1 changed file with 28 additions and 5 deletions.
33 changes: 28 additions & 5 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ const {installWebDrivers} = require('webdriver-installer');
const DRIVER_CACHE = path.join(os.homedir(), '.webdriver-installer-cache');
fs.mkdirSync(DRIVER_CACHE, {recursive: true});

// Delay on startup to allow the WebDriver server to start.
const WEBDRIVER_STARTUP_DELAY_SECONDS = 2;

// If it takes longer than this to close our WebDriver session, give up.
const CLOSE_WEBDRIVER_SESSION_TIMEOUT_SECONDS = 5;

let driversInstalledPromise = null;

// Map nodejs OS names to Selenium platform names.
Expand Down Expand Up @@ -91,7 +97,7 @@ const LocalWebDriverBase = function(baseBrowserDecorator, args, logger) {
});

this.on('start', async (url) => {
await new Promise((resolve) => setTimeout(resolve, 2000));
await delay(WEBDRIVER_STARTUP_DELAY_SECONDS);

this.browser.init(this.spec, (error) => {
if (error) {
Expand All @@ -116,9 +122,18 @@ const LocalWebDriverBase = function(baseBrowserDecorator, args, logger) {
await this.stopWebdriver_();
};

this.forceKillOperation_ = null;

this.forceKill = async () => {
// Don't nest force-kill operations. If forceKill() was already called,
// just return the same Promise again.
if (this.state == 'BEING_FORCE_KILLED') {
return this.forceKillOperation_;
}

this.state = 'BEING_FORCE_KILLED';
await this.stopWebdriver_();
this.forceKillOperation_ = this.stopWebdriver_();
await this.forceKillOperation_;
};

const originalStart = this.start;
Expand Down Expand Up @@ -173,17 +188,25 @@ const LocalWebDriverBase = function(baseBrowserDecorator, args, logger) {

this.stopWebdriver_ = async () => {
if (this.browser) {
await new Promise(resolve => this.browser.quit(resolve));
// If it takes too long to close the session, give up and move on.
await Promise.race([
delay(CLOSE_WEBDRIVER_SESSION_TIMEOUT_SECONDS),
new Promise(resolve => this.browser.quit(resolve)),
]);
}

// Now that the driver connection and browser are closed, emit the signal
// that shuts down the driver executable.
// Now that the driver connection and browser are closed (or have timed
// out), emit the signal that shuts down the driver executable.
await this.emitAsync('kill');

this.state = 'FINISHED';
};
}

async function delay(seconds) {
await new Promise((resolve) => setTimeout(resolve, seconds * 1000));
}

// Generate a subclass of LocalWebDriverBase and return it.
function generateSubclass(
browserName, launcherName, driverCommand, getDriverArgs,
Expand Down

0 comments on commit adc7898

Please sign in to comment.