java.lang.ArrayIndexOutOfBoundsException in 9.2.13.0 #6466
Comments
|
Ok I think this is a bug in how we handle exceptions in the thread spawned by jruby/core/src/main/java/org/jruby/RubyProcess.java Lines 1446 to 1455 in 8a9e777 Raising an exception from this thread is fine, except when we have tracing turned on (via Running this without tracing should work fine as a workaround, but I assume you have it on for a reason in these specs. On the JRuby side, moving the wait thread into Ruby would be a reasonable fix, but we should also (or should instead) fix the |
|
Trivial reproduction based on my theory: |
|
It appears that MRI does not raise errors from the detach thread. Need to research a bit more to see how they propagate the errno if one occurs. |
In jruby#6466 we see that the thread from a Process::detach is raising an error when the waitpid fails with an error. This does not appear to match the logic in MRI and also breaks if there is a "raise" hook installed in the system. This change should better match MRI logic for the detach thread and avoid the hook issue reported in jruby#6466. It does not address a separate bug in the trace hook logic when no Ruby frames are available.
|
I have pushed #6467 to address the detach bug. The fragility in the "raise" hook remains. @AlexWayfer You may be able to work around this locally by replacing our detach logic with a pure-Ruby version. See the PR and basically do that but in Ruby, something like this untested code: class << Process
def detach(pid)
Thread.new { while waitpid(pid) == 0; end; $? }
end
end |
What? Where? I have no
I'm not sure, but I see terminology confusion: do you mean "process" and not "thread"?
Now I'm getting (locally) this: I don't know, why is something like this here at all while nothing like this with MRI. |
Not backtrace – Ruby debugging trace hooks, like
Ok, my untested code is probably not quite right and might need some adjustment. It does seem as though the pid passed to this |
|
A more elaborate patch that may help. I still believe you have a process dying prematurely during those specs, though. require 'ffi'
class << Process
module Waitpid
extend FFI::Library
ffi_lib 'c'
attach_function :waitpid, [:long, :int], :long
end
def detach(pid)
Thread.new { while (ret = Waitpid.waitpid(pid, 0)) == 0; end; ret }
end
end |
|
Note that require 'ffi'
require 'jruby'
class << Process
module Waitpid
extend FFI::Library
ffi_lib 'c'
attach_function :waitpid, [:long, :int], :int
end
def detach(pid)
Thread.new do
while (ret = Waitpid.waitpid(pid, 0)) == 0; end
if ret == -1
nil
else
org.jruby.RubyProcess::RubyStatus.newProcessStatus(JRuby.runtime, ret, pid)
end
end
end
end |
I mean… "but I assume you have it on for a reason in these specs" is wrong for me — I didn't make something special for any reason.
Sorry, I don't understand completely: I have OK, I've added But why is it not here with MRI?
OK, with this code (patch, work-around) output is cleaner: So, I guess, such code should be in the core of JRuby to match MRI behavior. |
The PR in #6467 will fix the base issue with I also see now where the trace hook is happening. From your output this line indicates that you are running coverage in these spec runs, which uses the hook mechanism:
There is nothing wrong with this... I just point it out to show how this bug is being exposed. And it is indeed a bug here, which will be fixed by #6467. |
|
If you like we have snapshot builds of 9.2.14.0 at https://oss.sonatype.org/content/repositories/snapshots/org/jruby/jruby-dist/9.2.14.0-SNAPSHOT/ |
|
Hello! I just run into this same error here. But I'm using 9.2.14.0 there, so maybe there's another edge case not yet fixed? In that failing test I'm using I'm planning to not run tests with the |
|
@deivid-rodriguez Hmm that does look like another manifestation of the same issue.
I think it is appropriate to reopen this since it was not completely fixed. |
I don't think that will prevent this since even in non- |
|
@deivid-rodriguez I would appreciate a reproduction for this. I am not sure how to trigger it since the logic in I have a possible fix pending that I will do as a PR, so we can get a repro/test together before merging it. |
|
Hei @headius, not sure how to repro this, I only saw it once on a CI log. Actually the link I posted before is wrong, the log is this one: https://github.com/rubygems/rubygems/runs/1806426748?check_suite_focus=true. |
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.
|
@deivid-rodriguez Ok, I have pushed #6546 to fix this but I will have to think on how to test it. |
|
I have a way to reproduce: detach a non-existent pid. I also realized that even with my |
* 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.
|
@deivid-rodriguez FWIW you are probably seeing this because some library is causing a "raise" event hook to get installed, usually via |
|
You're guys awesome! |
Environment Information
Provide at least:
jruby 9.2.13.0 (2.5.7) 2020-08-03 9a89c94bcc OpenJDK 64-Bit Server VM 25.275-b01 on 1.8.0_275-b01 +jit [linux-x86_64]5.9.8-arch1-1)jruby 9.2.13.0 (2.5.7) 2020-08-03 9a89c94bcc OpenJDK 64-Bit Server VM 14.0.2+12 on 14.0.2+12 +jit [linux-x86_64]Other relevant info you may wish to add:
filewatcher-cliproject.Expected Behavior
No errors.
Actual Behavior
In Cirrus CI:
https://cirrus-ci.com/task/5614335160483840?command=test#L63
On Arch Linux:
The text was updated successfully, but these errors were encountered: