Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Any way to to turn console into fully synchronous? #11568

Closed
litmit opened this issue Feb 26, 2017 · 18 comments
Closed

Any way to to turn console into fully synchronous? #11568

litmit opened this issue Feb 26, 2017 · 18 comments
Labels
console Issues and PRs related to the console subsystem. question Issues that look for answers.

Comments

@litmit
Copy link

litmit commented Feb 26, 2017

With Node.js possible to write nice small an reliable fast synchronous console applications using Javascript.

But these applications may consume all available memory and crash with
message FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory.
After investigation was founded cause of crash - console.log()
See also #3524 for details.

With Node 4.x console.log() worked in synchronous mode when stdout redirected to file, therefore this problem has workaround, but not with Node 6.x (and this not described in doc https://nodejs.org/api/console.html#console_asynchronous_vs_synchronous_consoles)

Please, add feature which allow to turn console into fully synchronous mode!
This may be command line option (--console-sync) or API call like console.mode('sync') or something else.

Without this it very hard to debug a synchronous application using console.log() or console.error() when an application generate a tons of output.

@mscdex mscdex added console Issues and PRs related to the console subsystem. question Issues that look for answers. labels Feb 26, 2017
@Fishrock123
Copy link
Contributor

Fishrock123 commented Feb 27, 2017

There is more info here: https://github.com/nodejs/node/blob/master/doc/api/process.md#a-note-on-process-io

It will be up on the live docs in the next 7.x release.

The console (really process.{stdout|stderr}) is mostly synchronous but is sometimes not and there are good reasons for it. (mostly, maybe except windows pipes)

Please read the docs linked above before doing this.
You can force it into sync via:

if (process.stdout._handle) process.stdout._handle.setBlocking(true)

But, I don't recommend it.

Please let me know if you have more questions.

@litmit
Copy link
Author

litmit commented Feb 27, 2017

@Fishrock123

Writes may be synchronous depending on the what the stream is connected to and whether the system is Windows or Unix:

Files: synchronous on Windows and Linux

If console is mostly synchronous why console.log() eat all memory and crash as this described in #3524?
(on Node 6.x and 7.x)

Moreover @bnoordhuis wrote:

It's expected behavior: console.log is asynchronous,

I wrote a small package console-sync which replace console methods to be true synchronous, like this:

console.log = function log()
{
   fs.writeSync(this._stdout.fd, util.format.apply(null,arguments) + "\n");
} 

It some kind of hack but fixes problem with memory leaks and crashes. Moreover it works noticeably faster than the default asynchronous implementation!

console.time('console.log');
for(var i=0; i<1000000 ;++i)
{
   console.log("Value of i=%d",i);
}
console.timeEnd('console.log');

console.log: 10449.842ms

require('console-sync')
console.time('console.log sync');
for(var i=0; i<1000000 ;++i)
{
   console.log("Value of i=%d",i);
}
console.timeEnd('console.log sync');

console.log sync: 7735.458ms

@Fishrock123
Copy link
Contributor

It's expected behavior: console.log is asynchronous,

It some kind of hack but fixes problem with memory leaks and crashes. Moreover it works noticeably faster than the default asynchronous implementation!

It is not asynchronous unless you are in one of the conditions as documented above. (I made this change.)

In some older versions the behaviour was different.

It still goes through libuv blocking or not so it's possible that has some overhead.

@litmit
Copy link
Author

litmit commented Feb 28, 2017

@Fishrock123 thanks for clarifications.

I'm thinking that i found a cause of leaks in the console.
It is PR: #9744
Without this PR the infinite console loop does not consume a memory at all and does not crash Node.js.

bug.js:

for(var i=0; ;++i)
{
   console.log("Value of i=%d",i);
}

To ensure, that console is synchronous run this script with redirection to file:

node bug.js > out.txt

console.js from commit 2ebd445 working well.
console.js from commit f18e08d cause memory leaks.

Task manager screenshot: how works console.js from commit f18e08d
f18e08d820dde161788d95a5603546ceca021e90

@litmit
Copy link
Author

litmit commented Feb 28, 2017

But meanwhile...
It is theoretically possible to improve write function in console.js?
Some pseudocode:

function write(......) {
   if ( stream is synchronous )  { 
      fs.writeSync(stream.fd,.....)
   } else {
       //execute current code
   }
}

@Fishrock123
Copy link
Contributor

cc @addaleax, @mcollina, @jasnell

@addaleax
Copy link
Member

Hmm… so I assume the problem is that the callbacks passed to .write() are kept alive until the next tick?

@mcollina
Copy link
Member

mcollina commented Mar 3, 2017

A couple of things: console.log is a debugging tool. It is not supposed to be the way to write constantly to stdout. Node.js is an asynchronous platform, and using console.log to write synchronously blocks the event loop.

I can reproduce the memory problem on all the versions of Node I checked (including v0.10) when writing to standard out on Mac OS X. This is normal and expected because we are not respecting backpressure. However, at some point we were writing synchronously when it was file, causing the process to block and slow down. IMHO we are doing the correct behavior atm, e.g. Node.js is expected to ran out of memory when using it in the way described in #11568 (comment). Probably we should explain this better.

@mcollina
Copy link
Member

mcollina commented Mar 3, 2017

@litmit a stream cannot be synchronous.

@litmit
Copy link
Author

litmit commented Mar 3, 2017

A couple of things: console.log is a debugging tool.

YES!
And just because of this, the only one available embedded debugging tool should just work anytime and anywhere! And help me to debug apps but not crash them.

It is not supposed to be the way to write constantly to stdout

Simple call of console.log in a deep loop can generate a lot of output. But this really needed for find a bug.

using console.log to write synchronously blocks the event loop.

How it can block more when as described by @Fishrock123 stdout already "is mostly synchronous"?

a stream cannot be synchronous.

You completely confusing me because this does not match with https://github.com/nodejs/node/blob/master/doc/api/process.md#a-note-on-process-io

Suggested solution:
Detect if console attached to a streams like stdout and/or stderr and this streams have synchronous behaviour. Use synchronous API (fs.writeSync) for output in this case.

In this case we can turn console to be sync/async just using process.stdout._handle.setBlocking(true/false).

@Fishrock123
Copy link
Contributor

a stream cannot be synchronous.

@mcollina stdio is weird and "violates" this expectation.

@addaleax
Copy link
Member

addaleax commented Mar 4, 2017

Streams will always do their error handling/other callback stuff using process.nextTick, though

@Trott
Copy link
Member

Trott commented Jul 30, 2017

Should this issue stay open? Or is this close-able?

@mcollina
Copy link
Member

I do not think this is fixable, not in the generic case anyway.
It might be possible, as outlined in #11568 (comment) to do a special case for stdio, and completely bypass streams. I think it will be a bit of a stretch to have different behaviors if the stdout and stderr are piped or not, but I'm happy to review a PR.

@jasnell
Copy link
Member

jasnell commented Sep 5, 2017

I'd say this is closable

@BridgeAR
Copy link
Member

Closing due to the mentioned reasons.

@manoharreddyporeddy
Copy link

manoharreddyporeddy commented Dec 2, 2017

As suggested above
I used below in my /tests/ folder (unit tests) for debugging purposes:
if (process.stdout._handle) process.stdout._handle.setBlocking(true)
It works well.
Yes, this won't go in my production code.

@CMCDragonkai
Copy link

CMCDragonkai commented Jul 18, 2022

Why is:

if (process.stdout._handle) process.stdout._handle.setBlocking(true)

Not recommended?

I know that nodejs is probably attempting to maintain legacy behaviour. But why not make stdout/stderr synchronous when stdout is interacting with a TTY.

Example:

// Default behaviour on Node.js:
// Files: synchronous on Windows and POSIX
// TTYs (Terminals): asynchronous on Windows, synchronous on POSIX
// Pipes (and sockets): synchronous on Windows, asynchronous on POSIX
// In order to align Windows with POSIX behaviour:
if (process.stdout.isTTY) {
  process.stdout._handle.setBlocking(true);
} else if (os.platform() === 'win32' && !process.stdout.isTTY) {
  process.stdout._handle.setBlocking(false);
}

tash-2s added a commit to tash-2s/open-emoji-battler that referenced this issue Sep 2, 2022
The fact that stdout could be asynchronous concerns process.exit, but it should be OK except for Windows.
nodejs/node#11568
forbesus added a commit to forbesus/OpenEmojiBattler that referenced this issue Feb 18, 2024
The fact that stdout could be asynchronous concerns process.exit, but it should be OK except for Windows.
nodejs/node#11568
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console Issues and PRs related to the console subsystem. question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

10 participants