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

Implement Thread#report_on_exception #3937

Closed

Conversation

Projects
None yet
4 participants
@headius
Copy link
Member

headius commented May 28, 2016

This impl is a work in progress and has not been finalized yet.

It works as follows:

  • nil = report when thread is finalized 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.
  • Default global value is nil.
  • 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?

See the Ruby feature here: https://bugs.ruby-lang.org/issues/6647

headius added some commits May 28, 2016

Implement CharSequence in RubyString.
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.
Implement Thread{.,#}report_on_exception[=].
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?

@headius headius added this to the JRuby 9.2.0.0 milestone May 28, 2016

@headius

This comment has been minimized.

Copy link
Member Author

headius commented May 28, 2016

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 length method. All internal uses of that method have been fixed, but since it was public there may be Java code in the wild that will break as a result.

@headius

This comment has been minimized.

Copy link
Member Author

headius commented May 28, 2016

Output from a few interesting scenarios. I use gets here to pause for a while to let the thread finish without using join.

[] ~/projects/jruby $ jruby -e "Thread.report_on_exception = true; def foo; Thread.new { raise }; nil; end; foo; puts 'pausing'; gets"
pausing
warning: thread "Ruby-0-Thread-1: -e:1" terminated with exception:
RuntimeError: No current exception
  block in foo at -e:1


[] ~/projects/jruby $ jruby -e "Thread.report_on_exception = true; def foo; Thread.new { raise }.name = 'hello'; nil; end; foo; puts 'pausing'; gets"
pausing
warning: thread "hello (Ruby-0-Thread-1: -e:1)" terminated with exception:
RuntimeError: No current exception
  block in foo at -e:1


[] ~/projects/jruby $ jruby -e "Thread.report_on_exception = nil; def foo; Thread.new { raise }; nil; end; foo; puts 'pausin'; gets; puts :ok; loop { JRuby.gc; sleep 1 }"
pausing

ok
warning: thread "Ruby-0-Thread-1: -e:1" terminated with exception:
RuntimeError: No current exception
  block in foo at -e:1
^C
[] ~/projects/jruby $ jruby -e "Thread.report_on_exception = nil; def foo; Thread.new { raise }.join rescue nil; end; foo; puts :ok; loop { JRuby.gc; sleep 1 }"
ok
^C
@kares

This comment has been minimized.

Copy link
Member

kares commented May 28, 2016

👍 yay RubyString implements CharSequence now if only RubyNumeric extended java.lang.Number :)

... will miss the Ruby thread name showing up in Java's thread-name (e.g. VisualVM) does it need to go?

@eregon

This comment has been minimized.

Copy link
Member

eregon commented May 30, 2016

Could we try with the default being true?
I know it's currently problematic for MRI tests and maybe ruby/spec, but I still think it would be much better for user programs/applications.

@headius

This comment has been minimized.

Copy link
Member Author

headius commented May 31, 2016

@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.

@headius

This comment has been minimized.

Copy link
Member Author

headius commented May 31, 2016

@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;

This comment has been minimized.

Copy link
@kares

kares Jun 2, 2016

Member

str.length() should do (no need to convert a RubyString to a Java String here)

}
newName = name.toString();

thread.setName(newName);

This comment has been minimized.

Copy link
@kares

kares Jun 2, 2016

Member

should catch (SecurityException ignore) { } // current thread can not modify like before (an bellow)

}
} else {
if (nativeName.equals("")) {
return rubyName.toString();

This comment has been minimized.

Copy link
@kares

kares Jun 2, 2016

Member

would add '@' + rubyName as well as bellow (so that we're consistent to have @ prefix for Ruby thread-names)

@headius

This comment has been minimized.

Copy link
Member Author

headius commented Jun 28, 2016

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.

@eregon

This comment has been minimized.

Copy link
Member

eregon commented Jul 26, 2016

@eregon We certainly can...do you just want to see what happens if we run like that for a full CI?

Yeah, that would be interesting :)

kares added a commit to kares/jruby that referenced this pull request May 21, 2018

@enebo enebo modified the milestones: JRuby 9.2.0.0, JRuby 9.2.1.0 May 24, 2018

@headius

This comment has been minimized.

Copy link
Member Author

headius commented Oct 11, 2018

This has been superseded by e408638.

@headius headius closed this Oct 11, 2018

@headius headius deleted the headius:thread_report_and_string_charsequence branch Oct 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.