[ruby-core:58111] [CommonRuby - Feature #6308] Eliminate delegation from WeakRef

Issue #6308 has been updated by headius (Charles Nutter).


nobu (Nobuyoshi Nakada) wrote:
> Chained WeakRef (WeakRef to WeakRef) doesn't work well without delegation.
> 
>   w = WeakRef.new(obj)
>   w = WeakRef.new(w)
>   w.foo

I can't say I've ever had a need to wrap a weakref in a weakref. What's the use case?

> and
> 
>   w = WeakRefWithoutDelegation.new(obj)
>   w = WeakRefWithoutDelegation.new(w)
>   w.get.get.foo
> 
> How do you think?

This looks fine to me. WeakRef is a reference holder, so calling get is correct.

Let me summarize reasons why delegation is bad:

* Calls can fail at any time.
* Error handling is not possible because other consumers of the WeakRef won't know it's a WeakRef.
* On earlier versions of Ruby (pre-2.0) another object could get the same object_id and you'd call against the wrong object.

The bottom line is that delegating through a WeakRef is a big potential bug if you're not constantly checking that the referenced object still exists.

nobu (Nobuyoshi Nakada) wrote:
> Changing existing class drastically breaks existing code all.
> It's not acceptable.

Many behaviors change in breaking ways in x.y releases. I contend that WeakRef transparently delegating is a bug that needs to be fixed.

> For new name, what about WeakRef.[]?

I'll say it one more time: I don't believe *anyone* should be using the existing WeakRef behavior. Adding another class won't fix code that's out there doing things wrong (and delegating through WeakRef is *wrong*).

If we have to introduce a new class (I strongly protest) something like SimpleWeakRef might be better than a symbolic name.
----------------------------------------
Feature #6308: Eliminate delegation from WeakRef
https://bugs.ruby-lang.org/issues/6308#change-42700

Author: headius (Charles Nutter)
Status: Assigned
Priority: Normal
Assignee: matz (Yukihiro Matsumoto)
Category: 
Target version: Ruby 2.1.0


WeakRef's delegation features are a really awful pattern that should not be allowed in future versions of Ruby.

WeakRef makes no guarantees as to the liveness of its contained object. It can be collected at any time if there are no strong references to it.

WeakRef currently uses delegation to pass method calls through to the contained object. This encourages a pattern where a WeakRef is passed to methods that expect to have a reference to the underlying object, making it appear to be that object.

Unfortunately, this is *never* a good idea. Because the object can be collected at any time, you may get a nil reference from __getobj__ *arbitrarily* in code that tries to call methods against the given WeakRef. That means using WeakRef as a delegate will always result in unreliable code, and errors may happen for inexplicable reasons.

I believe Ruby 2.0 should eliminate WeakRef's delegation features and make it a simple reference holder. There's no safe way to use a weak reference except to grab a reference to the object, check that it is alive (non-nil) and then proceed with the use of the object, as follows:

obj = weakref.__getobj__
raise AppropriateError unless obj
obj.do_something
obj.do_something_else

Along with eliminating delegation, I would recommend simply making the get method #get, since the uglier #__getobj__ is only named that way because it is not delegated.


-- 
http://bugs.ruby-lang.org/