Extract subprocess management to a reusable module.

Author: jleyba

javascript/node/selenium-webdriver/io/exec.js

@@ -0,0 +1,134 @@
+// Copyright 2014 Selenium committers
+// Copyright 2014 Software Freedom Conservancy
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+//     You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+'use strict';
+
+var childProcess = require('child_process');
+
+var promise = require('..').promise;
+
+
+/**
+ * A hash with configuration options for an executed command.
+ * <ul>
+ * <li>
+ * <li>{@code args} - Command line arguments.
+ * <li>{@code env} - Command environment; will inherit from the current process
+ *     if missing.
+ * <li>{@code stdio} - IO configuration for the spawned server process. For
+ *     more information, refer to the documentation of
+ *     {@code child_process.spawn}.
+ * </ul>
+ *
+ * @typedef {{
+ *   args: (!Array.<string>|undefined),
+ *   env: (!Object.<string, string>|undefined),
+ *   stdio: (string|!Array.<string|number|!Stream|null|undefined>|undefined)
+ * }}
+ */
+var Options;
+
+
+/**
+ * Describes a command's termination conditions.
+ * @param {?number} code The exit code, or {@code null} if the command did not
+ *     exit normally.
+ * @param {?string} signal The signal used to kill the command, or
+ *     {@code null}.
+ * @constructor
+ */
+var Result = function(code, signal) {
+  /** @type {?number} */
+  this.code = code;
+
+  /** @type {?string} */
+  this.signal = signal;
+};
+
+
+/**
+ * Represents a command running in a sub-process.
+ * @param {!promise.Promise.<!Result>} result The command result.
+ * @constructor
+ */
+var Command = function(result, onKill) {
+  /** @return {boolean} Whether this command is still running. */
+  this.isRunning = function() {
+    return result.isPending();
+  };
+
+  /**
+   * @return {!promise.Promise.<!Result>} A promise for the result of this
+   *     command.
+   */
+  this.result = function() {
+    return result;
+  };
+
+  /**
+   * Sends a signal to the underlying process.
+   * @param {string=} opt_signal The signal to send; defaults to
+   *     {@code SIGTERM}.
+   */
+  this.kill = function(opt_signal) {
+    onKill(opt_signal || 'SIGTERM');
+  };
+};
+
+
+// PUBLIC API
+
+
+/**
+ * Spawns a child process. The returned {@link Command} may be used to wait
+ * for the process result or to send signals to the process.
+ *
+ * @param {string} command The executable to spawn.
+ * @param {Options=} opt_options The command options.
+ * @return {!Command} The launched command.
+ */
+module.exports = function(command, opt_options) {
+  var options = opt_options || {};
+
+  var proc = childProcess.spawn(command, options.args || [], {
+    env: options.env || process.env,
+    stdio: options.stdio || 'ignore'
+  }).once('exit', onExit);
+
+  // This process should not wait on the spawned child, however, we do
+  // want to ensure the child is killed when this process exits.
+  proc.unref();
+  process.once('exit', killCommand);
+
+  var result = promise.defer();
+  var cmd = new Command(result.promise, function(signal) {
+    if (!result.isPending() || !proc) {
+      return;  // No longer running.
+    }
+    proc.kill(signal);
+  });
+  return cmd;
+
+  function onExit(code, signal) {
+    proc = null;
+    process.removeListener('exit', killCommand);
+    result.fulfill(new Result(code, signal));
+  }
+
+  function killCommand() {
+    process.removeListener('exit', killCommand);
+    proc && proc.kill('SIGTERM');
+  }
+};

javascript/node/selenium-webdriver/remote/index.js

@@ -14,14 +14,13 @@
 
 'use strict';
 
-var spawn = require('child_process').spawn,
-    os = require('os'),
-    path = require('path'),
+var path = require('path'),
     url = require('url'),
     util = require('util');
 
 var promise = require('../').promise,
     httpUtil = require('../http/util'),
+    exec = require('../io/exec'),
     net = require('../net'),
     portprober = require('../net/portprober');
 
@@ -95,6 +94,21 @@ function DriverService(executable, options) {
 
   /** @private {(string|!Array.<string|number|!Stream|null|undefined>)} */
   this.stdio_ = options.stdio || 'ignore';
+
+  /**
+   * A promise for the managed subprocess, or null if the server has not been
+   * started yet. This promise will never be rejected.
+   * @private {promise.Promise.<!exec.Command>}
+   */
+  this.command_ = null;
+
+  /**
+   * Promise that resolves to the server's address or null if the server has
+   * not been started. This promise will be rejected if the server terminates
+   * before it starts accepting WebDriver requests.
+   * @private {promise.Promise.<string>}
+   */
+  this.address_ = null;
 }
 
 
@@ -106,26 +120,6 @@ function DriverService(executable, options) {
 DriverService.DEFAULT_START_TIMEOUT_MS = 30 * 1000;
 
 
-/** @private {child_process.ChildProcess} */
-DriverService.prototype.process_ = null;
-
-
-/**
- * Promise that resolves to the server's address or null if the server has not
- * been started.
- * @private {webdriver.promise.Promise.<string>}
- */
-DriverService.prototype.address_ = null;
-
-
-/**
- * Promise that tracks the status of shutting down the server, or null if the
- * server is not currently shutting down.
- * @private {webdriver.promise.Promise}
- */
-DriverService.prototype.shutdownHook_ = null;
-
-
 /**
  * @return {!webdriver.promise.Promise.<string>} A promise that resolves to
  *    the server's address.
@@ -140,6 +134,8 @@ DriverService.prototype.address = function() {
 
 
 /**
+ * Returns whether the underlying process is still running. This does not take
+ * into account whether the process is in the process of shutting down.
  * @return {boolean} Whether the underlying service process is running.
  */
 DriverService.prototype.isRunning = function() {
@@ -151,7 +147,7 @@ DriverService.prototype.isRunning = function() {
  * Starts the server if it is not already running.
  * @param {number=} opt_timeoutMs How long to wait, in milliseconds, for the
  *     server to start accepting requests. Defaults to 30 seconds.
- * @return {!webdriver.promise.Promise.<string>} A promise that will resolve
+ * @return {!promise.Promise.<string>} A promise that will resolve
  *     to the server's base URL when it has started accepting requests. If the
  *     timeout expires before the server has started, the promise will be
  *     rejected.
@@ -164,21 +160,28 @@ DriverService.prototype.start = function(opt_timeoutMs) {
   var timeout = opt_timeoutMs || DriverService.DEFAULT_START_TIMEOUT_MS;
 
   var self = this;
+  this.command_ = promise.defer();
   this.address_ = promise.defer();
   this.address_.fulfill(promise.when(this.port_, function(port) {
     if (port <= 0) {
       throw Error('Port must be > 0: ' + port);
     }
     return promise.when(self.args_, function(args) {
-      self.process_ = spawn(self.executable_, args, {
+      var command = exec(self.executable_, {
+        args: args,
         env: self.env_,
         stdio: self.stdio_
-      }).once('exit', onServerExit);
+      });
+
+      self.command_.fulfill(command);
 
-      // This process should not wait on the spawned child, however, we do
-      // want to ensure the child is killed when this process exits.
-      self.process_.unref();
-      process.once('exit', killServer);
+      command.result().then(function(result) {
+        self.address_.reject(result.code == null ?
+            Error('Server was killed with ' + result.signal) :
+            Error('Server exited with ' + result.code));
+        self.address_ = null;
+        self.command_ = null;
+      });
 
       var serverUrl = url.format({
         protocol: 'http',
@@ -195,26 +198,6 @@ DriverService.prototype.start = function(opt_timeoutMs) {
   }));
 
   return this.address_;
-
-  function onServerExit(code, signal) {
-    self.address_.reject(code == null ?
-        Error('Server was killed with ' + signal) :
-        Error('Server exited with ' + code));
-
-    if (self.shutdownHook_) {
-      self.shutdownHook_.fulfill();
-    }
-
-    self.shutdownHook_ = null;
-    self.address_ = null;
-    self.process_ = null;
-    process.removeListener('exit', killServer);
-  }
-
-  function killServer() {
-    process.removeListener('exit', killServer);
-    self.process_ && self.process_.kill('SIGTERM');
-  }
 };
 
 
@@ -226,26 +209,12 @@ DriverService.prototype.start = function(opt_timeoutMs) {
  *     the server has been stopped.
  */
 DriverService.prototype.kill = function() {
-  if (!this.address_) {
+  if (!this.address_ || !this.command_) {
     return promise.fulfilled();  // Not currently running.
   }
-
-  if (!this.shutdownHook_) {
-    // No process: still starting; wait on address.
-    // Otherwise, kill the process now. Exit handler will resolve the
-    // shutdown hook.
-    if (this.process_) {
-      this.shutdownHook_ = promise.defer();
-      this.process_.kill('SIGTERM');
-    } else {
-      var self = this;
-      this.shutdownHook_ = this.address_.thenFinally(function() {
-        self.process_ && self.process_.kill('SIGTERM');
-      });
-    }
-  }
-
-  return this.shutdownHook_;
+  return this.command_.then(function(command) {
+    command.kill('SIGTERM');
+  });
 };