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

Handle errors in Process.detach properly #6546

Merged
merged 2 commits into from Feb 8, 2021

Conversation

headius
Copy link
Member

@headius headius commented Feb 1, 2021

The rb_waitpid function in CRuby does not actually trigger an
exception raise when the waitpid errors, leaving that to callers
to decide. Because ours did so, any errors during the
Process.detach call to waitpid would trigger the raise event hook
and unfix #6466.

This change moves the errno check outside of our rb_waitpid
equivalent, making it safe to use in the Process.detach thread.

Fixes #6466 again.

@headius headius added this to the JRuby 9.2.15.0 milestone Feb 1, 2021
The rb-waitpid function in CRuby does not actually trigger an
exception raise when the waitpid errors, leaving that to callers
to decide. Because ours did so, any errors during the
Process.detach call to waitpid would trigger the raise event hook
and unfix jruby#6466.

This change moves the errno check outside of our rb_waitpid
equivalent, making it safe to use in the Process.detach thread.

Fixes jruby#6466 again.
* Propagate long pid.
* Push a dummy frame because of interrupts and thread events being
  able to raise an error during waitpid.

Additional fix for jruby#6466.
@headius
Copy link
Member Author

headius commented Feb 1, 2021

An additional change was needed here because even with the previous commit errors may still get raised out of the blocking waitpid call if it is interrupted by another thread. Since the detach thread must be able to interact as a full Ruby thread, we modify this logic to push a dummy frame.

@headius
Copy link
Member Author

headius commented Feb 1, 2021

The reproduction case is complicated and I am unsure how best to add a spec for it. Basically, the reduced case looks like this:

$ rvm jruby-9.2.14.0 do ruby -rtracer -e "Process.detach(12345).join"
/Users/headius/.rvm/rubies/jruby-9.2.14.0/lib/ruby/stdlib/tracer.rb:134: warning: tracing (e.g. set_trace_func) will not capture all events without --debug flag
warning: thread "Ruby-0-Thread-1: -e:1" terminated with exception (report_on_exception is true):
java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 10
	at org.jruby.dist/org.jruby.runtime.ThreadContext.getCurrentFrame(ThreadContext.java:541)

where -rtracer could be replaced by any logic that uses set_trace_func or TracePoint to trace raise events.

@headius headius changed the title Do not raise within rb_waitpid equivalent Handle errors in Process.detach properly Feb 1, 2021
@headius headius linked an issue Feb 1, 2021 that may be closed by this pull request
@headius headius merged commit 8dc89fe into jruby:jruby-9.2 Feb 8, 2021
@headius headius deleted the waitpid_no_exception branch February 8, 2021 15:59
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

Successfully merging this pull request may close these issues.

java.lang.ArrayIndexOutOfBoundsException in 9.2.13.0
1 participant