Merge pull request #32 from suredone/github-25

fix #25

package.json

@@ -1,6 +1,6 @@
 {
   "name": "qdone",
-  "version": "1.3.0",
+  "version": "1.4.0-alpha.1",
   "description": "Language agnostic job queue for SQS",
   "main": "index.js",
   "dependencies": {
@@ -11,6 +11,7 @@
     "command-line-usage": "^5.0.3",
     "debug": "^3.1.0",
     "q": "^1.5.1",
+    "tree-kill": "^1.2.0",
     "uuid": "^3.2.1"
   },
   "devDependencies": {

src/worker.js

@@ -3,6 +3,7 @@ const Q = require('q')
 const childProcess = require('child_process')
 const debug = require('debug')('qdone:worker')
 const chalk = require('chalk')
+const treeKill = require('tree-kill')
 const qrlCache = require('./qrlCache')
 const cheapIdleCheck = require('./idleQueues').cheapIdleCheck
 const AWS = require('aws-sdk')
@@ -68,11 +69,34 @@ function executeJob (job, qname, qrl, options) {
   timeoutExtender = setTimeout(extendTimeout, visibilityTimeout * 1000 * 0.5)
   debug('timeout', visibilityTimeout * 1000 * 0.5)
 
-  return Q
-    .nfcall(childProcess.exec, cmd, {timeout: options['kill-after'] * 1000})
+  // NOTE: Due to #25 we cannot rely on child_process.exec's timeout option because
+  // it does not seem to work for child processes of the shell, so we'll create our
+  // own timeout and use tree-kill to catch all of the child processes.
+
+  let child
+  function killTree () {
+    debug('killTree', child.pid)
+    treeKill(child.pid, 'SIGTERM')
+    setTimeout(function () {
+      treeKill(child.pid, 'SIGKILL')
+    }, 1000)
+  }
+  const treeKiller = setTimeout(killTree, options['kill-after'] * 1000)
+  debug({treeKiller: options['kill-after'] * 1000, date: Date.now()})
+
+  const promise = new Promise(function (resolve, reject) {
+    child = childProcess.exec(cmd, function (err, stdout, stderr) {
+      if (err) reject(err, stdout, stderr)
+      else resolve(stdout, stderr)
+    })
+  })
+
+  return promise
+    // Q.nfcall(childProcess.exec, cmd, {timeout: options['kill-after'] * 1000})
     .then(function (stdout, stderr) {
-      debug('childProcess.exec.then')
+      debug('childProcess.exec.then', Date.now())
       clearTimeout(timeoutExtender)
+      clearTimeout(treeKiller)
       if (options.verbose) {
         console.error(chalk.green('  SUCCESS'))
         if (stdout) console.error(chalk.blue('  stdout: ') + stdout)
@@ -97,6 +121,7 @@ function executeJob (job, qname, qrl, options) {
     .catch((err, stdout, stderr) => {
       debug('childProcess.exec.catch')
       clearTimeout(timeoutExtender)
+      clearTimeout(treeKiller)
       if (options.verbose) {
         console.error(chalk.red('  FAILED'))
         if (err.code) console.error(chalk.blue('  code  : ') + err.code)

test/fixtures/test-child-kill-linux.sh

@@ -0,0 +1,9 @@
+#!/bin/bash
+OUTFILE=/tmp/qdone-test-child-kill-linux.out
+rm $OUTFILE
+_term() { 
+  echo "terminated" > $OUTFILE
+  exit 1
+}
+trap _term SIGTERM
+for i in 1 2 3; do sleep 1; echo $i; echo $i >> $OUTFILE; done

test/test.cli.js

@@ -5,6 +5,7 @@ const chai = require('chai')
 const packageJson = require('../package.json')
 const sinon = require('sinon')
 const stripAnsi = require('strip-ansi')
+const fs = require('fs')
 const AWS = require('aws-sdk-mock')
 // const mockStdin = require('mock-stdin')
 
@@ -799,6 +800,40 @@ describe('cli', function () {
       }))
   })
 
+  describe('qdone worker test --drain --kill-after 1 --wait-time 1 --quiet # (job runs past kill timer)', function () {
+    before(function () {
+      AWS.mock('SQS', 'getQueueUrl', function (params, callback) {
+        callback(null, {QueueUrl: `https://q.amazonaws.com/123456789101/${params.QueueName}`})
+      })
+      AWS.mock('SQS', 'listQueues', function (params, callback) {
+        callback(null, {QueueUrls: [`https://q.amazonaws.com/123456789101/${params.QueueName}`]})
+      })
+      AWS.mock('SQS', 'receiveMessage', function (params, callback) {
+        callback(null, { Messages: [
+          { MessageId: 'da68f62c-0c07-4bee-bf5f-7e856EXAMPLE', Body: 'bash test/fixtures/test-child-kill-linux.sh', ReceiptHandle: 'AQEBzbVv...fqNzFw==' }
+        ] })
+        AWS.restore('SQS', 'receiveMessage')
+        // Subsequent calls return no message
+        AWS.mock('SQS', 'receiveMessage', function (params, callback) {
+          callback(null, {})
+        })
+        process.nextTick(function () {
+          clock.tick(1500)
+        })
+      })
+      AWS.mock('SQS', 'deleteMessage', function (params, callback) {
+        callback(null, {})
+      })
+    })
+    console.log('what')
+    it('should execute the job successfully and exit 0',
+      cliTest(['worker', 'test', '--drain', '--kill-after', '1', '--wait-time', '1'], function (result, stdout, stderr) {
+        expect(stderr).to.contain('FAILED')
+        // Check that file does not continue to be written to
+        expect(fs.readFileSync('/tmp/qdone-test-child-kill-linux.out').toString()).to.equal('terminated\n')
+      }))
+  })
+
   describe('qdone worker "test*" --drain # (9 queues, 1 successful job per queue)', function () {
     before(function () {
       AWS.mock('SQS', 'getQueueUrl', function (params, callback) {