child process now use net.Socket

doc/api.txt

@@ -192,23 +192,6 @@ The default is to only recurse twice.  To make it recurse indefinitely, pass
 in +null+ for +depth+.
 
 
-+exec(command, callback)+::
-Executes the command as a child process, buffers the output and returns it
-in a callback.
-+
-----------------------------------------
-var sys = require("sys");
-sys.exec("ls /", function (err, stdout, stderr) {
-  if (err) throw err;
-  sys.puts(stdout);
-});
-----------------------------------------
-+
-The callback gets the arguments +(err, stdout, stderr)+. On success +err+
-will be +null+. On error +err+ will be an instance of +Error+ and +err.code+
-will be the exit code of the child process.
-
-
 == Events
 
 Many objects in Node emit events: a TCP server emits an event each time
@@ -399,25 +382,18 @@ Stops a interval from triggering.
 == Child Processes
 
 Node provides a tridirectional +popen(3)+ facility through the class
-+process.ChildProcess+. It is possible to stream data through the child's +stdin+,
-+stdout+, and +stderr+ in a fully non-blocking way.
++ChildProcess+ class. It is possible to stream data through the child's
++stdin+, +stdout+, and +stderr+ in a fully non-blocking way.
+
+To create a child process use +require("child_process").spawn()+.
+
+Child processes always have three streams associated with them.
++child.stdin+, +child.stdout+, and +child.stderr+. 
 
-=== +process.ChildProcess+
 
 [cols="1,2,10",options="header"]
 |=========================================================
 | Event      | Parameters |Notes
-
-| +"output"+ | +data+     | Each time the child process 
-                            sends data to its +stdout+, this event is
-                            emitted. +data+ is a string. If the child
-                            process closes its +stdout+ stream (a common
-                            thing to do on exit), this event will be emitted
-                            with +data === null+.
-
-| +"error"+  | +data+     | Identical to the +"output"+ event except for
-                            +stderr+ instead of +stdout+.
-
 | +"exit"+   | +code+     | This event is emitted after the child process
                             ends. +code+ is the final exit code of the
                             process. One can be assured that after this
@@ -425,19 +401,18 @@ Node provides a tridirectional +popen(3)+ facility through the class
                             +"error"+ callbacks will no longer be made.
 |=========================================================
 
-+process.createChildProcess(command, args=[], env=process.env)+::
++require("child_process").spawn(command, args=[], env=process.env)+::
 Launches a new process with the given +command+, command line arguments, and
 environmental variables. For example:
 +
 ----------------------------------------
-var ls = process.createChildProcess("ls", ["-lh", "/usr"]);
-ls.addListener("output", function (data) {
-  sys.puts(data);
+// Pipe a child process output to
+// parent process output
+var ls = spawn("ls", ["-lh", "/usr"]);
+ls.stdout.addListener("data", function (data) {
+  process.stdout.write(data);
 });
 ----------------------------------------
-+
-Note, if you just want to buffer the output of a command and return it, then
-+exec()+ in +/sys.js+ might be better.
 
 
 +child.pid+ ::
@@ -459,6 +434,23 @@ Send a signal to the child process.  If no argument is given, the process
 will be sent +"SIGTERM"+. See signal(7) for a list of available signals.
 
 
++require("child_process").exec(command, callback)+::
+High-level way to executes a command as a child process and buffer the
+output and return it in a callback.
++
+----------------------------------------
+var exec = require("child_process").exec;
+exec("ls /", function (err, stdout, stderr) {
+  if (err) throw err;
+  sys.puts(stdout);
+});
+----------------------------------------
++
+The callback gets the arguments +(err, stdout, stderr)+. On success +err+
+will be +null+. On error +err+ will be an instance of +Error+ and +err.code+
+will be the exit code of the child process.
+
+
 
 == File System
 

lib/child_process.js

@@ -0,0 +1,100 @@
+var inherits = require('sys').inherits;
+var EventEmitter = require('events').EventEmitter;
+var Socket = require('net').Socket;
+var InternalChildProcess = process.binding('child_process').ChildProcess;
+
+
+var spawn = exports.spawn = function (path, args, env) {
+  var child = new ChildProcess();
+  child.spawn(path, args, env);
+  return child;
+};
+
+
+exports.exec = function (command, callback) {
+  var child = spawn("/bin/sh", ["-c", command]);
+  var stdout = "";
+  var stderr = "";
+
+  child.stdout.setEncoding('utf8');
+  child.stdout.addListener("data", function (chunk) { stdout += chunk; });
+
+  child.stderr.setEncoding('utf8');
+  child.stderr.addListener("data", function (chunk) { stderr += chunk; });
+
+  child.addListener("exit", function (code) {
+    if (code == 0) {
+      if (callback) callback(null, stdout, stderr);
+    } else {
+      var e = new Error("Command failed: " + stderr);
+      e.code = code;
+      if (callback) callback(e, stdout, stderr);
+    }
+  });
+};
+
+
+function ChildProcess () {
+  process.EventEmitter.call(this);
+
+  var self = this;
+
+  var gotCHLD = false;
+  var exitCode;
+  var internal = this._internal = new InternalChildProcess();
+
+  var stdin  = this.stdin  = new Socket();  
+  var stdout = this.stdout = new Socket();  
+  var stderr = this.stderr = new Socket();  
+
+  stderr.onend = stdout.onend = function () {
+    if (gotCHLD && !stdout.readable && !stderr.readable) {
+      self.emit('exit', exitCode);
+    }
+  };
+
+  internal.onexit = function (code) {
+    gotCHLD = true;
+    exitCode = code;
+    if (!stdout.readable && !stderr.readable) {
+      self.emit('exit', exitCode);
+    }
+  };
+
+  this.__defineGetter__('pid', function () { return internal.pid; });
+}
+inherits(ChildProcess, EventEmitter);
+
+
+ChildProcess.prototype.kill = function (sig) {
+  return this._internal.kill(sig);
+};
+
+
+ChildProcess.prototype.spawn = function (path, args, env) {
+  args = args || [];
+  env = env || process.env;
+  var envPairs = [];
+  for (var key in env) {
+    if (env.hasOwnProperty(key)) {
+      envPairs.push(key + "=" + env[key]);
+    }
+  }
+
+  var fds = this._internal.spawn(path, args, envPairs);
+
+  this.stdin.open(fds[0]);
+  this.stdin.writable = true;
+  this.stdin.readable = false;
+
+  this.stdout.open(fds[1]);
+  this.stdout.writable = false;
+  this.stdout.readable = true;
+  this.stdout.resume();
+
+  this.stderr.open(fds[2]);
+  this.stderr.writable = false;
+  this.stderr.readable = true;
+  this.stderr.resume();
+};
+

lib/net.js

@@ -367,20 +367,25 @@ function Socket (fd) {
   this.fd = null;
 
   if (parseInt(fd) >= 0) {
-    initSocket(this);
-
-    this.fd = fd;
-
-    this.readable = true;
-
-    this._writeWatcher.set(this.fd, false, true);
-    this.writable = true;
+    this.open(fd);
   }
 };
 sys.inherits(Socket, events.EventEmitter);
 exports.Socket = Socket;
 
 
+Socket.prototype.open = function (fd) {
+  initSocket(this);
+
+  this.fd = fd;
+
+  this.readable = true;
+
+  this._writeWatcher.set(this.fd, false, true);
+  this.writable = true;
+}
+
+
 exports.createConnection = function (port, host) {
   var s = new Socket();
   s.connect(port, host);
@@ -716,6 +721,7 @@ Socket.prototype.forceClose = function (exception) {
 
   timeout.unenroll(this);
 
+  // FIXME Bug when this.fd == 0
   if (this.fd) {
     close(this.fd);
     debug('close ' + this.fd);

lib/sys.js

@@ -16,7 +16,7 @@ exports.debug = function (x) {
   process.binding('stdio').writeError("DEBUG: " + x + "\n");
 };
 
-exports.error = function (x) {
+var error = exports.error = function (x) {
   for (var i = 0, len = arguments.length; i < len; ++i) {
     process.binding('stdio').writeError(arguments[i] + '\n');
   }
@@ -184,7 +184,7 @@ exports.inspect = function (obj, showHidden, depth) {
 
 exports.p = function () {
   for (var i = 0, len = arguments.length; i < len; ++i) {
-    exports.error(exports.inspect(arguments[i]));
+    error(exports.inspect(arguments[i]));
   }
 };
 
@@ -207,29 +207,14 @@ exports.log = function (msg) {
   exports.puts(timestamp() + ' - ' + msg.toString());
 }
 
-exports.exec = function (command, callback) {
-  var child = process.createChildProcess("/bin/sh", ["-c", command]);
-  var stdout = "";
-  var stderr = "";
-
-  child.addListener("output", function (chunk) {
-    if (chunk) stdout += chunk;
-  });
-
-  child.addListener("error", function (chunk) {
-    if (chunk) stderr += chunk;
-  });
-
-  child.addListener("exit", function (code) {
-    if (code == 0) {
-      if (callback) callback(null, stdout, stderr);
-    } else {
-      var e = new Error("Command failed: " + stderr);
-      e.code = code;
-      if (callback) callback(e, stdout, stderr);
-    }
-  });
-};
+var execWarning;
+exports.exec = function () {
+  if (!execWarning) {
+    execWarning = 'sys.exec has moved to the "child_process" module. Please update your source code.'
+    error(execWarning);
+  }
+  return require('child_process').exec.apply(this, arguments);
+}
 
 /**
  * Inherit the prototype methods from one constructor into another.

src/node.cc

@@ -1073,6 +1073,8 @@ static Handle<Value> Binding(const Arguments& args) {
 
   Local<Object> exports;
 
+  // TODO DRY THIS UP!
+
   if (!strcmp(*module_v, "stdio")) {
     if (binding_cache->Has(module)) {
       exports = binding_cache->Get(module)->ToObject();
@@ -1157,6 +1159,15 @@ static Handle<Value> Binding(const Arguments& args) {
       binding_cache->Set(module, exports);
     }
 
+  } else if (!strcmp(*module_v, "child_process")) {
+    if (binding_cache->Has(module)) {
+      exports = binding_cache->Get(module)->ToObject();
+    } else {
+      exports = Object::New();
+      ChildProcess::Initialize(exports);
+      binding_cache->Set(module, exports);
+    }
+
   } else if (!strcmp(*module_v, "natives")) {
     if (binding_cache->Has(module)) {
       exports = binding_cache->Get(module)->ToObject();
@@ -1165,6 +1176,7 @@ static Handle<Value> Binding(const Arguments& args) {
       // Explicitly define native sources.
       // TODO DRY/automate this?
       exports->Set(String::New("assert"),       String::New(native_assert));
+      exports->Set(String::New("child_process"),String::New(native_child_process));
       exports->Set(String::New("dns"),          String::New(native_dns));
       exports->Set(String::New("events"),       String::New(native_events));
       exports->Set(String::New("file"),         String::New(native_file));
@@ -1285,7 +1297,6 @@ static void Load(int argc, char *argv[]) {
   IOWatcher::Initialize(process);              // io_watcher.cc
   IdleWatcher::Initialize(process);            // idle_watcher.cc
   Timer::Initialize(process);                  // timer.cc
-  ChildProcess::Initialize(process);           // child_process.cc
   DefineConstants(process);                    // constants.cc
 
   // Compile, execute the src/node.js file. (Which was included as static C

src/node.js

@@ -25,6 +25,7 @@ process.unwatchFile = removed("process.unwatchFile() has moved to fs.unwatchFile
 GLOBAL.node = {};
 
 node.createProcess = removed("node.createProcess() has been changed to process.createChildProcess() update your code");
+process.createChildProcess = removed("childProcess API has changed. See doc/api.txt.");
 node.exec = removed("process.exec() has moved. Use require('sys') to bring it back.");
 node.inherits = removed("node.inherits() has moved. Use require('sys') to access it.");
 process.inherits = removed("process.inherits() has moved to sys.inherits.");
@@ -89,24 +90,6 @@ function requireNative (id) {
 }
 
 
-process.createChildProcess = function (file, args, env) {
-  var child = new process.ChildProcess();
-  args = args || [];
-  env = env || process.env;
-  var envPairs = [];
-  for (var key in env) {
-    if (env.hasOwnProperty(key)) {
-      envPairs.push(key + "=" + env[key]);
-    }
-  }
-  // TODO Note envPairs is not currently used in child_process.cc. The PATH
-  // needs to be searched for the 'file' command if 'file' does not contain
-  // a '/' character.
-  child.spawn(file, args, envPairs);
-  return child;
-};
-
-
 process.assert = function (x, msg) {
   if (!(x)) throw new Error(msg || "assertion error");
 };
@@ -797,7 +780,6 @@ process.openStdin = function () {
   var net = requireNative('net');
   var fd = process.binding('stdio').openStdin();
   stdin = new net.Socket(fd);
-  process.stdout.write(stdin.fd + "\n");
   stdin.resume();
   stdin.readable = true;
   return stdin;

src/node_child_process.cc

@@ -15,56 +15,55 @@ namespace node {
 
 using namespace v8;
 
-Persistent<FunctionTemplate> ChildProcess::constructor_template;
-
 static Persistent<String> pid_symbol;
-static Persistent<String> exit_symbol;
-static Persistent<String> output_symbol;
-static Persistent<String> error_symbol;
+static Persistent<String> onexit_symbol;
+
+
+// TODO share with other modules
+static inline int SetNonBlocking(int fd) {
+  int flags = fcntl(fd, F_GETFL, 0);
+  int r = fcntl(fd, F_SETFL, flags | O_NONBLOCK);
+  if (r != 0) {
+    perror("SetNonBlocking()");
+  }
+  return r;
+}
+
 
 void ChildProcess::Initialize(Handle<Object> target) {
   HandleScope scope;
 
   Local<FunctionTemplate> t = FunctionTemplate::New(ChildProcess::New);
-  constructor_template = Persistent<FunctionTemplate>::New(t);
-  constructor_template->Inherit(EventEmitter::constructor_template);
-  constructor_template->InstanceTemplate()->SetInternalFieldCount(1);
-  constructor_template->SetClassName(String::NewSymbol("ChildProcess"));
+  t->InstanceTemplate()->SetInternalFieldCount(1);
+  t->SetClassName(String::NewSymbol("ChildProcess"));
 
   pid_symbol = NODE_PSYMBOL("pid");
-  exit_symbol = NODE_PSYMBOL("exit");
-  output_symbol = NODE_PSYMBOL("output");
-  error_symbol = NODE_PSYMBOL("error");
+  onexit_symbol = NODE_PSYMBOL("onexit");
 
-  NODE_SET_PROTOTYPE_METHOD(constructor_template, "spawn", ChildProcess::Spawn);
-  NODE_SET_PROTOTYPE_METHOD(constructor_template, "write", ChildProcess::Write);
-  NODE_SET_PROTOTYPE_METHOD(constructor_template, "close", ChildProcess::Close);
-  NODE_SET_PROTOTYPE_METHOD(constructor_template, "kill", ChildProcess::Kill);
+  NODE_SET_PROTOTYPE_METHOD(t, "spawn", ChildProcess::Spawn);
+  NODE_SET_PROTOTYPE_METHOD(t, "kill", ChildProcess::Kill);
 
-  target->Set(String::NewSymbol("ChildProcess"),
-      constructor_template->GetFunction());
+  target->Set(String::NewSymbol("ChildProcess"), t->GetFunction());
 }
 
+
 Handle<Value> ChildProcess::New(const Arguments& args) {
   HandleScope scope;
-
   ChildProcess *p = new ChildProcess();
   p->Wrap(args.Holder());
-
   return args.This();
 }
 
+
 // This is an internal function. The third argument should be an array
 // of key value pairs seperated with '='.
 Handle<Value> ChildProcess::Spawn(const Arguments& args) {
   HandleScope scope;
 
-  if ( args.Length() != 3
-    || !args[0]->IsString()
-    || !args[1]->IsArray()
-    || !args[2]->IsArray()
-     )
-  {
+  if (args.Length() != 3 ||
+      !args[0]->IsString() ||
+      !args[1]->IsArray() ||
+      !args[2]->IsArray()) {
     return ThrowException(Exception::Error(String::New("Bad argument.")));
   }
 
@@ -98,7 +97,9 @@ Handle<Value> ChildProcess::Spawn(const Arguments& args) {
     env[i] = strdup(*pair);
   }
 
-  int r = child->Spawn(argv[0], argv, env);
+  int fds[3];
+
+  int r = child->Spawn(argv[0], argv, env, fds);
 
   for (i = 0; i < argv_length; i++) free(argv[i]);
   delete [] argv;
@@ -110,33 +111,19 @@ Handle<Value> ChildProcess::Spawn(const Arguments& args) {
     return ThrowException(Exception::Error(String::New("Error spawning")));
   }
 
-  child->handle_->Set(pid_symbol, Integer::New(child->pid_));
-
-  return Undefined();
-}
-
-Handle<Value> ChildProcess::Write(const Arguments& args) {
-  HandleScope scope;
-  ChildProcess *child = ObjectWrap::Unwrap<ChildProcess>(args.Holder());
-  assert(child);
-
-  enum encoding enc = ParseEncoding(args[1]);
-  ssize_t len = DecodeBytes(args[0], enc);
-
-  if (len < 0) {
-    Local<Value> exception = Exception::TypeError(String::New("Bad argument"));
-    return ThrowException(exception);
-  }
+  Local<Array> a = Array::New(3);
 
-  char * buf = new char[len];
-  ssize_t written = DecodeWrite(buf, len, args[0], enc);
-  assert(written == len);
-  int r = child->Write(buf, len);
-  delete [] buf;
+  assert(fds[0] >= 0);
+  a->Set(0, Integer::New(fds[0])); // stdin
+  assert(fds[1] >= 0);
+  a->Set(1, Integer::New(fds[1])); // stdout
+  assert(fds[2] >= 0);
+  a->Set(2, Integer::New(fds[2])); // stderr
 
-  return r == 0 ? True() : False();
+  return scope.Close(a);
 }
 
+
 Handle<Value> ChildProcess::Kill(const Arguments& args) {
   HandleScope scope;
   ChildProcess *child = ObjectWrap::Unwrap<ChildProcess>(args.Holder());
@@ -167,160 +154,59 @@ Handle<Value> ChildProcess::Kill(const Arguments& args) {
   return Undefined();
 }
 
-Handle<Value> ChildProcess::Close(const Arguments& args) {
-  HandleScope scope;
-  ChildProcess *child = ObjectWrap::Unwrap<ChildProcess>(args.Holder());
-  assert(child);
-  return child->Close() == 0 ? True() : False();
-}
-
-void ChildProcess::reader_closed(evcom_reader *r) {
-  ChildProcess *child = static_cast<ChildProcess*>(r->data);
-  if (r == &child->stdout_reader_) {
-    child->stdout_fd_ = -1;
-  } else {
-    assert(r == &child->stderr_reader_);
-    child->stderr_fd_ = -1;
-  }
-  evcom_reader_detach(r);
-  child->MaybeShutdown();
-}
-
-void ChildProcess::stdin_closed(evcom_writer *w) {
-  ChildProcess *child = static_cast<ChildProcess*>(w->data);
-  assert(w == &child->stdin_writer_);
-  child->stdin_fd_ = -1;
-  evcom_writer_detach(w);
-  child->MaybeShutdown();
-}
-
-void ChildProcess::on_read(evcom_reader *r, const void *buf, size_t len) {
-  ChildProcess *child = static_cast<ChildProcess*>(r->data);
-  HandleScope scope;
-
-  bool isSTDOUT = (r == &child->stdout_reader_);
-  enum encoding encoding = isSTDOUT ?
-    child->stdout_encoding_ : child->stderr_encoding_;
-
-  // TODO emit 'end' event instead of null.
-
-  Local<Value> data = len ? Encode(buf, len, encoding) : Local<Value>::New(Null());
-  child->Emit(isSTDOUT ? output_symbol : error_symbol, 1, &data);
-  child->MaybeShutdown();
-}
-
-ChildProcess::ChildProcess() : EventEmitter() {
-  evcom_reader_init(&stdout_reader_);
-  stdout_reader_.data     = this;
-  stdout_reader_.on_read  = on_read;
-  stdout_reader_.on_close = reader_closed;
-
-  evcom_reader_init(&stderr_reader_);
-  stderr_reader_.data     = this;
-  stderr_reader_.on_read  = on_read;
-  stderr_reader_.on_close = reader_closed;
-
-  evcom_writer_init(&stdin_writer_);
-  stdin_writer_.data      = this;
-  stdin_writer_.on_close  = stdin_closed;
-
-  ev_init(&child_watcher_, ChildProcess::OnCHLD);
-  child_watcher_.data = this;
-
-  stdout_fd_ = -1;
-  stderr_fd_ = -1;
-  stdin_fd_ = -1;
-
-  stdout_encoding_ = UTF8;
-  stderr_encoding_ = UTF8;
-
-  got_chld_ = false;
-  exit_code_ = 0;
-
-  pid_ = 0;
-}
-
-ChildProcess::~ChildProcess() {
-  Shutdown();
-}
 
-void ChildProcess::Shutdown() {
-  if (stdin_fd_ >= 0) {
-    evcom_writer_close(&stdin_writer_);
+void ChildProcess::Stop() {
+  if (ev_is_active(&child_watcher_)) {
+    ev_child_stop(EV_DEFAULT_UC_ &child_watcher_);
+    Unref();
   }
-
-  if (stdin_fd_  >= 0) close(stdin_fd_);
-  if (stdout_fd_ >= 0) close(stdout_fd_);
-  if (stderr_fd_ >= 0) close(stderr_fd_);
-
-  stdin_fd_ = -1;
-  stdout_fd_ = -1;
-  stderr_fd_ = -1;
-
-  evcom_writer_detach(&stdin_writer_);
-  evcom_reader_detach(&stdout_reader_);
-  evcom_reader_detach(&stderr_reader_);
-
-  ev_child_stop(EV_DEFAULT_UC_ &child_watcher_);
-
-  /* XXX Kill the PID? */
-  pid_ = 0;
+  // Don't kill the PID here. We want to allow for killing the parent
+  // process and reparenting to initd. This is perhaps not going the best
+  // technique for daemonizing, but I don't want to rule it out.
+  pid_ = -1;
 }
 
-static inline int SetNonBlocking(int fd) {
-  int flags = fcntl(fd, F_GETFL, 0);
-  int r = fcntl(fd, F_SETFL, flags | O_NONBLOCK);
-  if (r != 0) {
-    perror("SetNonBlocking()");
-  }
-  return r;
-}
 
 // Note that args[0] must be the same as the "file" param.  This is an
 // execvp() requirement.
-int ChildProcess::Spawn(const char *file, char *const args[], char **env) {
-  assert(pid_ == 0);
-  assert(stdout_fd_ == -1);
-  assert(stderr_fd_ == -1);
-  assert(stdin_fd_ == -1);
+//
+int ChildProcess::Spawn(const char *file,
+                        char *const args[],
+                        char **env,
+                        int stdio_fds[3]) {
+  HandleScope scope;
+  assert(pid_ == -1);
+  assert(!ev_is_active(&child_watcher_));
 
-  int stdout_pipe[2], stdin_pipe[2], stderr_pipe[2];
+  int stdin_pipe[2], stdout_pipe[2], stderr_pipe[2];
 
   /* An implementation of popen(), basically */
-  if (pipe(stdout_pipe) < 0) {
+  if (pipe(stdin_pipe) < 0 ||
+      pipe(stdout_pipe) < 0 ||
+      pipe(stderr_pipe) < 0) {
     perror("pipe()");
     return -1;
   }
 
-  if (pipe(stderr_pipe) < 0) {
-    perror("pipe()");
-    return -2;
-  }
-
-  if (pipe(stdin_pipe) < 0) {
-    perror("pipe()");
-    return -3;
-  }
-
   // Save environ in the case that we get it clobbered
   // by the child process.
   char **save_our_env = environ;
 
   switch (pid_ = vfork()) {
     case -1:  // Error.
-      Shutdown();
+      Stop();
       return -4;
 
     case 0:  // Child.
+      close(stdin_pipe[1]);  // close write end
+      dup2(stdin_pipe[0],  STDIN_FILENO);
+
       close(stdout_pipe[0]);  // close read end
       dup2(stdout_pipe[1], STDOUT_FILENO);
 
       close(stderr_pipe[0]);  // close read end
       dup2(stderr_pipe[1], STDERR_FILENO);
 
-      close(stdin_pipe[1]);  // close write end
-      dup2(stdin_pipe[0],  STDIN_FILENO);
-
       environ = env;
 
       execvp(file, args);
@@ -328,81 +214,59 @@ int ChildProcess::Spawn(const char *file, char *const args[], char **env) {
       _exit(127);
   }
 
+  // Parent.
+
   // Restore environment.
   environ = save_our_env;
 
-  // Parent.
-
   ev_child_set(&child_watcher_, pid_, 0);
   ev_child_start(EV_DEFAULT_UC_ &child_watcher_);
+  Ref();
+  handle_->Set(pid_symbol, Integer::New(pid_));
+
+  close(stdin_pipe[0]);
+  stdio_fds[0] = stdin_pipe[1];
+  SetNonBlocking(stdin_pipe[1]);
 
   close(stdout_pipe[1]);
-  stdout_fd_ = stdout_pipe[0];
-  SetNonBlocking(stdout_fd_);
+  stdio_fds[1] = stdout_pipe[0];
+  SetNonBlocking(stdout_pipe[0]);
 
   close(stderr_pipe[1]);
-  stderr_fd_ = stderr_pipe[0];
-  SetNonBlocking(stderr_fd_);
-
-  close(stdin_pipe[0]);
-  stdin_fd_ = stdin_pipe[1];
-  SetNonBlocking(stdin_fd_);
+  stdio_fds[2] = stderr_pipe[0];
+  SetNonBlocking(stderr_pipe[0]);
 
-  evcom_reader_set(&stdout_reader_, stdout_fd_);
-  evcom_reader_attach(EV_DEFAULT_UC_ &stdout_reader_);
-
-  evcom_reader_set(&stderr_reader_, stderr_fd_);
-  evcom_reader_attach(EV_DEFAULT_UC_ &stderr_reader_);
+  return 0;
+}
 
-  evcom_writer_set(&stdin_writer_, stdin_fd_);
-  evcom_writer_attach(EV_DEFAULT_UC_ &stdin_writer_);
 
-  Ref();
+void ChildProcess::OnExit(int code) {
+  HandleScope scope;
 
-  return 0;
-}
+  pid_ = -1;
+  Stop();
 
-void ChildProcess::OnCHLD(EV_P_ ev_child *watcher, int revents) {
-  ev_child_stop(EV_A_ watcher);
-  ChildProcess *child = static_cast<ChildProcess*>(watcher->data);
+  handle_->Set(pid_symbol, Null());
 
-  assert(revents == EV_CHILD);
-  assert(child->pid_ == watcher->rpid);
-  assert(&child->child_watcher_ == watcher);
+  Local<Value> onexit_v = handle_->Get(onexit_symbol);
+  assert(onexit_v->IsFunction());
+  Local<Function> onexit = Local<Function>::Cast(onexit_v);
 
-  child->got_chld_ = true;
-  child->exit_code_ = watcher->rstatus;
+  TryCatch try_catch;
 
-  if (child->stdin_fd_  >= 0) evcom_writer_close(&child->stdin_writer_);
+  Local<Value> argv[1];
+  argv[0] = Integer::New(code);
 
-  child->MaybeShutdown();
-}
+  onexit->Call(handle_, 1, argv);
 
-int ChildProcess::Write(const char *str, size_t len) {
-  if (stdin_fd_ < 0 || got_chld_) return -1;
-  evcom_writer_write(&stdin_writer_, str, len);
-  return 0;
+  if (try_catch.HasCaught()) {
+    FatalException(try_catch);
+  }
 }
 
-int ChildProcess::Close(void) {
-  if (stdin_fd_ < 0 || got_chld_) return -1;
-  evcom_writer_close(&stdin_writer_);
-  return 0;
-}
 
 int ChildProcess::Kill(int sig) {
-  if (got_chld_ || pid_ == 0) return -1;
   return kill(pid_, sig);
 }
 
-void ChildProcess::MaybeShutdown(void) {
-  if (stdout_fd_ < 0 && stderr_fd_ < 0 && got_chld_) {
-    HandleScope scope;
-    Handle<Value> argv[1] = { Integer::New(exit_code_) };
-    Emit(exit_symbol, 1, argv);
-    Shutdown();
-    Unref();
-  }
-}
-
 }  // namespace node

src/node_child_process.h

@@ -1,65 +1,68 @@
 // Copyright 2009 Ryan Dahl <ry@tinyclouds.org>
-#ifndef SRC_CHILD_PROCESS_H_
-#define SRC_CHILD_PROCESS_H_
+#ifndef NODE_CHILD_PROCESS_H_
+#define NODE_CHILD_PROCESS_H_
 
 #include <node.h>
-#include <node_events.h>
-
+#include <node_object_wrap.h>
 #include <v8.h>
 #include <ev.h>
-#include <evcom.h>
+
+// ChildProcess is a thin wrapper around ev_child. It has the extra
+// functionality that it can spawn a child process with pipes connected to
+// its stdin, stdout, stderr. This class is not meant to be exposed to but
+// wrapped up in a more friendly EventEmitter with streams for each of the
+// pipes.
+//
+// When the child process exits (when the parent receives SIGCHLD) the
+// callback child.onexit will be called.
 
 namespace node {
 
-class ChildProcess : EventEmitter {
+class ChildProcess : ObjectWrap {
  public:
   static void Initialize(v8::Handle<v8::Object> target);
 
  protected:
-  static v8::Persistent<v8::FunctionTemplate> constructor_template;
   static v8::Handle<v8::Value> New(const v8::Arguments& args);
   static v8::Handle<v8::Value> Spawn(const v8::Arguments& args);
-  static v8::Handle<v8::Value> Write(const v8::Arguments& args);
-  static v8::Handle<v8::Value> Close(const v8::Arguments& args);
   static v8::Handle<v8::Value> Kill(const v8::Arguments& args);
-  static v8::Handle<v8::Value> PIDGetter(v8::Local<v8::String> _,
-                                         const v8::AccessorInfo& info);
 
-  ChildProcess();
-  ~ChildProcess();
-
-  int Spawn(const char *file, char *const argv[], char **env);
-  int Write(const char *str, size_t len);
-  int Close(void);
+  ChildProcess() : ObjectWrap() {
+    ev_init(&child_watcher_, ChildProcess::on_chld);
+    child_watcher_.data = this;
+    pid_ = -1;
+  }
+
+  ~ChildProcess() {
+    Stop();
+  }
+
+  // Returns 0 on success. stdio_fds will contain file desciptors for stdin,
+  // stdout, and stderr of the subprocess. stdin is writable; the other two
+  // are readable.
+  // The user of this class has responsibility to close these pipes after
+  // the child process exits.
+  int Spawn(const char *file, char *const argv[], char **env, int stdio_fds[3]);
+
+  // Simple syscall wrapper. Does not disable the watcher. onexit will be
+  // called still.
   int Kill(int sig);
 
  private:
-  static void on_read(evcom_reader *r, const void *buf, size_t len);
-  static void reader_closed(evcom_reader *r);
-  static void stdin_closed(evcom_writer *w);
-  static void OnCHLD(EV_P_ ev_child *watcher, int revents);
-
-  void MaybeShutdown(void);
-  void Shutdown(void);
+  void OnExit(int code);
+  void Stop(void);
 
-  evcom_reader stdout_reader_;
-  evcom_reader stderr_reader_;
-  evcom_writer stdin_writer_;
+  static void on_chld(EV_P_ ev_child *watcher, int revents) {
+    ChildProcess *child = static_cast<ChildProcess*>(watcher->data);
+    assert(revents == EV_CHILD);
+    assert(child->pid_ == watcher->rpid);
+    assert(&child->child_watcher_ == watcher);
+    child->OnExit(watcher->rstatus);
+  }
 
   ev_child child_watcher_;
-
-  int stdout_fd_;
-  int stderr_fd_;
-  int stdin_fd_;
-
-  enum encoding stdout_encoding_;
-  enum encoding stderr_encoding_;
-
   pid_t pid_;
-
-  bool got_chld_;
-  int exit_code_;
 };
 
 }  // namespace node
-#endif  // SRC_CHILD_PROCESS_H_
+#endif  // NODE_CHILD_PROCESS_H_

test/fixtures/echo.js

@@ -1,12 +1,13 @@
 require("../common");
-process.stdio.open();
 
 print("hello world\r\n");
 
-process.stdio.addListener("data", function (data) {
-  print(data);
+var stdin = process.openStdin();
+
+stdin.addListener("data", function (data) {
+  process.stdout.write(data);
 });
 
-process.stdio.addListener("close", function () {
-  process.stdio.close();
+stdin.addListener("end", function () {
+  process.stdout.close();
 });

test/fixtures/print-chars.js

@@ -3,8 +3,8 @@ require("../common");
 var n = parseInt(process.argv[2]);
 
 var s = "";
-for (var i = 0; i < n-1; i++) {
+for (var i = 0; i < n; i++) {
   s += 'c';
 }
 
-puts(s); // \n is the nth char.
+process.stdout.write(s);

test/pummel/test-process-spawn-loop.js

@@ -1,33 +0,0 @@
-require("../common");
-
-var N = 40;
-var finished = false;
-
-function spawn (i) {
-  var child = process.createChildProcess( 'python'
-                                     , ['-c', 'print 500 * 1024 * "C"']
-                                     );
-  var output = "";
-
-  child.addListener("output", function(chunk) {
-    if (chunk) output += chunk;
-  });
-
-  child.addListener("error", function(chunk) {
-    if (chunk) error(chunk)
-  });
-
-  child.addListener("exit", function () {
-    puts(output);
-    if (i < N)
-      spawn(i+1);
-    else
-      finished = true;
-  });
-}
-
-spawn(0);
-
-process.addListener("exit", function () {
-  assert.equal(true, finished);
-});

test/simple/test-process-buffering.js → test/simple/test-child-process-buffering.js

@@ -1,14 +1,19 @@
 require("../common");
 
+var spawn = require('child_process').spawn;
+
 var pwd_called = false;
 
 function pwd (callback) {
   var output = "";
-  var child = process.createChildProcess("pwd");
-  child.addListener("output", function (s) {
+  var child = spawn("pwd");
+
+  child.stdout.setEncoding('utf8');
+  child.stdout.addListener("data", function (s) {
     puts("stdout: " + JSON.stringify(s));
-    if (s) output += s;
+    output += s;
   });
+
   child.addListener("exit", function (c) {
     puts("exit: " + c);
     assert.equal(0, c);

test/simple/test-child-process-env.js

@@ -1,10 +1,15 @@
 require("../common");
-child = process.createChildProcess('/usr/bin/env', [], {'HELLO' : 'WORLD'});
+
+var spawn = require('child_process').spawn;
+child = spawn('/usr/bin/env', [], {'HELLO' : 'WORLD'});
+
 response = "";
 
-child.addListener("output", function (chunk) {
-  puts("stdout: " + JSON.stringify(chunk));
-  if (chunk) response += chunk;
+child.stdout.setEncoding('utf8');
+
+child.stdout.addListener("data", function (chunk) {
+  puts("stdout: " + chunk);
+  response += chunk;
 });
 
 process.addListener('exit', function () {

test/simple/test-child-process-ipc.js

@@ -0,0 +1,41 @@
+require("../common");
+
+var spawn = require('child_process').spawn;
+
+var path = require('path');
+
+var sub = path.join(fixturesDir, 'echo.js');
+
+var gotHelloWorld = false;
+var gotEcho = false;
+
+var child = spawn(process.argv[0], [sub]);
+
+child.stderr.addListener("data", function (data){
+  puts("parent stderr: " + data);
+});
+
+child.stdout.setEncoding('utf8');
+
+child.stdout.addListener("data", function (data){
+  puts('child said: ' + JSON.stringify(data));
+  if (!gotHelloWorld) {
+    assert.equal("hello world\r\n", data);
+    gotHelloWorld = true;
+    child.stdin.write('echo me\r\n');
+  } else {
+    assert.equal("echo me\r\n", data);
+    gotEcho = true;
+    child.stdin.close();
+  }
+});
+
+child.stdout.addListener("end", function (data){
+  puts('child end');
+});
+
+
+process.addListener('exit', function () {
+  assert.ok(gotHelloWorld);
+  assert.ok(gotEcho);
+});

test/simple/test-child-process-kill.js

@@ -0,0 +1,38 @@
+require("../common");
+
+var spawn = require('child_process').spawn;
+
+var exitStatus = -1;
+var gotStdoutEOF = false;
+var gotStderrEOF = false;
+
+var cat = spawn("cat");
+
+
+cat.stdout.addListener("data", function (chunk) {
+  assert.ok(false);
+});
+
+cat.stdout.addListener("end", function () {
+  gotStdoutEOF = true;
+});
+
+cat.stderr.addListener("data", function (chunk) {
+  assert.ok(false);
+});
+
+cat.stderr.addListener("end", function () {
+  gotStderrEOF = true;
+});
+
+cat.addListener("exit", function (status) {
+  exitStatus = status;
+});
+
+cat.kill();
+
+process.addListener("exit", function () {
+  assert.ok(exitStatus > 0);
+  assert.ok(gotStdoutEOF);
+  assert.ok(gotStderrEOF);
+});

test/simple/test-child-process-spawn-loop.js

@@ -0,0 +1,36 @@
+require("../common");
+
+var spawn = require('child_process').spawn;
+
+var SIZE = 1000 * 1024;
+var N = 40;
+var finished = false;
+
+function doSpawn (i) {
+  var child = spawn( 'python', ['-c', 'print ' + SIZE + ' * "C"']);
+  var count = 0;
+
+  child.stdout.setEncoding('ascii');
+  child.stdout.addListener("data", function (chunk) {
+    count += chunk.length;
+  });
+
+  child.stderr.addListener("data", function (chunk) {
+    puts('stderr: ' + chunk);
+  });
+
+  child.addListener("exit", function () {
+    assert.equal(SIZE + 1, count); // + 1 for \n
+    if (i < N) {
+      doSpawn(i+1);
+    } else {
+      finished = true;
+    }
+  });
+}
+
+doSpawn(0);
+
+process.addListener("exit", function () {
+  assert.ok(finished);
+});

test/simple/test-child-process-stdin.js

@@ -0,0 +1,48 @@
+require("../common");
+
+var spawn = require('child_process').spawn;
+
+var cat = spawn("cat");
+cat.stdin.write("hello");
+cat.stdin.write(" ");
+cat.stdin.write("world");
+cat.stdin.close();
+
+var response = "";
+var exitStatus = -1;
+
+var gotStdoutEOF = false;
+
+cat.stdout.setEncoding('utf8');
+cat.stdout.addListener("data", function (chunk) {
+  puts("stdout: " + chunk);
+  response += chunk;
+});
+
+cat.stdout.addListener('end', function () {
+  gotStdoutEOF = true;
+});
+
+
+var gotStderrEOF = false;
+
+cat.stderr.addListener("data", function (chunk) {
+  // shouldn't get any stderr output
+  assert.ok(false);
+});
+
+cat.stderr.addListener("end", function (chunk) {
+  gotStderrEOF = true;
+});
+
+
+cat.addListener("exit", function (status) {
+  puts("exit event");
+  exitStatus = status;
+  assert.equal("hello world", response);
+});
+
+process.addListener("exit", function () {
+  assert.equal(0, exitStatus);
+  assert.equal("hello world", response);
+});

test/simple/test-child-process-stdout-flush.js

@@ -0,0 +1,27 @@
+require("../common");
+var path = require('path');
+var spawn = require('child_process').spawn;
+var sub = path.join(fixturesDir, 'print-chars.js');
+
+n = 500000;
+
+var child = spawn(process.argv[0], [sub, n]);
+
+var count = 0;
+
+child.stderr.setEncoding('utf8');
+child.stderr.addListener("data", function (data) {
+  puts("parent stderr: " + data);
+  assert.ok(false);
+});
+
+child.stderr.setEncoding('utf8');
+child.stdout.addListener("data", function (data) {
+  count += data.length;
+  puts(count);
+});
+
+child.addListener("exit", function (data) {
+  assert.equal(n, count);
+  puts("okay");
+});

test/simple/test-exec.js

@@ -1,5 +1,5 @@
 require("../common");
-
+var exec = require('child_process').exec;
 success_count = 0;
 error_count = 0;
 

test/simple/test-http-parser.js

@@ -1,12 +1,13 @@
-process.mixin(require("../common"));
+require("../common");
 
 // The purpose of this test is not to check HTTP compliance but to test the
 // binding. Tests for pathological http messages should be submitted
 // upstream to http://github.com/ry/http-parser for inclusion into
 // deps/http-parser/test.c
 
+var HTTPParser = process.binding('http_parser').HTTPParser;
 
-var parser = new process.HTTPParser("request");
+var parser = new HTTPParser("request");
 
 var buffer = new process.Buffer(1024);
 

test/simple/test-process-kill.js

@@ -1,15 +0,0 @@
-require("../common");
-
-var exit_status = -1;
-
-var cat = process.createChildProcess("cat");
-
-cat.addListener("output", function (chunk) { assert.equal(null, chunk); });
-cat.addListener("error", function (chunk) { assert.equal(null, chunk); });
-cat.addListener("exit", function (status) { exit_status = status; });
-
-cat.kill();
-
-process.addListener("exit", function () {
-  assert.equal(true, exit_status > 0);
-});

test/simple/test-process-simple.js

@@ -1,34 +0,0 @@
-require("../common");
-
-var cat = process.createChildProcess("cat");
-
-var response = "";
-var exit_status = -1;
-
-cat.addListener("output", function (chunk) {
-  puts("stdout: " + JSON.stringify(chunk));
-  if (chunk) {
-    response += chunk;
-    if (response === "hello world") {
-      puts("closing cat");
-      cat.close();
-    }
-  }
-});
-cat.addListener("error", function (chunk) {
-  puts("stderr: " + JSON.stringify(chunk));
-  assert.equal(null, chunk);
-});
-cat.addListener("exit", function (status) {
-  puts("exit event");
-  exit_status = status;
-});
-
-cat.write("hello");
-cat.write(" ");
-cat.write("world");
-
-process.addListener("exit", function () {
-  assert.equal(0, exit_status);
-  assert.equal("hello world", response);
-});

test/simple/test-stdio.js

@@ -1,36 +0,0 @@
-require("../common");
-var path = require('path');
-
-var sub = path.join(fixturesDir, 'echo.js');
-
-var gotHelloWorld = false;
-var gotEcho = false;
-
-var child = process.createChildProcess(process.argv[0], [sub]);
-
-child.addListener("error", function (data){
-  puts("parent stderr: " + data);
-});
-
-child.addListener("output", function (data){
-  if (data) {
-    puts('child said: ' + JSON.stringify(data));
-    if (!gotHelloWorld) {
-      assert.equal("hello world\r\n", data);
-      gotHelloWorld = true;
-      child.write('echo me\r\n');
-    } else {
-      assert.equal("echo me\r\n", data);
-      gotEcho = true;
-      child.close();
-    }
-  } else {
-    puts('child end');
-  }
-});
-
-
-process.addListener('exit', function () {
-  assert.ok(gotHelloWorld);
-  assert.ok(gotEcho);
-});

test/simple/test-stdout-flush.js

@@ -1,29 +0,0 @@
-require("../common");
-var path = require('path');
-
-var sub = path.join(fixturesDir, 'print-chars.js');
-
-n = 100000;
-
-var child = process.createChildProcess(process.argv[0], [sub, n]);
-
-var count = 0;
-
-child.addListener("error", function (data){
-  if (data) {
-    puts("parent stderr: " + data);
-    assert.ok(false);
-  }
-});
-
-child.addListener("output", function (data){
-  if (data) {
-    count += data.length;
-    puts(count);
-  }
-});
-
-child.addListener("exit", function (data) {
-  assert.equal(n, count);
-  puts("okay");
-});