From b1914c7dcc5c3708e09249b19d25464df02e7ce3 Mon Sep 17 00:00:00 2001
From: Ryan Milbourne <ryanbmilbourne@users.noreply.github.com>
Date: Tue, 4 Oct 2016 21:14:51 -0700
Subject: [PATCH] Resolve scalability issues (#137)

* Fix issues with event emission and send throwing

This commit fixes two scalability issues I identified in lab.  First, we
will defer event emissions to ensure that the listners can register
their callbacks.  Second, we add some defensive coding around the
session child writing to handle write errors gracefully.

Tested in our lab, with 75+ concurrent telnet sessions.

fixes #135, fixes #136

* Roll PATCH version
---
 CHANGELOG.md    |  6 ++++++
 lib/spectcl.js  | 50 ++++++++++++++++++++++++++++++++++++++-----------
 package.json    |  2 +-
 test/spectcl.js |  4 ++--
 4 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1da0542..62c0ced 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,9 @@
+v0.6.3 - Oct 4 2016
+
+* Fix issue with exceptions being thrown by `session.send` (Ryan Milbourne)
+* Fix issue with events being emitted before listener callbacks are registered (Ryan Milbourne)
+* Build: Update dependencies (greenkeeper)
+
 v0.6.2 - May 10 2016
 
 * modify "sendEof" to invoke callback w/o waiting for child stream closure (Ryan Milbourne)
diff --git a/lib/spectcl.js b/lib/spectcl.js
index 9b89b3d..ebdfef0 100644
--- a/lib/spectcl.js
+++ b/lib/spectcl.js
@@ -135,7 +135,9 @@ Spectcl.prototype.spawn = function (command, cmdParams, cmdOptions, spawnOptions
             self.fullBuffer = true
             self.cache = self.cache.slice(self.cache.length-self.options.matchMax, self.cache.length)
         }
-        self.emit('data', data)
+        process.nextTick(function(){
+            self.emit('data', data)
+        })
     }
 
     // Arguments handling
@@ -193,17 +195,20 @@ Spectcl.prototype.spawn = function (command, cmdParams, cmdOptions, spawnOptions
         debug('[spawn] child tty:\t'+self.child.stdout.ttyname)
     }
 
-    debug('[spawn] child pid:\t'+self.child.pid)
-
     self.child.on('error', function(err){
         debug('[spawn] child %d error: '+err,self.child.pid)
-        self.emit('error', err)
+        process.nextTick(function(){
+            self.emit('error', err)
+        })
     })
 
     self.child.on('exit', function(code, signal){
         debug('[spawn] child %d exit. code: %s\tsignal: %s',self.child.pid,code,signal)
-        self.emit('exit', code, signal)
+        process.nextTick(function(){
+            self.emit('exit', code, signal)
+        })
     })
+    debug('[spawn] child pid:\t'+self.child.pid)
 
     return
 }
@@ -259,7 +264,7 @@ Spectcl.prototype.expect = function(expArr, cb){
          * @returns {String|RegExp|Number} The Expectation that was found in the cache or null if no match was found.
          */
       , matchCache = function(){
-          debug('[expect] self.cache = %s',self.cache)
+          debug('[expect] [%d] cache contents: %s',self.child.pid, util.inspect(self.cache))
           for(var i=0; i<expectations.length; i++){
               var exp = expectations[i]
                 , matched
@@ -390,6 +395,7 @@ Spectcl.prototype.expect = function(expArr, cb){
     }
     debug('[expect] [%d] expectations: '+Object.keys(expectObject), self.child.pid)
 
+
     /**
      * Handles end of readable stream.
      * Remove the data listener and clear the timeout interval.
@@ -476,13 +482,35 @@ Spectcl.prototype.expect = function(expArr, cb){
  * @return {undefined}
  */
 Spectcl.prototype.send = function(data, cb){
-    debug('[send] writing %s to child stdin',data)
-    this.child.stdin.write(data, function(){
-        debug('[send] wrote to child stdin')
+    var self = this
+    debug('[send] writing %s to child stdin',util.inspect(data))
+
+    var writeErrListener = function(err){
+        debug('[send] err %s',err)
+    }
+    self.child.stdin.on('error', writeErrListener)
+
+    try {
+        self.child.stdin.write(data, function(err){
+            if(err){
+                self.child.stdin.removeListener('error',writeErrListener)
+                debug('[send] error: %s',err)
+                if(cb){
+                    return cb(err)
+                }
+            }
+            self.child.stdin.removeListener('error',writeErrListener)
+            debug('[send] wrote to child stdin')
+            if(cb){
+                return cb()
+            }
+        })
+    } catch(err) {
+        debug('[send] error: %s',err)
         if(cb){
-            return cb()
+            return cb(err)
         }
-    })
+    }
 }
 
 
diff --git a/package.json b/package.json
index 7e04c9b..1a0f5ce 100644
--- a/package.json
+++ b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "spectcl",
   "description": "Spawns and interacts with child processes using spawn / expect commands",
-  "version": "0.6.2",
+  "version": "0.6.3",
   "author": "Ryan Milbourne <ryan.milbourne@viasat.com>",
   "maintainers": [
     "Greg Cochard <greg@gregcochard.com>",
diff --git a/test/spectcl.js b/test/spectcl.js
index 04891f7..402ca5c 100644
--- a/test/spectcl.js
+++ b/test/spectcl.js
@@ -616,10 +616,10 @@ describe('spectcl', function(){
                 '>', function(){
                     assert.notEqual(session.expect_out.match, null, 'null expect_out match')
                     assert.notEqual(session.expect_out.buffer, '', 'empty expect_out buffer')
-                    session.send('process.exit()\r', function(){
+                    session.child.on('exit', function(){
                         finished()
                     })
-                    session.child.on('exit', function(){
+                    session.send('process.exit()\r', function(){
                         finished()
                     })
                 }