Feature #15974
closedWarn in verbose mode on defining a finalizer that captures the object
Description
There's a really common mistake people make when using define_finalizer
- they capture the object in the finalizer.
https://www.mikeperham.com/2010/02/24/the-trouble-with-ruby-finalizers/
This PR warns about that, when in verbose mode (it's too slow to do always.)
Updated by ko1 (Koichi Sasada) almost 5 years ago
it makes sense.
Updated by mame (Yusuke Endoh) almost 5 years ago
Matz has approved this proposal.
Updated by shevegen (Robert A. Heiler) almost 5 years ago
Matz has approved this proposal.
\o/
Updated by nobu (Nobuyoshi Nakada) almost 5 years ago
This is a good feature, but I don't think this should belong to the "spec" as seems an implementation detail.
Updated by chrisseaton (Chris Seaton) almost 5 years ago
I think Ruby Spec considers warnings to be user-visible features, so does normally spec them. This prompts alternative implementations like JRuby and TruffleRuby to implement the same warning so that the user experience is the same on all implementations.
@Eregon (Benoit Daloze) will be able to give a definitive opinion on whether it should be spec'd or not.
Updated by chrisseaton (Chris Seaton) almost 5 years ago
- Status changed from Open to Closed
Applied in changeset git|928260c2a613bbdd4402c300e0bf86ae7562e52a.
Warn in verbose mode on defining a finalizer that captures the object
[Feature #15974]
Updated by Eregon (Benoit Daloze) almost 5 years ago
chrisseaton (Chris Seaton) wrote:
I think Ruby Spec considers warnings to be user-visible features, so does normally spec them. This prompts alternative implementations like JRuby and TruffleRuby to implement the same warning so that the user experience is the same on all implementations.
@Eregon (Benoit Daloze) will be able to give a definitive opinion on whether it should be spec'd or not.
Yes, exactly, it's a useful feature for other implementations to have too and it's user-visible so it's good to add to ruby/spec, similar to warnings.
Updated by Dan0042 (Daniel DeLorme) over 4 years ago
Question: shouldn't this be an error rather than a warning? Defining a finalizer that immortalizes its object and can never run seems like an outright bug, not something that you should merely warn about in verbose mode only.
Updated by chrisseaton (Chris Seaton) over 4 years ago
Question: shouldn't this be an error rather than a warning?
No, because it's possible to have a reference from the finaliser to the object when the finaliser is created, but then to later clear that reference and the finaliser to work properly. The specs cover this.
However this PR was reverted due to some bugs (I wrote it in a couple of hours at a hackathon and it was merged then and there, so not surprising it wasn't really fully baked yet!) I hope to get it back in at some point.
Updated by Dan0042 (Daniel DeLorme) over 4 years ago
Ah, good point.
But maybe ObjectSpace.define_finalizer(obj, aProc)
should raise an error if aProc.binding.receiver.equal?(obj)
, because that's a reference that can never be cleared.
Updated by chrisseaton (Chris Seaton) over 4 years ago
Yes we could do that, and enable it always as that'd be cheap.
Updated by chrisseaton (Chris Seaton) over 3 years ago
A partial solution to this but with a better implementation is at https://github.com/ruby/ruby/pull/3444.