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

Question: embedding node in non-console application #1251

Closed
Let0s opened this issue May 4, 2018 · 13 comments
Closed

Question: embedding node in non-console application #1251

Let0s opened this issue May 4, 2018 · 13 comments

Comments

@Let0s
Copy link

Let0s commented May 4, 2018

  • Node.js Version: 8.x
  • OS: MS Windows 7
  • Scope (install, code, runtime, meta, other?): embedding

I try to embed node into non-console application. With console application it works fine. But with non-console application node doesn't evaluate script. debugger output writes next: internal/process/stdio.js:4433: Uncaught Unknown stream file type.
I think it is realted with tty_wrap.guessHandleType(fd) call in lib\internal\process\stdio.js file and GuessHandleType function in tty_wrap.cc file where t variable have UV_UNKNOWN_HANDLE value in my case. And it seems, that there is no any supported input/output in non-console application and node stops script execution.
I need an advice - maybe there is any way to change input/output to what i need or block it?

@Let0s
Copy link
Author

Let0s commented May 4, 2018

Update: if i change source file tty_wrap.cc and will return "FILE" instead of "UNKNOWN" in UV_UNKNOWN_HANDLE case, all works fine. But it is bad solution and I want not to change node source files.

@gireeshpunathil
Copy link
Member

Can you see if you can follow what uv_spawn does when child_process.spawn with stdio: ignore option is supplied, and mimic that behavior prior to launching the node dll and see if it meets your requirement?

Also /cc @bnoordhuis for better insights
Also /cc @addaleax who has been leading the embedder's use cases.
Also link #1031

@addaleax
Copy link
Member

addaleax commented May 5, 2018

@Let0s We’ve had bug reports into that direction in the past. Not knowing much about Windows, do you know what value GetFileType() inside uv_guess_handle() returns and to which stream concept, if any, that should match to?

@Let0s
Copy link
Author

Let0s commented May 7, 2018

@gireeshpunathil Thanks for advice.
@addaleax In my case GetFileType() returns 0 and handle value is 0xfffffffe - it looks like handle doesn't exist.
Does it mean, that I should create stdio from my app and node will find it and connect to it automatically?

@gireeshpunathil
Copy link
Member

AFAIK, Non-console mode windows apps never opens standard streams. So handles for fds 0,1,2 will all be garbage.

@Let0s
Copy link
Author

Let0s commented May 7, 2018

OK. Thank you, I'll try it.

@Let0s Let0s closed this as completed May 7, 2018
@addaleax
Copy link
Member

addaleax commented May 7, 2018

It still seems like a valid question to ask what Node.js would do in this case. I think it might make sense to provide dummy stdio streams?

@gireeshpunathil
Copy link
Member

@addaleax - yes, agreed on the dummy stdio streams (or at least a valid destination for them so that Node does not crash) and hence my suggestion on uv_spawn with ignore - isn't it aims to achieve the same thing?

@addaleax
Copy link
Member

addaleax commented May 7, 2018

@gireeshpunathil On UNIX systems we get fds 0,1,2 that each refer to /dev/null (reading immediately stops, writing is a no-op). I could imaginge uv_spawn with ignore does something similar on Windows?

@gireeshpunathil
Copy link
Member

If I understand correctly, [this sequence] (https://github.com/libuv/libuv/blob/d9d207774ea1f734310982300e55dd64b3f5682b/src/win/process-stdio.c#L290-L293) initializes all the handles with invalid (though not garbage) values, and later fills in the proper values according to user options. We could actually invalidate the host process's handles in this way prior to launching Node and see how does it behave. @Let0s

@Let0s
Copy link
Author

Let0s commented May 8, 2018

@gireeshpunathil in my case (embedding without stdio) uv__stdio_create is not calling. I think it is because node can't find any stdio, and tty_wrap.guessHandleType(fd) returns 'UNKNOWN' and this line stops execution.
About my question: I created pipe in my app, then dynamically call node.dll and it works - node got pipe's handle. But even there uv__stdio_create is not calling.
And i found that uv__stdio_create is used only in uv_spawn function and I know only one way to call it - it is child_process.spawn function call inside already running script.
P.S. I test it all on node v8.x, maybe something was changed in later versions.

@gireeshpunathil
Copy link
Member

@Let0s - ok, I did not mean that the routine will be invoked in the regular sequence. Instead, I suggested to follow those steps to make your standard streams to be similar to those, in the embedder itself, prior to the node.dll load. (closing the fds if they are already opened, and assigning their os_handles with INVALID_HANDLE_VALUE

@Let0s
Copy link
Author

Let0s commented May 8, 2018

@gireeshpunathil Maybe I misunderstood you, but I think it is almost impossible. In case with child_process.spawn node creates handles even for 'ignore' option. This comment tells that std(in/out/err) should be initialized, but others can be INVALID_HANDLE_VALUE.
My problem is solved - if I haven't stdio streams in my app, I create them before node initialization.
If you want to solve it globally, as @addaleax said:

It still seems like a valid question to ask what Node.js would do in this case. I think it might make sense to provide dummy stdio streams?

Then I think, Node.js should check (on initialization?) if stdio exists and if it is not exist, Node.js should create dummy stdio streams.

addaleax added a commit to addaleax/node that referenced this issue Dec 4, 2018
The only known condition where we could not provide appropriate
stdio streams so far were non-console Windows applications.
Since this issue has come up a few times in our issue tracker now,
switch to providing dummy streams for these cases instead.

If there are other valid cases in which `uv_guess_handle` fails,
and where there is a more sensible way to provide stdio,
we’ll probably still find out because the streams don’t work
properly either way.

Refs: nodejs/help#1251
Trott pushed a commit to Trott/io.js that referenced this issue Dec 4, 2018
The only known condition where we could not provide appropriate
stdio streams so far were non-console Windows applications.
Since this issue has come up a few times in our issue tracker now,
switch to providing dummy streams for these cases instead.

If there are other valid cases in which `uv_guess_handle` fails,
and where there is a more sensible way to provide stdio,
we’ll probably still find out because the streams don’t work
properly either way.

Refs: nodejs/help#1251

PR-URL: nodejs#20640
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BridgeAR pushed a commit to nodejs/node that referenced this issue Dec 5, 2018
The only known condition where we could not provide appropriate
stdio streams so far were non-console Windows applications.
Since this issue has come up a few times in our issue tracker now,
switch to providing dummy streams for these cases instead.

If there are other valid cases in which `uv_guess_handle` fails,
and where there is a more sensible way to provide stdio,
we’ll probably still find out because the streams don’t work
properly either way.

Refs: nodejs/help#1251

PR-URL: #20640
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax added a commit to addaleax/node that referenced this issue Jan 5, 2019
The only known condition where we could not provide appropriate
stdio streams so far were non-console Windows applications.
Since this issue has come up a few times in our issue tracker now,
switch to providing dummy streams for these cases instead.

If there are other valid cases in which `uv_guess_handle` fails,
and where there is a more sensible way to provide stdio,
we’ll probably still find out because the streams don’t work
properly either way.

Refs: nodejs/help#1251

PR-URL: nodejs#20640
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BridgeAR pushed a commit to nodejs/node that referenced this issue Jan 10, 2019
The only known condition where we could not provide appropriate
stdio streams so far were non-console Windows applications.
Since this issue has come up a few times in our issue tracker now,
switch to providing dummy streams for these cases instead.

If there are other valid cases in which `uv_guess_handle` fails,
and where there is a more sensible way to provide stdio,
we’ll probably still find out because the streams don’t work
properly either way.

Refs: nodejs/help#1251

PR-URL: #20640
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax added a commit to nodejs/node that referenced this issue Jan 14, 2019
The only known condition where we could not provide appropriate
stdio streams so far were non-console Windows applications.
Since this issue has come up a few times in our issue tracker now,
switch to providing dummy streams for these cases instead.

If there are other valid cases in which `uv_guess_handle` fails,
and where there is a more sensible way to provide stdio,
we’ll probably still find out because the streams don’t work
properly either way.

Refs: nodejs/help#1251

PR-URL: #20640
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
refack pushed a commit to refack/node that referenced this issue Jan 14, 2019
The only known condition where we could not provide appropriate
stdio streams so far were non-console Windows applications.
Since this issue has come up a few times in our issue tracker now,
switch to providing dummy streams for these cases instead.

If there are other valid cases in which `uv_guess_handle` fails,
and where there is a more sensible way to provide stdio,
we’ll probably still find out because the streams don’t work
properly either way.

Refs: nodejs/help#1251

PR-URL: nodejs#20640
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this issue Jan 16, 2019
The only known condition where we could not provide appropriate
stdio streams so far were non-console Windows applications.
Since this issue has come up a few times in our issue tracker now,
switch to providing dummy streams for these cases instead.

If there are other valid cases in which `uv_guess_handle` fails,
and where there is a more sensible way to provide stdio,
we’ll probably still find out because the streams don’t work
properly either way.

Refs: nodejs/help#1251

PR-URL: nodejs#20640
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants