Paused stream keeps node alive

[piscisaureus]

Test code:

var conn = require('net').connect(80, 'www.google.com');
// Should not be necessary but just to be sure:
conn.pause();

Both node v0.10.x and master hang and don't exit.

@isaacs, any idea? The libuv handle is clearly reading; is node secretly reading under the hood?

[indutny]

Why should it exit?

[piscisaureus]

@indutny Because no more events will happen. Like this:

process.stdin.resume();
process.stdin.pause();

[trevnorris]

This is a good point. The only way it could be resumed is if another asynchronous event is fired after. Which would also keep the process alive. So there should be no need to keep the process open on pause().

[tjfontaine]

see also #6305, also for some reason ._handle.readStop() isn't being triggered here, I would have presumed we would have hit it when hwm was reached.

[tjfontaine]

ok, so in this case we're hanging because there's no data to be read from the socket since http doesn't respond until you actually GET something, if you switch to towel.blinkenlights.nl:23 you'll eventually hit hwm and dump out. So, with that in mind I have a semi working patch, that subscribes net.Socket to the pause handler and readStops it unfortunately breaks 2 tests

diff --git a/lib/net.js b/lib/net.js
index 93d942a..000bad4 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -167,6 +167,7 @@ function Socket(options) {
   // shut down the socket when we're finished with it.
   this.on('finish', onSocketFinish);
   this.on('_socketEnd', onSocketEnd);
+  this.on('pause', onSocketPause);

   initSocketHandle(this);

@@ -220,6 +221,13 @@ function onSocketFinish() {
 }


+function onSocketPause() {
+  debug('onSocketPause');
+  if (this._handle && this._handle.readStop)
+    this._handle.readStop();
+}
+
+
 function afterShutdown(status, handle, req) {
   var self = handle.owner;

[jasnell]

@joyent/node-coreteam @piscisaureus ... the hang still happens in v0.10 but not in v0.12 or io.js... do we care? Is there a fix we can / should backport to v0.10?

[piscisaureus]

This is a streams question - @cjihrig or @chrisdickinson may know the answer

[chrisdickinson]

Yep, looks like I patched it to solve #8200: 4ef2a5a. Downside is that it adds a new API (isPaused.)

[jasnell]

Hmm.. I'm inclined just to close then and leave the v0.10 behavior as is.

[jasnell]

Closing. This leaves the existing v0.10 behavior alone. Can reopen if folks feel it's necessary to fix this in v0.10