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

mruby-io doesn't work in process started as Windows service #5114

Closed
kou opened this issue Nov 5, 2020 · 7 comments
Closed

mruby-io doesn't work in process started as Windows service #5114

kou opened this issue Nov 5, 2020 · 7 comments

Comments

@kou
Copy link
Contributor

kou commented Nov 5, 2020

#4968 added file descriptor check in IO#initialize.
It raises an exception at IO.open(0, "r") https://github.com/mruby/mruby/blob/master/mrbgems/mruby-io/mrblib/io.rb#L363 when a process is started as Windows service. Because a process started as Windows service doesn't have a valid standard input. fstat(0, buf) returns non-zero.

Here are solutions I think:

  1. Don't check file descriptor for 0, 1 and 2.
  2. Open an internal pipe for 0, 1 and 2 when they are invalid.

Which solution do you like?

I think that 1. is enough.

FYI: CRuby uses 2.: fill_standard_fds() https://github.com/ruby/ruby/blob/master/ruby.c#L2556-L2584

@matz
Copy link
Member

matz commented Nov 6, 2020

I agree that (1) is sufficient. Could you prepare a pull-request? Or shall I work on it?

kou added a commit to kou/mruby that referenced this issue Nov 6, 2020
We don't need to require valid STDIN/STDOUT/STDERR. If we require, we
can't use mruby environment that doesn't have valid
STDIN/STDOUT/STDERR such as Windows service process. Windows service
process doesn't have valid STDIN.
@kou
Copy link
Contributor Author

kou commented Nov 6, 2020

OK. I'll do it.

kou added a commit to kou/mruby that referenced this issue Nov 6, 2020
We don't need to require valid STDIN/STDOUT/STDERR. If we require, we
can't use mruby environment that doesn't have valid
STDIN/STDOUT/STDERR such as Windows service process. Windows service
process doesn't have valid STDIN.
@matz
Copy link
Member

matz commented Nov 7, 2020

Hmm. Are you going to make a pull-request from the above commit? Or shall I merge it?

@kou
Copy link
Contributor Author

kou commented Nov 9, 2020

Sorry for not sharing the status of the commit.
I don't have an environment that can reproduce this problem. I need to ask our customer to confirm the commit fixes this problem.
It'll be done in a few days.

But the patch is simple and it'll fix this problem. So we can merge it without waiting the confirmation. (I'll share the result of the confirmation later.)
I've opened #5119.

kou added a commit to kou/mruby that referenced this issue Nov 9, 2020
We don't need to require valid STDIN/STDOUT/STDERR. If we require it,
we can't use mruby on an environment that doesn't have valid
STDIN/STDOUT/STDERR such as Windows service process. Windows service
process doesn't have valid STDIN.
kou added a commit to kou/mruby that referenced this issue Nov 9, 2020
We don't need to require valid STDIN/STDOUT/STDERR. If we require it,
we can't use mruby on an environment that doesn't have valid
STDIN/STDOUT/STDERR such as Windows service process. Windows service
process doesn't have valid STDIN.
matz added a commit that referenced this issue Nov 9, 2020
Don't check FD for STDIN/STDOUT/STDERR; #5114
@kou
Copy link
Contributor Author

kou commented Nov 10, 2020

We've confirmed that #5119 fixes this problem.

@kou kou closed this as completed Nov 10, 2020
@matz
Copy link
Member

matz commented Nov 10, 2020

Does this fix migw crash issue?

@kou
Copy link
Contributor Author

kou commented Nov 10, 2020

I think that this is unrelated to the MinGW crash issue.
But I don't know why the issue was occurred and fixed...

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

2 participants