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.

Sign up for GitHub

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

Jump to bottom

Raising SIGTERM SignalException returns wrong process exit code #5134

Closed
perlun opened this issue Apr 11, 2018 · 5 comments
Closed

Raising SIGTERM SignalException returns wrong process exit code #5134

perlun opened this issue Apr 11, 2018 · 5 comments
Milestone
JRuby 9.2.0.0

Comments

@perlun
Copy link
Contributor

perlun commented Apr 11, 2018

(This was originally spotted in puma/puma#1558, thanks @dekellum for great help there.)

With JRuby 9.1.16.0 on Java 8 (on latest macOS), I get this:

$ jruby -e 'raise SignalException, "SIGTERM"' ; echo $?
SignalException: SIGTERM
  <main> at -e:1
1

Same with JRuby 1.7 actually:

$ rvm use jruby-1.7.27
[09:59:19] plundberg@ecvaawplun6:~/git/tmp (master)
$ jruby -e 'raise SignalException, "SIGTERM"' ; echo $?
SignalException: SIGTERM
  (root) at -e:1
1

However, with MRI 2.5.1 I get this:

$ ruby -e 'raise SignalException, "SIGTERM"' ; echo $?
Terminated: 15
143

...which is indeed the correct value for a SIGTERM-triggered exit of a process.

As inspired by #5049, I tried another approach also which worked slightly differently:

$ rvm use 2.5.1
Using /Users/plundberg/.rvm/gems/ruby-2.5.1
$ ruby -e "Process.kill('TERM', Process.pid)" ; echo $?
Terminated: 15
143

Here be the surprise: JRuby behaves differently, and even differently than the raise SignalException, "SIGTERM" scenario.

$ rvm use jruby
Using /Users/plundberg/.rvm/gems/jruby-9.1.16.0
$ ruby -e "Process.kill('TERM', Process.pid)" ; echo $?
0

Exit code 0 is not entirely correct, but it's at least much better than exit code 1 which indicates failure. 😄

Inspired by this Oracle article linked to from the JRuby wiki, I tested with the -Xrs parameter:

$ rvm use jruby
Using /Users/plundberg/.rvm/gems/jruby-9.1.16.0
$ export JAVA_OPTS=-Xrs
$ ruby -e "Process.kill('TERM', Process.pid)" ; echo $?
Terminated: 15
143
$ ruby -e 'raise SignalException, "SIGTERM"' ; echo $?
SignalException: SIGTERM
  <main> at -e:1
1

So that makes the Process.kill approach work just like MRI, which is good (so we have a decent enough workaround for that.) However, the raise SignalException, 'SIGTERM' still gives the seemingly incorrect exit code.

Is this by design or is it a bug?
@headius
Copy link
Member

headius commented Apr 12, 2018

This is most likely a bug. I believe we are only looking for MainExitException to provide an error code, when we should also be checking for signal exceptions that get reported in return code.

@headius
Copy link
Member

headius commented Apr 12, 2018

Ok, I pushed a simple fix for this to master. It should be producing the right return codes for SignalException now.

@perlun I don't think there's a spec for this. Can you add one?

@headius headius added this to the JRuby 9.2.0.0 milestone Apr 12, 2018
@headius
Copy link
Member

headius commented Apr 12, 2018

Fixed for 9.2.

perlun added a commit to perlun/spec that referenced this issue Apr 12, 2018
@perlun
Process::Status#termsig: Add SignalException spec
3adc285 
We found a case in JRuby where this was not working as expected: jruby/jruby#5134

This spec describes the expected behavior in this scenario. It works on MRI, but fails on the currently released JRuby version (because of the bug, which is fixed in jruby master.)
@perlun perlun mentioned this issue Apr 12, 2018
Merged
@perlun
Copy link
Contributor Author

perlun commented Apr 12, 2018 •
edited
Loading

@headius Thank you for that! In return, I added the spec that you suggested: ruby/spec#596

(It works on MRI but fails on jruby 9.1.16.0. Haven't tested with jruby master, but running rvm install jruby-head now so will check once it's done.)

perlun added a commit to perlun/spec that referenced this issue Apr 12, 2018
@perlun
Process::Status#termsig: Add SignalException spec
6ae890d 
We found a case in JRuby where this was not working as expected: jruby/jruby#5134

This spec describes the expected behavior in this scenario. It works on MRI, but fails on the currently released JRuby version (because of the bug, which is fixed in jruby master.)
@perlun
Copy link
Contributor Author

perlun commented Apr 12, 2018

I tested now with jruby-head. Unfortunately, there still seems to be some issues:

$ ../mspec/bin/mspec /Volumes/git/3rd-party/spec/core/process/status/exitstatus_spec.rb
$ ruby /Volumes/git/3rd-party/mspec/bin/mspec-run /Volumes/git/3rd-party/spec/core/process/status/exitstatus_spec.rb
jruby 9.2.0.0-SNAPSHOT (2.5.0) 2018-04-12 34bec6e Java HotSpot(TM) 64-Bit Server VM 25.152-b16 on 1.8.0_152-b16 +jit [darwin-x86_64]

1)
Process::Status#exitstatus for a child that raised SignalException returns nil FAILED
Expected 143
 to equal nil

/Volumes/git/3rd-party/spec/core/process/status/exitstatus_spec.rb:21:in `block in (root)'
org/jruby/RubyBasicObject.java:2596:in `instance_eval'
org/jruby/RubyEnumerable.java:1737:in `all?'
org/jruby/RubyArray.java:1778:in `each'
org/jruby/RubyArray.java:1778:in `each'
/Volumes/git/3rd-party/spec/core/process/status/exitstatus_spec.rb:3:in `<main>'
org/jruby/RubyKernel.java:990:in `load'
org/jruby/RubyBasicObject.java:2596:in `instance_eval'
org/jruby/RubyArray.java:1778:in `each'
[/ | ==================100%================== | 00:00:00]      1F      0E

Finished in 5.506023 seconds

1 file, 2 examples, 2 expectations, 1 failure, 0 errors, 0 tagged
$ ../mspec/bin/mspec /Volumes/git/3rd-party/spec/core/process/status/termsig_spec.rb
$ ruby /Volumes/git/3rd-party/mspec/bin/mspec-run /Volumes/git/3rd-party/spec/core/process/status/termsig_spec.rb
jruby 9.2.0.0-SNAPSHOT (2.5.0) 2018-04-12 34bec6e Java HotSpot(TM) 64-Bit Server VM 25.152-b16 on 1.8.0_152-b16 +jit [darwin-x86_64]

1)
Process::Status#termsig for a child that raised SignalException returns the signal FAILED
Expected nil
 to equal 15

/Volumes/git/3rd-party/spec/core/process/status/termsig_spec.rb:23:in `block in (root)'
org/jruby/RubyBasicObject.java:2596:in `instance_eval'
org/jruby/RubyEnumerable.java:1737:in `all?'
org/jruby/RubyArray.java:1778:in `each'
org/jruby/RubyArray.java:1778:in `each'
/Volumes/git/3rd-party/spec/core/process/status/termsig_spec.rb:3:in `<main>'
org/jruby/RubyKernel.java:990:in `load'
org/jruby/RubyBasicObject.java:2596:in `instance_eval'
org/jruby/RubyArray.java:1778:in `each'
[- | ==================100%================== | 00:00:00]      1F      0E

Finished in 5.705038 seconds

1 file, 3 examples, 3 expectations, 1 failure, 0 errors, 0 tagged

These issues are closely related, but they probably are in the Process::Status class. Do you want me to file another issue about it or can we reopen this one?

@perlun perlun mentioned this issue Apr 13, 2018
Closed
@nateberkopec nateberkopec mentioned this issue May 9, 2018
Closed
@perlun perlun mentioned this issue Jun 21, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
JRuby 9.2.0.0
Development

No branches or pull requests
2 participants
@headius @perlun