Unexpected value for `$?.exitstatus` when process exited successfully #4361

Closed
javierhonduco opened this Issue Dec 6, 2016 · 4 comments

Projects

None yet

4 participants

@javierhonduco
Contributor

Environment

  • jruby 9.1.5.0 (2.3.1) 2016-09-07 036ce39 Java HotSpot(TM) 64-Bit Server VM 24.65-b04 on 1.7.0_67-b01 +jit [darwin-x86_64]
  • Darwin 16.1.0 Darwin Kernel Version 16.1.0: Wed Oct 19 20:31:56 PDT 2016; root:xnu-3789.21.4~4/RELEASE_X86_64 x86_64

There's nothing special (gems or whatsoever) running on this machine.

Expected Behavior

When running this snippet of code:

system "exit 0"
puts "ruby_success=#{$?.success?} exit_code=#{$?.exitstatus}"

MRI (2.3.1) outputs
ruby_success=true exit_code=0

Actual Behavior

Unfortunately this is the output I'm getting under the JRuby version mentioned above:

ruby_success=false exit_code=127

I don't know if this is expected as per JRuby system's implementation, sorry if that's the case!
Thanks,

@olleolleolle
Contributor

Same on 9.1.6.0.

$ rvm jruby-9.1.6.0 do ruby -e 'system "exit 0"; puts "ruby_success=#{$?.success?} exit_code=#{$?.exitstatus}"'
ruby_success=false exit_code=127
@headius
Member
headius commented Dec 6, 2016

Looks like we're not doing the right masking of the exit status.

@headius
Member
headius commented Dec 6, 2016

Ok, so here's the problem. We do the correct masking; however in this case the system call is actually failing. It tries to find an executable exit command, and seeing none it bails out with exit code of 127. MRI must be doing some extra logic here to know it needs to run with sh -c.

@headius headius added a commit that closed this issue Dec 6, 2016
@headius headius Appropriate short-circuit for shell commands. Fixes #4361.
The original logic was a bad port of the following C code:

```c
static const char posix_sh_cmds[][9] = {
...
}
if (first.len > 0 && first.len <= sizeof(posix_sh_cmds[0]) &&
...
```

In C, the array allocation sets all elements to be sizeof = 9,
even though most terminate via null at a shorter C string length.
The check here is to filter out any commands longer than those
known to be shell commands; my fix is to just have that max length
be a constant and reference it where sizeof is used in MRI.
69da0af
@headius headius closed this in 69da0af Dec 6, 2016
@enebo enebo added this to the JRuby 9.1.7.0 milestone Dec 6, 2016
@javierhonduco
Contributor
javierhonduco commented Dec 6, 2016 edited

Thanks a lot guys! You were super-fast!

💚

@javierhonduco javierhonduco added a commit to javierhonduco/jruby that referenced this issue Dec 6, 2016
@javierhonduco javierhonduco Add regression test for shell exit
Adds a regression test for 69da0af reported in this issue #4361.
fb5647e
@kares kares added a commit that referenced this issue Dec 7, 2016
@javierhonduco @kares javierhonduco + kares Add regression test for shell exit (#4364)
* Add regression test for shell exit (#4361)

Adds a regression test for 69da0af reported in this issue #4361.

* Update test_kernel.rb
587bb74
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment