mirror of
https://github.com/appium/appium.git
synced 2026-01-26 12:18:51 -06:00
fix async bug in ios crash waterfall
previously, we'd respond to the client's current callback and then start the shutdown sequence. this opened up a situation where the client could then call another command (say driver.quit) during shutdown, enabling the possibility of two simultaneous shutdown flows causing undefined behavior. this fix makes sure we don't respond to the client for a command that caused a crash until we're done shutting down.
This commit is contained in:
@@ -37,6 +37,12 @@ var logTypesSupported = {
|
||||
'crashlog': 'Crash logs for iOS applications on real devices and simulators'
|
||||
};
|
||||
|
||||
iOSController.getStatusExtensions = function () {
|
||||
var ext = {};
|
||||
ext.isShuttingDown = this.isShuttingDown; // this is for testing purposes
|
||||
return ext;
|
||||
};
|
||||
|
||||
iOSController.createGetElementCommand = function (strategy, selector, ctx,
|
||||
many) {
|
||||
var ext = many ? 's' : '';
|
||||
|
||||
@@ -122,6 +122,7 @@ IOS.prototype.init = function () {
|
||||
this.landscapeWebCoordsOffset = 0;
|
||||
this.localizableStrings = {};
|
||||
this.keepAppToRetainPrefs = false;
|
||||
this.isShuttingDown = false;
|
||||
};
|
||||
|
||||
IOS.prototype._deviceConfigure = Device.prototype.configure;
|
||||
@@ -440,22 +441,24 @@ IOS.prototype.configureBootstrap = function (cb) {
|
||||
|
||||
IOS.prototype.onUnexpectedInstrumentsExit = function (code) {
|
||||
logger.debug("Instruments exited unexpectedly");
|
||||
if (typeof this.cbForCurrentCmd === "function") {
|
||||
// we were in the middle of waiting for a command when it died
|
||||
// so let's actually respond with something
|
||||
var error = new UnknownError("Instruments died while responding to " +
|
||||
"command, please check appium logs");
|
||||
this.cbForCurrentCmd(error, null);
|
||||
if (!code) {
|
||||
code = 1; // this counts as an error even if instruments doesn't think so
|
||||
this.isShuttingDown = true;
|
||||
var postShutdown = function () {
|
||||
if (typeof this.cbForCurrentCmd === "function") {
|
||||
logger.debug("We were in the middle of processing a command when " +
|
||||
"instruments died; responding with a generic error");
|
||||
var error = new UnknownError("Instruments died while responding to " +
|
||||
"command, please check appium logs");
|
||||
this.onInstrumentsDie(error, this.cbForCurrentCmd);
|
||||
} else {
|
||||
this.onInstrumentsDie();
|
||||
}
|
||||
}
|
||||
}.bind(this);
|
||||
if (this.commandProxy) {
|
||||
this.commandProxy.safeShutdown(function () {
|
||||
this.shutdown(code, this.onInstrumentsDie);
|
||||
this.shutdown(code, postShutdown);
|
||||
}.bind(this));
|
||||
} else {
|
||||
this.shutdown(code, this.onInstrumentsDie);
|
||||
this.shutdown(code, postShutdown);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -1251,6 +1254,11 @@ IOS.prototype.postCleanup = function (cb) {
|
||||
this.stopRemote();
|
||||
}
|
||||
|
||||
var final = function () {
|
||||
this.isShuttingDown = false;
|
||||
cb();
|
||||
}.bind(this);
|
||||
|
||||
if (this.args.reset || this.args.fullReset) {
|
||||
// The simulator process must be ended before we delete applications.
|
||||
async.series([
|
||||
@@ -1269,10 +1277,10 @@ IOS.prototype.postCleanup = function (cb) {
|
||||
cb();
|
||||
}
|
||||
}.bind(this),
|
||||
], cb);
|
||||
], final);
|
||||
} else {
|
||||
logger.debug("Reset set to false, not ending sim or cleaning up app state");
|
||||
cb();
|
||||
final();
|
||||
}
|
||||
|
||||
};
|
||||
|
||||
@@ -62,6 +62,9 @@ exports.getStatus = function (req, res) {
|
||||
if (typeof gitSha !== "undefined") {
|
||||
data.build.revision = gitSha;
|
||||
}
|
||||
if (req.device && typeof req.device.getStatusExtensions === "function") {
|
||||
data = _.extend(data, req.device.getStatusExtensions());
|
||||
}
|
||||
respondSuccess(req, res, data);
|
||||
};
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
"use strict";
|
||||
|
||||
var setup = require("../common/setup-base")
|
||||
, _ = require("underscore")
|
||||
, getAppPath = require('../../helpers/app').getAppPath;
|
||||
|
||||
describe('crash recovery', function () {
|
||||
@@ -36,3 +37,27 @@ describe('crash recovery', function () {
|
||||
});
|
||||
});
|
||||
|
||||
describe('crash commands', function () {
|
||||
|
||||
var driver;
|
||||
var desired = {
|
||||
app: getAppPath('TestApp')
|
||||
};
|
||||
|
||||
setup(this, desired, {}, {FAST_TESTS: false}).then(function (d) { driver = d; });
|
||||
|
||||
it('should not process new commands until after crash shutdown', function (done) {
|
||||
driver
|
||||
.execute("$.crash()") // this causes instruments to shutdown during
|
||||
// this command
|
||||
.should.eventually.be.rejectedWith('13')
|
||||
.status()
|
||||
.then(function (s) {
|
||||
if (_.has(s, 'isShuttingDown')) {
|
||||
s.isShuttingDown.should.eql(false);
|
||||
}
|
||||
})
|
||||
.nodeify(done);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user