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

Better testing for finalization #935

Open
headius opened this issue Aug 4, 2022 · 2 comments
Open

Better testing for finalization #935

headius opened this issue Aug 4, 2022 · 2 comments

Comments

@headius
Copy link
Contributor

headius commented Aug 4, 2022

While attempting to write a spec for jruby/jruby#7267 I ran into various issues and questions...

  1. There are no specs testing that GC eventually finalizes objects. This is obviously difficult to predict, but it is behavior I believe we should be testing one way or another.
  2. I was testing that exceptions in one finalizer are not seen by the next finalizer, but could not figure out a way to eliminate the warning output from Ruby indicating that a finalizer raised an exception.

The spec I attempted is below, but only makes a best attempt at forcing GC-oriented finalization and still does not suppress the error output.

  it "hides raised exceptions from one finalizer to the next" do
    def scoped(result)
        Proc.new { result << "ok" if $!.nil?; raise }
    end
    def test(result)
      obj = "Test"
      # finalizer order may vary so both handlers check $! and raise an error
      ObjectSpace.define_finalizer(obj, scoped(result))
      ObjectSpace.define_finalizer(obj, scoped(result))
    end

    result = []
    begin
      old_verbose, $VERBOSE = $VERBOSE, false
      test(result)
    ensure
      $VERBOSE = old_verbose
    end

    100.times { GC.start; break if result.size == 2 }

    result.should == ["ok", "ok"]
  end

I don't want to leave the fix for jruby/jruby#7267 untested, but I'm unsure how we should move forward to improve the GC-triggered finalization specs.

headius added a commit to headius/jruby that referenced this issue Aug 4, 2022
In jruby#7267 we had a report of endless exception cause processing
that turned out to be triggered by a bad finalizer (that allowed
an exception to bubble out) stacking up causes from previous calls
of that finalizer.

The fix here mimics what CRuby does: where they reset the errinfo
to what it was prior to the finalizer running (because CRuby's GC
often/usually runs on the current user thread), we simply clear it
after each finalizer has run (because the JDK runs finalizers on
a separate thread, as will our future non-JVM-finalizer version of
this logic).

No spec is provided yet due to the difficulty of testing
GC-triggered events across VMs. See ruby/spec#935
for more details.

Fixes jruby#7267
@eregon
Copy link
Member

eregon commented Aug 6, 2022

We've tried in the past and it seems near-impossible to have a reliable test for this unfortunately.
PR welcome of course if you find one way.

GC in a loop is also something I tried but it's 1) very slow 2) still doesn't finalize sometimes.
Also on CRuby because of the conservative GC it's completely possible some object never gets finalized even though it is semantically unreachable.

My take is: no one should rely on finalizers on Ruby, they run unreliably (on all Ruby impls) and always late. Releasing resources should always be done promptly with an ensure or block pattern .

See

# Why do we not test that finalizers are run by the GC? The documentation
# says that finalizers are never guaranteed to be run, so we can't
# spec that they are. On some implementations of Ruby the finalizers may
# run asynchronously, meaning that we can't predict when they'll run,
# even if they were guaranteed to do so. Even on MRI finalizers can be
# very unpredictable, due to conservative stack scanning and references
# left in unused memory.

TruffleRuby also tried using TruffleRuby-specific methods but even that spuriously fails:
https://github.com/oracle/truffleruby/blob/4054469bbd985db7dc5c86299004ff86a2835baf/spec/truffle/objspace/define_finalizer_spec.rb#L17-L27

@eregon
Copy link
Member

eregon commented Aug 6, 2022

One possibility might be to rely on the fact CRuby runs finalizers on exit, and use that as a way of testing finalizers aspects other than "they happen when the object is GC'd".
However, IIRC that's actually tricky to implement as there might be ordering rules between objects (based on what they reference) and which object should finalize first. I'm not sure how CRuby deals with that.

headius added a commit to headius/jruby that referenced this issue Aug 9, 2022
In jruby#7267 we had a report of endless exception cause processing
that turned out to be triggered by a bad finalizer (that allowed
an exception to bubble out) stacking up causes from previous calls
of that finalizer.

The fix here mimics what CRuby does: where they reset the errinfo
to what it was prior to the finalizer running (because CRuby's GC
often/usually runs on the current user thread), we simply clear it
after each finalizer has run (because the JDK runs finalizers on
a separate thread, as will our future non-JVM-finalizer version of
this logic).

No spec is provided yet due to the difficulty of testing
GC-triggered events across VMs. See ruby/spec#935
for more details.

Fixes jruby#7267
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants