From 3c6a25b5ae0bf8c51146bb5b9919f709cebb7646 Mon Sep 17 00:00:00 2001
From: Jonathan Lipps <jlipps@gmail.com>
Date: Fri, 29 Aug 2014 14:05:39 -0700
Subject: [PATCH] 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.

---
 lib/devices/ios/ios-controller.js  |  6 ++++++
 lib/devices/ios/ios.js             | 34 ++++++++++++++++++------------
 lib/server/controller.js           |  3 +++
 test/functional/ios/crash-specs.js | 25 ++++++++++++++++++++++
 4 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/lib/devices/ios/ios-controller.js b/lib/devices/ios/ios-controller.js
index a5e09e39c..696aa9b11 100644
--- a/lib/devices/ios/ios-controller.js
+++ b/lib/devices/ios/ios-controller.js
@@ -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' : '';
diff --git a/lib/devices/ios/ios.js b/lib/devices/ios/ios.js
index 2b9b8b56f..b2192c304 100644
--- a/lib/devices/ios/ios.js
+++ b/lib/devices/ios/ios.js
@@ -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();
   }
 
 };
diff --git a/lib/server/controller.js b/lib/server/controller.js
index 63adc8253..c74d2f746 100644
--- a/lib/server/controller.js
+++ b/lib/server/controller.js
@@ -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);
 };
 
diff --git a/test/functional/ios/crash-specs.js b/test/functional/ios/crash-specs.js
index 7401c26bd..004672895 100644
--- a/test/functional/ios/crash-specs.js
+++ b/test/functional/ios/crash-specs.js
@@ -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);
+  });
+});
+