-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Implement Thread#report_on_exception #3937
Implement Thread#report_on_exception #3937
Conversation
There's an important breaking change here: the `length` method had only its return value changed, from RubyFixnum to int. We only called this method from within RubyString itself, but it was public and could be called by third-party Java code that would now break at runtime and fail to compile. Ruby code calling `length` will continue to work properly, since the new int return value will just coerce into a RubyFixnum.
This impl works as follows: * Default global value is nil. * nil = report when thread is GC if nobody has captured the exception (i.e. called #join or #value or abort_on_exception logic has fired). * true = report when the thread terminates, regardless of capture. * false = never report. * New threads inherit the current global setting. * If a thread name has been set from Ruby, it will be combined with JRuby's internal name for the report. If it has not been set, we will just report the internal thread name. There are some open questions for this feature: * If join/value are interrupted, should they still set the capture bit? My impl does not; they must complete. * Is the VERBOSE-like nil/true/false clear enough or should we use symbols like :off, :gc, :on?
NOTE there's a big possibly-breaking change in here: in order to implement CharSequence in RubyString, I had to change the return value of the |
Output from a few interesting scenarios. I use
|
👍 yay ... will miss the Ruby thread name showing up in Java's thread-name (e.g. VisualVM) does it need to go? |
Could we try with the default being |
@kares It doesn't really need to be gone but I'd need to manage three separate names (ruby thread name, java thread name, thread's combined name) to be able to easily recombine them later. I'm open to suggestions on how the three should fit together. |
@eregon We certainly can...do you just want to see what happens if we run like that for a full CI? |
@@ -1115,6 +1103,10 @@ public synchronized IRubyObject inspect() { | |||
return getRuntime().newString(part.toString()); | |||
} | |||
|
|||
private boolean notEmpty(CharSequence str) { | |||
return str != null && str.toString().length() > 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
str.length()
should do (no need to convert a RubyString
to a Java String
here)
FWIW a limited version of this was added to MRI in the related feature linked above. They just went with a simple on/off setup for now, defaulting to off. |
Yeah, that would be interesting :) |
This has been superseded by e408638. |
This impl is a work in progress and has not been finalized yet.
It works as follows:
exception (i.e. called #join or #value or abort_on_exception
logic has fired).
with JRuby's internal name for the report. If it has not been
set, we will just report the internal thread name.
There are some open questions for this feature:
bit? My impl does not; they must complete.
symbols like :off, :gc, :on?
See the Ruby feature here: https://bugs.ruby-lang.org/issues/6647