New rewrite of provider. By moving to promises we should no longer have issues with double-quits.

lib/cli.js

@@ -28,10 +28,6 @@ program
     , 'name (or os.hostname())'
     , String
     , os.hostname())
-  .option('-t, --restart-threshold <ms>'
-    , 'restart worker only if it stays alive for longer than this'
-    , Number
-    , 10000)
   .option('--min-port <port>'
     , 'minimum port number for worker use'
     , Number
@@ -57,8 +53,6 @@ program
 
     require('./roles/provider')({
       name: options.name
-    , restartThreshold: options.restartThreshold
-    , restartTimeout: 1000
     , killTimeout: 10000
     , ports: cliutil.range(options.minPort, options.maxPort)
     , filter: function(device) {

lib/roles/provider.js

@@ -9,12 +9,12 @@ var _ = require('lodash')
 var logger = require('../util/logger')
 var wire = require('../wire')
 var wireutil = require('../wire/util')
+var procutil = require('../util/procutil')
 
 module.exports = function(options) {
   var log = logger.createLogger('provider')
   var client = Promise.promisifyAll(adb.createClient())
-  var workers = Object.create(null)
+  var workers = {}
-  var tracker = new events.EventEmitter()
   var lists = {
     all: []
   , ready: []
@@ -58,8 +58,31 @@ module.exports = function(options) {
     push.connect(endpoint)
   })
 
-  tracker.on('add', function(device) {
+  // Track and manage devices
-    lists.all.push(device.id)
+  client.trackDevicesAsync().then(function(tracker) {
+    log.info('Tracking devices')
+
+    // Helper for ignoring unwanted devices
+    function filterDevice(listener) {
+      if (options.filter) {
+        return function(device) {
+          if (options.filter(device)) {
+            listener(device)
+          }
+        }
+      }
+      return listener
+    }
+
+    // To make things easier, we're going to cheat a little, and make all
+    // device events go to their own EventEmitters. This way we can keep all
+    // device data in the same scope.
+    var flippedTracker = new events.EventEmitter()
+
+    tracker.on('add', filterDevice(function(device) {
+      log.info('Found device "%s" (%s)', device.id, device.type)
+
+      // Tell others we found a device
       push.send([
         wireutil.global
       , wireutil.envelope(new wire.DevicePresentMessage(
@@ -68,10 +91,28 @@ module.exports = function(options) {
         , wireutil.toDeviceStatus(device.type)
         ))
       ])
-    maybeConnect(device)
-  })
 
-  tracker.on('change', function(device) {
+      // Statistics
+      lists.all.push(device.id)
+      delayedTotals()
+
+      var privateTracker = new events.EventEmitter()
+        , resolver = Promise.defer()
+        , timer
+        , worker
+
+      // When any event occurs on the added device
+      function deviceListener(type, device) {
+        // Okay, this is a bit unnecessary but it allows us to get rid of an
+        // ugly switch statement and return to the original style.
+        privateTracker.emit(type, device)
+      }
+
+      // When the added device changes
+      function changeListener(device) {
+        log.info('Device "%s" is now "%s"', device.id, device.type)
+
+        // Tell others the device changed
         push.send([
           wireutil.global
         , wireutil.envelope(new wire.DeviceStatusMessage(
@@ -79,91 +120,95 @@ module.exports = function(options) {
           , wireutil.toDeviceStatus(device.type)
           ))
         ])
-    maybeConnect(device) || maybeDisconnect(device)
-  })
 
-  tracker.on('remove', function(device) {
+        check(device)
+      }
+
+      // When the added device gets removed
+      function removeListener(device) {
+        log.info('Lost device "%s" (%s)', device.id, device.type)
+
+        clearTimeout(timer)
+        flippedTracker.removeListener(device.id, deviceListener)
         _.pull(lists.all, device.id)
+        delayedTotals()
+
+        // Tell others the device is gone
         push.send([
           wireutil.global
         , wireutil.envelope(new wire.DeviceAbsentMessage(
             device.id
           ))
         ])
-    maybeDisconnect(device)
-  })
 
-  client.trackDevicesAsync()
+        stop()
-    .then(function(unfilteredTracker) {
-      log.info('Tracking devices')
-
-      unfilteredTracker.on('add', function(device) {
-        if (isWantedDevice(device)) {
-          log.info('Found device "%s" (%s)', device.id, device.type)
-          tracker.emit('add', device)
-        }
-        else {
-          log.info('Ignoring device "%s" (%s)', device.id, device.type)
-        }
-      })
-
-      unfilteredTracker.on('change', function(device) {
-        if (isWantedDevice(device)) {
-          log.info('Device "%s" is now "%s"', device.id, device.type)
-          tracker.emit('change', device)
-        }
-      })
-
-      unfilteredTracker.on('remove', function(device) {
-        if (isWantedDevice(device)) {
-          log.info('Lost device "%s" (%s)', device.id, device.type)
-          tracker.emit('remove', device)
-        }
-      })
-    })
-
-  function pushDeviceStatus(device, type) {
-    push.send([wireutil.global,
-      wireutil.makeDeviceStatusMessage(device.id, type, options.name)])
       }
 
-  function isWantedDevice(device) {
+      // Check if we can do anything with the device
-    return options.filter ? options.filter(device) : true
+      function check(device) {
-  }
+        clearTimeout(timer)
-
-  function isConnectable(device) {
         switch (device.type) {
           case 'device':
           case 'emulator':
-        return true
+            timer = setTimeout(work, 100)
+            break
           default:
-        return false
+            stop()
+            break
         }
       }
 
-  function isConnected(device) {
+      // Starts a device worker and keeps it alive
-    return workers[device.id]
+      function work() {
+        return worker = workers[device.id] = spawn(device)
+          .then(function() {
+            log.info('Device worker "%s" has retired', device.id)
+            worker = workers[device.id] = null
+          })
+          .catch(procutil.ExitError, function(err) {
+            log.info('Restarting device worker "%s"', device.id)
+            return Promise.delay(500)
+              .then(function() {
+                return work()
+              })
+          })
       }
 
-  function maybeConnect(device) {
+      // No more work required
-    if (isConnectable(device) && !isConnected(device)) {
+      function stop() {
-      log.info('Spawning device worker "%s"', device.id)
+        if (worker) {
+          log.info('Shutting down device worker "%s"', device.id)
+          worker.cancel()
+        }
+      }
+
+      // Spawn a device worker
+      function spawn(device) {
         var ports = options.ports.splice(0, 2)
           , proc = options.fork(device, ports)
+          , resolver = Promise.defer()
 
-      function messageListener(message) {
+        function exitListener(code, signal) {
-        switch (message) {
+          if (signal) {
-          case 'ready':
-            _.pull(lists.waiting, device.id)
-            lists.ready.push(device.id)
-            break
-          default:
             log.warn(
-              'Unknown message from worker "%s": "%s"'
+              'Device worker "%s" was killed with signal %s, assuming ' +
+              'deliberate action and not restarting'
             , device.id
-            , message
+            , signal
             )
-            break
+            resolver.resolve()
+          }
+          else if (code === 0) {
+            log.info('Device worker "%s" stopped cleanly', device.id)
+            resolver.resolve()
+          }
+          else {
+            log.error(
+              'Device worker "%s" died with code %s'
+            , device.id
+            , code
+            )
+            resolver.reject(new procutil.ExitError(code))
           }
         }
 
@@ -175,183 +220,81 @@ module.exports = function(options) {
           )
         }
 
-      function exitListener(code, signal) {
+        function messageListener(message) {
-        var worker = cleanupWorker(device.id)
+          switch (message) {
-        switch (code) {
+            case 'ready':
-          case 0:
+              _.pull(lists.waiting, device.id)
-            log.info('Device worker "%s" stopped cleanly', device.id)
+              lists.ready.push(device.id)
-            break
-          case 143: // SIGTERM
-            log.warn('Device worker "%s" was killed before becoming operational'
-              , device.id)
               break
             default:
-            if (Date.now() - worker.started < options.restartThreshold) {
-              log.error(
-                'Device worker "%s" died with exit code %d, ' +
-                'NOT restarting due to threshold of %dms not being met'
-              , device.id
-              , code
-              , options.restartThreshold
-              )
-            }
-            else {
-              log.error(
-                'Device worker "%s" died with exit code %d, ' +
-                'attempting to restart in %dms if device is still around'
-              , device.id
-              , code
-              , options.restartTimeout
-              )
-              waitForAnyChanges(device)
-                .timeout(options.restartTimeout)
-                .then(function(device) {
-                  // Most likely we lost the device, but our tracker didn't
-                  // see it before the process died
               log.warn(
-                    'Not restarting device worker "%s" due to tracker ' +
+                'Unknown message from device worker "%s": "%s"'
-                    'activity (but the change may cause it to start)'
               , device.id
+              , message
               )
-                })
-                .catch(function() {
-                  log.info('Restarting device worker "%s"', device.id)
-                  maybeConnect(device)
-                })
-            }
               break
           }
         }
 
-      proc.on('error', errorListener)
         proc.on('exit', exitListener)
+        proc.on('error', errorListener)
         proc.on('message', messageListener)
 
-      workers[device.id] = {
+        return resolver.promise
-        device: device
+          .finally(function() {
-      , proc: proc
+            log.info('Cleaning up device worker "%s"', device.id)
-      , started: Date.now()
+
-      , ports: ports
-      , unbind: function() {
-          proc.removeListener('error', errorListener)
             proc.removeListener('exit', exitListener)
+            proc.removeListener('error', errorListener)
             proc.removeListener('message', messageListener)
-        }
-      }
 
-      lists.waiting.push(device.id)
+            // Return used ports to the main pool
+            Array.prototype.push.apply(options.ports, ports)
 
-      delayedTotals()
+            // Update lists
-
+            _.pull(lists.ready, device.id)
-      return true
+            _.pull(lists.waiting, device.id)
-    }
+          })
-    return false
+          .cancellable()
-  }
+          .catch(Promise.CancellationError, function(err) {
-
+            log.info('Gracefully killing device worker "%s"', device.id)
-  function maybeDisconnect(device) {
+            return procutil.gracefullyKill(proc, options.killTimeout)
-    if (isConnected(device)) {
+          })
-      log.info('Releasing device worker "%s"', device.id)
+          .catch(Promise.TimeoutError, function(err) {
-      gracefullyKillWorker(device.id)
+            log.error(
-      return true
+              'Device worker "%s" did not stop in time: %s'
-    }
+            , device.id
-    return false
+            , err.message
-  }
+            )
-
-  function waitForAnyChanges(device) {
-    var resolver = Promise.defer()
-
-    function maybeResolve(otherDevice) {
-      if (otherDevice.id === device.id) {
-        resolver.resolve(otherDevice)
-      }
-    }
-
-    tracker.on('add', maybeResolve)
-    tracker.on('change', maybeResolve)
-    tracker.on('remove', maybeResolve)
-
-    return resolver.promise.finally(function() {
-      tracker.removeListener('add', maybeResolve)
-      tracker.removeListener('change', maybeResolve)
-      tracker.removeListener('remove', maybeResolve)
           })
       }
 
-  function tryKillWorker(id) {
+      flippedTracker.on(device.id, deviceListener)
-    var deferred = Promise.defer()
+      privateTracker.on('change', changeListener)
-      , worker = workers[id]
+      privateTracker.on('remove', removeListener)
+      check(device)
+    }))
 
-    function onExit() {
+    tracker.on('change', filterDevice(function(device) {
-      cleanupWorker(id)
+      flippedTracker.emit(device.id, 'change', device)
-      log.info('Gracefully killed device worker "%s"', id)
+    }))
-      deferred.resolve()
-    }
 
-    worker.unbind()
+    tracker.on('remove', filterDevice(function(device) {
-    worker.proc.once('exit', onExit)
+      flippedTracker.emit(device.id, 'remove', device)
-    worker.proc.kill('SIGTERM')
+    }))
-
-    return deferred.promise.finally(function() {
-      worker.proc.removeListener('exit', onExit)
   })
-  }
-
-  function forceKillWorker(id) {
-    log.warn('Force killing device worker "%s"', id)
-
-    var deferred = Promise.defer()
-      , worker = workers[id]
-
-    function onExit() {
-      cleanupWorker(id)
-      log.warn('Force killed device worker "%s"', id)
-      deferred.resolve()
-    }
-
-    worker.unbind()
-    worker.proc.once('exit', onExit)
-    worker.proc.kill('SIGKILL')
-
-    return deferred.promise.finally(function() {
-      worker.proc.removeListener('exit', onExit)
-    })
-  }
-
-  function gracefullyKillWorker(id) {
-    return tryKillWorker(id)
-      .timeout(options.killTimeout)
-      .catch(function() {
-        log.error('Device worker "%s" did not stop in time', id)
-        return forceKillWorker(id)
-          .timeout(options.killTimeout)
-      })
-  }
 
   function gracefullyExit() {
     log.info('Stopping all workers')
-    Promise.all(Object.keys(workers).map(gracefullyKillWorker))
+    Promise.all(Object.keys(workers).map(function(serial) {
+        return workers[serial].cancel()
+      }))
       .done(function() {
         log.info('All cleaned up')
         process.exit(0)
       })
   }
 
-  function cleanupWorker(id) {
-    var worker = workers[id]
-    delete workers[id]
-    Array.prototype.push.apply(options.ports, worker.ports)
-    _.pull(lists.ready, id)
-    _.pull(lists.waiting, id)
-    push.send([
-      wireutil.global
-    , wireutil.envelope(new wire.DeviceAbsentMessage(
-        id
-      ))
-    ])
-    delayedTotals()
-    return worker
-  }
-
   process.on('SIGINT', function(e) {
     log.info('Received SIGINT')
     gracefullyExit()

lib/util/procutil.js

@@ -34,3 +34,28 @@ module.exports.fork = function() {
     })
   })
 }
+
+// Export
+module.exports.gracefullyKill = function(proc, timeout) {
+  function killer(signal) {
+    var deferred = Promise.defer()
+
+    function onExit() {
+      deferred.resolve()
+    }
+
+    proc.once('exit', onExit)
+    proc.kill(signal)
+
+    return deferred.promise.finally(function() {
+      proc.removeListener('exit', onExit)
+    })
+  }
+
+  return killer('SIGTERM')
+    .timeout(timeout)
+    .catch(function() {
+      return killer('SIGKILL')
+        .timeout(timeout)
+    })
+}