worker: skip main thread when stdio is inherited #47036
Conversation
nodejs-github-bot
added
needs-ci
| if (directStdout) { | ||
| process.stdout._writev = function _writev(chunks, cb) { | ||
| const { chunk, encoding } = ArrayPrototypePop(chunks); | ||
| write(1, chunk, { __proto__: null }, encoding, (err) => { |
There was a problem hiding this comment.
This won't work on Windows with non-ASCII text, unfortunately
There was a problem hiding this comment.
That's a bummer. Do we know where does this limitation come from? EDIT: seems that it's explained in #24550.
There was a problem hiding this comment.
I would love to land this PR. Is this non-ASCII thing an issue that needs resolution beyond just documenting as a known limitation?
Assuming that it is, could we use whatever solution console.log itself uses for outputting to stdout for Windows?
Just to say it explicitly because the wording above might give a different impression: These statements aren't mutually exclusive and I support both of them. Fwiw, #30491 is another issue about this problem and contains a bit more context, also in terms of potential solutions (that may be hard to implement if they are intended to be correct). |
|
cc @nodejs/loaders since relevant to #44710 |
|
When Antoine and I were pairing a few weeks ago, this came up, and I suggested for Windows only, check whether the output is non-ASCII and then whether whatever Windows needs has been set; if not, warn/throw. The check itself is expensive, so other platforms shouldn't suffer for the one; but the hit on Windows seems a far better alternative that unintelligible output. |
|
FWIW I tried experimenting with Jacob's idea, but didn't get very far with it, it feels wrong to have a Windows-specific code path when the issue is not Windows specific (it's only the Windows default behavior that is weird with non-ASCII data, but there are known workaround, but AFAIK there's no way for the Node.js process to know wether such workaround is in place for the system it's running on). It's probably going to be harder to document and create weird race conditions for users to have two completely different calls depending if the code is running on a Windows or a non-Windows host. Maybe a better way of doing this would be to have an env variable to opt-in to that behavior, since the problem is environment specific. |
|
This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open. |
|
Closing this because it has stalled. Feel free to reopen if this issue/PR is still relevant, or to ping the collaborator who labelled it stalled if you have any questions. |
Currently, when the main thread is blocked, the
console.logcalls from the worker are swallowed.In #25630, @jasnell and @addaleax seems to think the current behavior is correct, but I have to side with @devsnek on this, I think we can do better. Here's my (naive) attempt at this. There are some broken tests with this PR, but I would like to hear more from @nodejs/workers before spending more time on this, in case there are good reasons to disregard this approach.
Fixes: #25630