src: close libuv handles on exit

Commit 556b890 added a call to uv_loop_delete() with the intent of
catching handle lifecycle bugs.  It worked because it exposed one:

    process.on('exit', function() {
      console.log('bye');  // Asserts.
    });

When run, it asserts with the following message:

    Assertion failed: (!uv__has_active_reqs(loop)), function
    uv__loop_delete, file ../deps/uv/src/unix/loop.c, line 150.

That's because libuv as of joyent/libuv@3f2d4d5 checks that there are
no in-flight requests when the event loop is destroyed.  In the test
case above, the write request for the string hasn't completed yet by
the time node.js exits: the string itself has most likely been written
but libuv hasn't had the opportunity to return the write request to
node.js.

That's why this commit adds a cleanup step right before exit where it
explicitly closes all open handles, then waits until the event loop
exits naturally.

Named pipes (UNIX domain sockets) are shut down first in order to flush
pending write requests.  Should go some way towards fixing the Windows
issue where output on stdout/stderr sometimes gets truncated.

Fixes joyent/libuv#911.

diff --git a/src/node.cc b/src/node.cc
index b8270f7..989c025 100644 (file)
--- a/src/node.cc
+++ b/src/node.cc
@@ -197,6 +197,14 @@ static uv_async_t emit_debug_enabled_async;
 // Declared in node_internals.h
 Isolate* node_isolate = NULL;
 
+enum ProcessState {
+  INITIALIZING,
+  RUNNING,
+  SHUTTING_DOWN
+};
+
+static ProcessState process_state = INITIALIZING;
+
 
 class ArrayBufferAllocator : public ArrayBuffer::Allocator {
  public:
@@ -1026,6 +1034,9 @@ MakeCallback(const Handle<Object> object,
              const Handle<Function> callback,
              int argc,
              Handle<Value> argv[]) {
+  if (process_state == SHUTTING_DOWN)
+    return Undefined(node_isolate);
+
   // TODO(trevnorris) Hook for long stack traces to be made here.
   Local<Object> process = PersistentToLocal(node_isolate, process_p);
 
@@ -1714,9 +1725,66 @@ static void InitGroups(const FunctionCallbackInfo<Value>& args) {
 #endif  // __POSIX__ && !defined(__ANDROID__)
 
 
+void MaybeCloseNamedPipe(uv_shutdown_t* req, int status) {
+  uv_handle_t* handle = reinterpret_cast<uv_handle_t*>(req->handle);
+  delete req;
+
+  if (status == UV_ECANCELED) {
+    return;  // Already closing.
+  }
+  if (status == UV_EPIPE) {
+    return;  // Read end went away before shutdown completed.
+  }
+  assert(status == 0);
+
+  if (uv_is_closing(handle)) {
+    return;
+  }
+
+  uv_close(handle, NULL);
+}
+
+
+void MaybeCloseHandle(uv_handle_t* handle, void* unused) {
+  if (uv_is_closing(handle)) {
+    return;
+  }
+
+  // Named pipes get special treatment: we do a shutdown first to flush
+  // pending writes.  Avoids truncated stdout/stderr output on Windows.
+  bool do_shutdown = handle->type == UV_NAMED_PIPE &&
+                     uv_is_writable(reinterpret_cast<uv_stream_t*>(handle));
+
+  if (do_shutdown == false) {
+    uv_close(handle, NULL);
+    return;
+  }
+
+  uv_shutdown_t* req = new uv_shutdown_t;
+  uv_stream_t* stream = reinterpret_cast<uv_stream_t*>(handle);
+  int err = uv_shutdown(req, stream, MaybeCloseNamedPipe);
+  if (err) {
+    assert(err == UV_ENOTCONN);
+    delete req;
+  }
+}
+
+
+void DisposeEventLoop(uv_loop_t* loop) {
+  // Don't call into JS land from now on.
+  process_state = SHUTTING_DOWN;
+  // Force-close open handles.
+  uv_walk(loop, MaybeCloseHandle, NULL);
+  uv_run(loop, UV_RUN_DEFAULT);
+  uv_loop_delete(loop);
+}
+
+
 void Exit(const FunctionCallbackInfo<Value>& args) {
   HandleScope scope(node_isolate);
-  exit(args[0]->IntegerValue());
+  int32_t exit_code = args[0]->Int32Value();
+  DisposeEventLoop(uv_default_loop());
+  exit(exit_code);
 }
 
 
@@ -3170,17 +3238,15 @@ int Start(int argc, char *argv[]) {
     // there are no watchers on the loop (except for the ones that were
     // uv_unref'd) then this function exits. As long as there are active
     // watchers, it blocks.
+    process_state = RUNNING;
     uv_run(uv_default_loop(), UV_RUN_DEFAULT);
 
     EmitExit(process_l);
     RunAtExit();
   }
 
-#ifndef NDEBUG
-  // Clean up. Not strictly necessary.
+  DisposeEventLoop(uv_default_loop());
   V8::Dispose();
-  uv_loop_delete(uv_default_loop());
-#endif  // NDEBUG
 
   // Clean up the copy:
   free(argv_copy);

diff --git a/test/simple/test-process-exit-print.js b/test/simple/test-process-exit-print.js
new file mode 100644 (file)
index 0000000..3addc73
--- /dev/null
+++ b/test/simple/test-process-exit-print.js
@@ -0,0 +1,27 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+
+process.on('exit', function() {
+  console.log('');  // Should not assert, see joyent/libuv#911.
+});