Add basic synchonization to session creation and deletion (#8983)

* Guard sessions and pending drivers lists

* Address review comments

* Improve session deletion

* Perform fast return if the session is not present in the sessions list
This commit is contained in:
Mykola Mokhnach
2017-08-21 20:16:12 +02:00
committed by Isaac A. Murchie
parent 0a0e024565
commit 64ccbc1a98
2 changed files with 75 additions and 54 deletions
+74 -54
View File
@@ -15,8 +15,12 @@ import { MacDriver } from 'appium-mac-driver';
import { EspressoDriver } from 'appium-espresso-driver';
import B from 'bluebird';
import util from 'util';
import AsyncLock from 'async-lock';
const sessionsListGuard = new AsyncLock();
const pendingDriversGuard = new AsyncLock();
class AppiumDriver extends BaseDriver {
constructor (args) {
super();
@@ -26,14 +30,20 @@ class AppiumDriver extends BaseDriver {
this.args = args;
// Access to sessions list must be guarded with a Semaphore, because
// it might be changed by other async calls at any time
// It is not recommended to access this property directly from the outside
this.sessions = {};
// Access to pending drivers list must be guarded with a Semaphore, because
// it might be changed by other async calls at any time
// It is not recommended to access this property directly from the outside
this.pendingDrivers = {};
}
sessionExists (sessionId) {
return _.includes(_.keys(this.sessions), sessionId) &&
this.sessions[sessionId].sessionId !== null;
const dstSession = this.sessions[sessionId];
return dstSession && dstSession.sessionId !== null;
}
driverForSession (sessionId) {
@@ -138,11 +148,11 @@ class AppiumDriver extends BaseDriver {
}
async getSessions () {
let sessions = [];
for (let [id, driver] of _.toPairs(this.sessions)) {
sessions.push({id, capabilities: driver.caps});
}
return sessions;
const sessions = await sessionsListGuard.acquire(AppiumDriver.name, () => this.sessions);
return _.toPairs(sessions)
.map(([id, driver]) => {
return {id, capabilities: driver.caps};
});
}
printNewSessionAnnouncement (driver, caps) {
@@ -163,39 +173,39 @@ class AppiumDriver extends BaseDriver {
let InnerDriver = this.getDriverForCaps(caps);
this.printNewSessionAnnouncement(InnerDriver, caps);
// sessionOverride server flag check
// this will need to be re-thought when we go to multiple session support
if (this.args.sessionOverride && !!this.sessions && _.keys(this.sessions).length > 0) {
log.info('Session override is on. Deleting other sessions.');
for (let id of _.keys(this.sessions)) {
log.info(` Deleting session '${id}'`);
if (this.args.sessionOverride) {
const sessionIdsToDelete = await sessionsListGuard.acquire(AppiumDriver.name, () => _.keys(this.sessions));
if (sessionIdsToDelete.length) {
log.info(`Session override is on. Deleting other ${sessionIdsToDelete.length} active session${sessionIdsToDelete.length ? '' : 's'}.`);
try {
await this.deleteSession(id);
} catch (ign) {
// the error has already been logged in AppiumDriver.deleteSession
// continue
}
await B.map(sessionIdsToDelete, (id) => this.deleteSession(id));
} catch (ign) {}
}
}
let runningDriversData;
let runningDriversData, otherPendingDriversData;
let d = new InnerDriver(this.args);
try {
runningDriversData = this.curSessionDataForDriver(InnerDriver);
runningDriversData = await this.curSessionDataForDriver(InnerDriver);
} catch (e) {
throw new errors.SessionNotCreatedError(e.message);
}
let innerSessionId, dCaps;
let d = new InnerDriver(this.args);
try {
await pendingDriversGuard.acquire(AppiumDriver.name, () => {
this.pendingDrivers[InnerDriver.name] = this.pendingDrivers[InnerDriver.name] || [];
const otherPendingDriversData = this.pendingDrivers[InnerDriver.name].map((drv) => drv.driverData);
otherPendingDriversData = this.pendingDrivers[InnerDriver.name].map((drv) => drv.driverData);
this.pendingDrivers[InnerDriver.name].push(d);
});
let innerSessionId, dCaps;
try {
[innerSessionId, dCaps] = await d.createSession(caps, reqCaps, [...runningDriversData, ...otherPendingDriversData]);
await sessionsListGuard.acquire(AppiumDriver.name, () => {
this.sessions[innerSessionId] = d;
});
} finally {
_.pull(this.pendingDrivers[InnerDriver.name], d);
await pendingDriversGuard.acquire(AppiumDriver.name, () => {
_.pull(this.pendingDrivers[InnerDriver.name], d);
});
}
this.sessions[innerSessionId] = d;
// this is an async function but we don't await it because it handles
// an out-of-band promise which is fulfilled if the inner driver
@@ -228,14 +238,17 @@ class AppiumDriver extends BaseDriver {
}
log.warn(`Closing session, cause was '${e.message}'`);
log.info(`Removing session ${innerSessionId} from our master session list`);
delete this.sessions[innerSessionId];
await sessionsListGuard.acquire(AppiumDriver.name, () => {
delete this.sessions[innerSessionId];
});
}
}
curSessionDataForDriver (InnerDriver) {
let data = _.values(this.sessions)
.filter((s) => s.constructor.name === InnerDriver.name)
.map((s) => s.driverData);
async curSessionDataForDriver (InnerDriver) {
const sessions = await sessionsListGuard.acquire(AppiumDriver.name, () => this.sessions);
const data = _.values(sessions)
.filter((s) => s.constructor.name === InnerDriver.name)
.map((s) => s.driverData);
for (let datum of data) {
if (!datum) {
throw new Error(`Problem getting session data for driver type ` +
@@ -248,22 +261,27 @@ class AppiumDriver extends BaseDriver {
async deleteSession (sessionId) {
try {
if (this.sessions[sessionId]) {
let otherSessionsData = null;
let dstSession = null;
await sessionsListGuard.acquire(AppiumDriver.name, () => {
if (!this.sessions[sessionId]) {
return;
}
const curConstructorName = this.sessions[sessionId].constructor.name;
const otherSessionsData = _.toPairs(this.sessions)
.filter(([key, value]) => value.constructor.name === curConstructorName && key !== sessionId)
.map(([, value]) => value.driverData);
await this.sessions[sessionId].deleteSession(sessionId, otherSessionsData);
}
otherSessionsData = _.toPairs(this.sessions)
.filter(([key, value]) => value.constructor.name === curConstructorName && key !== sessionId)
.map(([, value]) => value.driverData);
dstSession = this.sessions[sessionId];
log.info(`Removing session ${sessionId} from our master session list`);
// regardless of whether the deleteSession completes successfully or not
// make the session unavailable, because who knows what state it might
// be in otherwise
delete this.sessions[sessionId];
});
await dstSession.deleteSession(sessionId, otherSessionsData);
} catch (e) {
log.error(`Had trouble ending session ${sessionId}: ${e.message}`);
throw e;
} finally {
// regardless of whether the deleteSession completes successfully or not
// make the session unavailable, because who knows what state it might
// be in otherwise
log.info(`Removing session ${sessionId} from our master session list`);
delete this.sessions[sessionId];
}
}
@@ -277,25 +295,27 @@ class AppiumDriver extends BaseDriver {
return super.executeCommand(cmd, ...args);
}
let sessionId = args[args.length - 1];
return this.sessions[sessionId].executeCommand(cmd, ...args);
const sessionId = _.last(args);
const dstSession = await sessionsListGuard.acquire(AppiumDriver.name, () => this.sessions[sessionId]);
if (!dstSession) {
throw new Error(`The session with id '${sessionId}' does not exist`);
}
return dstSession.executeCommand(cmd, ...args);
}
proxyActive (sessionId) {
return this.sessions[sessionId] &&
_.isFunction(this.sessions[sessionId].proxyActive) &&
this.sessions[sessionId].proxyActive(sessionId);
const dstSession = this.sessions[sessionId];
return dstSession && _.isFunction(dstSession.proxyActive) && dstSession.proxyActive(sessionId);
}
getProxyAvoidList (sessionId) {
if (!this.sessions[sessionId]) {
return [];
}
return this.sessions[sessionId].getProxyAvoidList();
const dstSession = this.sessions[sessionId];
return dstSession ? dstSession.getProxyAvoidList() : [];
}
canProxy (sessionId) {
return this.sessions[sessionId] && this.sessions[sessionId].canProxy(sessionId);
const dstSession = this.sessions[sessionId];
return dstSession && dstSession.canProxy(sessionId);
}
}
+1
View File
@@ -47,6 +47,7 @@
"appium-xcuitest-driver": "2.x",
"appium-youiengine-driver": "1.x",
"argparse": "1.x",
"async-lock": "^1.0.0",
"asyncbox": "2.x",
"babel-runtime": "=5.8.24",
"bluebird": "2.x",