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

qjs fails to output content via stdout to pipe or non-console receiver #635

Closed
harig opened this issue Oct 28, 2024 · 2 comments · Fixed by #642
Closed

qjs fails to output content via stdout to pipe or non-console receiver #635

harig opened this issue Oct 28, 2024 · 2 comments · Fixed by #642
Labels
bug Something isn't working

Comments

@harig
Copy link

harig commented Oct 28, 2024

After 72d4587 content emitted by qjs (via console.log/print etc. ) to stdout fails to reach a receiver through a pipe or other process reading from its stdout on Windows 10.
For example, if we run the following inside cmd.exe
qjs -e "print(1);"
will print 1
qjs -e "print(1);" | cat
will not output anything.
Similar issue can be seen with qjs launched as a process and some other program reads its output.

@bnoordhuis
Copy link
Contributor

Yeah, I'm not too surprised that broke. Pull request welcome.

You can use something like this to detect whether stdout refers to the console:

#ifdef _WIN32
int isatty(int fd) { // if (isatty(1)) { /* ... */ } else { /* ... */ }
   DWORD mode;
   HANDLE handle = (HANDLE)_get_osfhandle(fd);
   if (GetFileType(handle) == FILE_TYPE_CHAR)
       if (GetConsoleMode(handle, &mode))
           return 1;
   return 0;
}
#endif

There's one very annoying gotcha though: _get_osfhandle() asserts in msvcrt debug builds if the file descriptor is invalid. You can suppress it with _CrtSetReportHook but it's super kludgy and libraries shouldn't touch that.

Hooking it in qjs.c is fine but not in quickjs-libc.c.

@bnoordhuis bnoordhuis added the bug Something isn't working label Oct 28, 2024
bnoordhuis added a commit to bnoordhuis/quickjs that referenced this issue Oct 30, 2024
Use regular libc stdio (fwrite) when stdout is redirected, don't
call WriteConsoleA because that circumvents the redirection.

Fixes: quickjs-ng#635
@bnoordhuis
Copy link
Contributor

@harig can you test #642 and report back?

bnoordhuis added a commit to bnoordhuis/quickjs that referenced this issue Oct 31, 2024
Use regular libc stdio (fwrite) when stdout is redirected, don't
call WriteConsoleA because that circumvents the redirection.

Fixes: quickjs-ng#635
bnoordhuis added a commit to bnoordhuis/quickjs that referenced this issue Oct 31, 2024
Use regular libc stdio (fwrite) when stdout is redirected, don't
call WriteConsoleA because that circumvents the redirection.

Fixes: quickjs-ng#635
bnoordhuis added a commit that referenced this issue Oct 31, 2024
Use regular libc stdio (fwrite) when stdout is redirected, don't
call WriteConsoleA because that circumvents the redirection.

Fixes: #635
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants