This repository has been archived by the owner. It is now read-only.

Fix blocking / non-blocking stdio woes #3584

Closed
piscisaureus opened this Issue Jun 29, 2012 · 49 comments

Comments

Projects
None yet
@piscisaureus
Member

piscisaureus commented Jun 29, 2012

Currently process.stdin / stdout / stderr is blocking, except when it is a pipe on windows. Weird and surprising. Very unpractical in cases where stdio is used as an IPC mechanism between node processes.

Discussion:http://logs.nodejs.org/libuv/2012-06-29#00:40:38.256

Preliminary results: have process.stdout.setBlocking(true|false) or process.stdout.writeSync.

Feel free to post thoughts here.

@cowboy

This comment has been minimized.

Show comment
Hide comment
@cowboy

cowboy Aug 26, 2012

I've experienced a similar issue with stdio in Windows, and have somewhat worked around it in the meantime with this horrible, horrible hack.

While my mini exit lib works well in place of process.exit in some cases, it completely fails to help in this case:

var exitHack = require('./exit').exit;

// If there are pending stdout / stderr writes when "condition" is met...
if (condition) {
  exitHack();
}

// ...this code will still do something. Whoops!
doSomething();

I haven't seen this problem in OS X or Linux. It would be great if Node.js behaved the same cross-OS.

cowboy commented Aug 26, 2012

I've experienced a similar issue with stdio in Windows, and have somewhat worked around it in the meantime with this horrible, horrible hack.

While my mini exit lib works well in place of process.exit in some cases, it completely fails to help in this case:

var exitHack = require('./exit').exit;

// If there are pending stdout / stderr writes when "condition" is met...
if (condition) {
  exitHack();
}

// ...this code will still do something. Whoops!
doSomething();

I haven't seen this problem in OS X or Linux. It would be great if Node.js behaved the same cross-OS.

@bpasero

This comment has been minimized.

Show comment
Hide comment
@bpasero

bpasero Jan 17, 2013

Any update? Will this land in 0.10?

bpasero commented Jan 17, 2013

Any update? Will this land in 0.10?

@isaacs

This comment has been minimized.

Show comment
Hide comment
@isaacs

isaacs Mar 6, 2013

This issue has been stagnant for months. In order for this to happen, it will need a champion.

Are you that champion? Would you like to see stdio have a more consistently blocking/nonblocking interface, which works the same on Windows and Unix?

Heed the call. Build the bits that need to go into libuv. Sketch out an API in node for it.

It seems that it won't make the 0.10 cut-off, I'm afraid. Maybe next pass.

isaacs commented Mar 6, 2013

This issue has been stagnant for months. In order for this to happen, it will need a champion.

Are you that champion? Would you like to see stdio have a more consistently blocking/nonblocking interface, which works the same on Windows and Unix?

Heed the call. Build the bits that need to go into libuv. Sketch out an API in node for it.

It seems that it won't make the 0.10 cut-off, I'm afraid. Maybe next pass.

@HenryRawas

This comment has been minimized.

Show comment
Hide comment
@HenryRawas

HenryRawas May 24, 2013

I have started to look into this issue. Here is what I have found so far.
If stdout is not piped, then the output goes through libuv tty code which is blocking as expected.
If stdout is piped, the output goes through libuv pipe code.
The pipe code checks to see if the pipe supports overlapped IO.
In the case of piped stdout it does not - NtQueryInformationFile returns a mode of FILE_SYNCHRONOUS_IO_NONALERT, which means:
All operations on the file are performed synchronously.
Wait requests in the system that must synchronize I/O queuing and completion are not subject to alerts
In this case the pipe code uses the thread pool to perform all IO.
So when the process exits, the write requests are queued up for the thread pool to execute, but they are never executed.

I made a quick hack to simply let the Write operations execute synchronously when the pipe is in this mode.
In the tests that I have, this fixes the problem of data being lost when the process exits.

This approach means that stdout would have blocking IO behavior even when piped, without requiring any new API.

Other pipes that have mode FILE_SYNCHRONOUS_IO_NONALERT or FILE_SYNCHRONOUS_IO_ALERT would also end up being blocking.
These modes may be set on a pipe or file when creating it, but libuv never sets it.

Does this seem like an acceptable way of fixing this issue?

HenryRawas commented May 24, 2013

I have started to look into this issue. Here is what I have found so far.
If stdout is not piped, then the output goes through libuv tty code which is blocking as expected.
If stdout is piped, the output goes through libuv pipe code.
The pipe code checks to see if the pipe supports overlapped IO.
In the case of piped stdout it does not - NtQueryInformationFile returns a mode of FILE_SYNCHRONOUS_IO_NONALERT, which means:
All operations on the file are performed synchronously.
Wait requests in the system that must synchronize I/O queuing and completion are not subject to alerts
In this case the pipe code uses the thread pool to perform all IO.
So when the process exits, the write requests are queued up for the thread pool to execute, but they are never executed.

I made a quick hack to simply let the Write operations execute synchronously when the pipe is in this mode.
In the tests that I have, this fixes the problem of data being lost when the process exits.

This approach means that stdout would have blocking IO behavior even when piped, without requiring any new API.

Other pipes that have mode FILE_SYNCHRONOUS_IO_NONALERT or FILE_SYNCHRONOUS_IO_ALERT would also end up being blocking.
These modes may be set on a pipe or file when creating it, but libuv never sets it.

Does this seem like an acceptable way of fixing this issue?

@bpasero

This comment has been minimized.

Show comment
Hide comment
@bpasero

bpasero May 29, 2013

Excellent! Any chance of getting this into node 0.10.x? This fix will allow me to get rid of ugly workarounds I need to use currently to not loose process output.

bpasero commented May 29, 2013

Excellent! Any chance of getting this into node 0.10.x? This fix will allow me to get rid of ugly workarounds I need to use currently to not loose process output.

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis May 29, 2013

Member

@piscisaureus and/or @sblom: request for comment, please.

Member

bnoordhuis commented May 29, 2013

@piscisaureus and/or @sblom: request for comment, please.

@HenryRawas

This comment has been minimized.

Show comment
Hide comment
@HenryRawas

HenryRawas May 29, 2013

Second pull request fixes commit message problems.

HenryRawas commented May 29, 2013

Second pull request fixes commit message problems.

@sblom

This comment has been minimized.

Show comment
Hide comment
@sblom

sblom May 30, 2013

@HenryRawas works with me. I'll look at thiS.

sblom commented May 30, 2013

@HenryRawas works with me. I'll look at thiS.

@HenryRawas

This comment has been minimized.

Show comment
Hide comment
@HenryRawas

HenryRawas Jun 10, 2013

Updated the proposed fix. Added API uv_pipe_setblocking() to libuv and changed windows pipe to use synchronous writes if this is set. Updated node code to call uv_pipe_setblocking() for stdio over pipe.

HenryRawas commented Jun 10, 2013

Updated the proposed fix. Added API uv_pipe_setblocking() to libuv and changed windows pipe to use synchronous writes if this is set. Updated node code to call uv_pipe_setblocking() for stdio over pipe.

@bpasero

This comment has been minimized.

Show comment
Hide comment
@bpasero

bpasero Jun 14, 2013

@HenryRawas is it realistic to get this fix in for 0.10.x?

bpasero commented Jun 14, 2013

@HenryRawas is it realistic to get this fix in for 0.10.x?

@HenryRawas

This comment has been minimized.

Show comment
Hide comment
@HenryRawas

HenryRawas Jun 14, 2013

I am still trying to get the change accepted. I don’t know where it will land.

From: Benjamin Pasero [mailto:notifications@github.com]
Sent: Friday, June 14, 2013 2:07 AM
To: joyent/node
Cc: Henry Rawas
Subject: Re: [node] Fix blocking / non-blocking stdio woes (#3584)

@HenryRawashttps://github.com/HenryRawas is it realistic to get this fix in for 0.10.x?


Reply to this email directly or view it on GitHubhttps://github.com//issues/3584#issuecomment-19446612.

HenryRawas commented Jun 14, 2013

I am still trying to get the change accepted. I don’t know where it will land.

From: Benjamin Pasero [mailto:notifications@github.com]
Sent: Friday, June 14, 2013 2:07 AM
To: joyent/node
Cc: Henry Rawas
Subject: Re: [node] Fix blocking / non-blocking stdio woes (#3584)

@HenryRawashttps://github.com/HenryRawas is it realistic to get this fix in for 0.10.x?


Reply to this email directly or view it on GitHubhttps://github.com//issues/3584#issuecomment-19446612.

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Jun 27, 2013

Member

Just wanted to 👍 this issue. Was just bit by it in Windows 8 with Node v0.10.12. I'm still looking for a workaround atm.

Member

jdalton commented Jun 27, 2013

Just wanted to 👍 this issue. Was just bit by it in Windows 8 with Node v0.10.12. I'm still looking for a workaround atm.

@dcherman

This comment has been minimized.

Show comment
Hide comment
@dcherman

dcherman Jun 27, 2013

@jdalton Did you try the pipe to file workaround?

dcherman commented Jun 27, 2013

@jdalton Did you try the pipe to file workaround?

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Jun 27, 2013

Member

@dcherman No I haven't. Did I miss it already covered?

Here is what I'm trying to do. I noticed that the stdout output was truncated:

var foo = cp.spawn(fooPath, fooOptions);

var output = '';
foo.stdout.on('data', function(data) {
  output += data;
});

foo.on('exit', function() {
  fs.writeFileSync(outputPath, output, 'utf8');
});
Member

jdalton commented Jun 27, 2013

@dcherman No I haven't. Did I miss it already covered?

Here is what I'm trying to do. I noticed that the stdout output was truncated:

var foo = cp.spawn(fooPath, fooOptions);

var output = '';
foo.stdout.on('data', function(data) {
  output += data;
});

foo.on('exit', function() {
  fs.writeFileSync(outputPath, output, 'utf8');
});
@cowboy

This comment has been minimized.

Show comment
Hide comment
@cowboy

cowboy Jun 27, 2013

No matter what, don't do string concat like that. Use Buffer instances and .concat them.

cowboy commented Jun 27, 2013

No matter what, don't do string concat like that. Use Buffer instances and .concat them.

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Jun 27, 2013

Member

@dcherman Woo doing:

var logStream = fs.createWriteStream(outputPath);
foo.stdout.pipe(logStream);
foo.on('exit', function() {
  // do some cleanup
});

seems to work :)

Member

jdalton commented Jun 27, 2013

@dcherman Woo doing:

var logStream = fs.createWriteStream(outputPath);
foo.stdout.pipe(logStream);
foo.on('exit', function() {
  // do some cleanup
});

seems to work :)

@jdalton

This comment has been minimized.

Show comment
Hide comment
@jdalton

jdalton Jun 28, 2013

Member

Actually nix my issue. I tried it manually via the console and for some reason > isn't capturing all the stdout that's normally being displayed to the console. Looks like it's not a node issue.

Member

jdalton commented Jun 28, 2013

Actually nix my issue. I tried it manually via the console and for some reason > isn't capturing all the stdout that's normally being displayed to the console. Looks like it's not a node issue.

sonicdoe added a commit to sonicdoe/brave-mouse that referenced this issue Jan 31, 2015

avital added a commit to meteor/meteor that referenced this issue Feb 2, 2015

Flush buffer on `process.exit` in Windows
This works around a bug that's present in Node
0.8 and 0.10. This will likely be fixed in 0.12.
nodejs/node-v0.x-archive#3584

amavisca pushed a commit to jasmine/jasmine-npm that referenced this issue Feb 4, 2015

Christopher Amavisca
Use exit module instead of process.exit for Windows workaround
- process.exit isn't properly waiting for buffers to finish writing in Windows
- issue in node fixed in v0.11.12: nodejs/node-v0.x-archive#3584

Fixes #20

agebert added a commit to e2ebridge/e2e-conf that referenced this issue Feb 6, 2015

bug: Fix for Windows' drain error on process.exit (Issue 3584)
Output to stderr or stdout is not always completly written if
you exit the process. Issue happens only on Windows using pipes.

Issue nodejs/node-v0.x-archive#3584 will be fixed in
node.js 0.12.

@springmeyer springmeyer referenced this issue Feb 23, 2015

Open

Tests for atom-shell startup #1208

0 of 4 tasks complete

pmeijer added a commit to webgme/webgme that referenced this issue Apr 6, 2015

@pgilad

This comment has been minimized.

Show comment
Hide comment
@pgilad

pgilad May 26, 2015

Hi, is this considered resolved? Is the use of https://github.com/cowboy/node-exit still recommended for some versions of node?

pgilad commented May 26, 2015

Hi, is this considered resolved? Is the use of https://github.com/cowboy/node-exit still recommended for some versions of node?

@orangemocha

This comment has been minimized.

Show comment
Hide comment
@orangemocha

orangemocha May 26, 2015

Member

Yes, it is resolved. It was fixed with 20176a9.
You shouldn't need the workaround anymore.

Member

orangemocha commented May 26, 2015

Yes, it is resolved. It was fixed with 20176a9.
You shouldn't need the workaround anymore.

@pgilad

This comment has been minimized.

Show comment
Hide comment
@pgilad

pgilad May 26, 2015

Great, thanks!

pgilad commented May 26, 2015

Great, thanks!

@gigapromoters

This comment has been minimized.

Show comment
Hide comment
@gigapromoters

gigapromoters Jun 1, 2015

Issue still exists for me in 0.12.4, Windows 8.1:

var child = require('child_process').spawn('php',
['-S', 'localhost:8007']);
child.stdout.on('data', function(data) {
console.log("child: "+data.toString());
});

Not sure if this is a bug, but if the process is infinite - then the child.stdout's callback is never triggered.

gigapromoters commented Jun 1, 2015

Issue still exists for me in 0.12.4, Windows 8.1:

var child = require('child_process').spawn('php',
['-S', 'localhost:8007']);
child.stdout.on('data', function(data) {
console.log("child: "+data.toString());
});

Not sure if this is a bug, but if the process is infinite - then the child.stdout's callback is never triggered.

@gigapromoters

This comment has been minimized.

Show comment
Hide comment
@gigapromoters

gigapromoters Jun 1, 2015

I wrapped php process in another child script, now it works fine.

gigapromoters commented Jun 1, 2015

I wrapped php process in another child script, now it works fine.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.