Project

General

Profile

Actions

Feature #4168

closed

WeakRef is unsafe to use in Ruby 1.9

Added by bdurand (Brian Durand) about 14 years ago. Updated about 12 years ago.

Status:
Closed
Target version:
[ruby-core:33763]

Description

=begin
I've found a couple issues with the implementation of WeakRef in Ruby 1.9.

  1. WeakRef is unsafe to use because the ObjectSpace will recycle object ids for use with newly allocated objects after the old objects have been garbage collected. This problem was not present in 1.8, but with the 1.9 threading improvements there is no guarantee that the garbage collector thread has finished running the finalizers before new objects are allocated. This can result in a WeakRef returning a different object than the one it originally referenced.

  2. In Ruby 1.9 a Mutex is used to synchronize access to the shared data structures. However, Mutexes are not reentrant and the finalizers are silently throwing deadlock errors if they run while new WeakRefs are being created.

I've included in my patch a reimplementation of WeakRef called WeakReference that does not extend Delegator. This code is based on the original WeakRef code but has been rewritten so the variable names are a little clearer. It replaces the Mutex with a Monitor and adds an additional check to make sure that the object returned by the reference is the same one it originally referred to. WeakRef is rewritten as a Delegator wrapper around a WeakReference for backward compatibility.

I went this route because in some Ruby implementations (like Ruby 1.8), Delegator is very heavy weight to instantiate and creating a collection of thousands of WeakRefs will be slow and use up far too much memory (in fact, it would be nice to either backport this patch to Ruby 1.8 or the delegate changes from 1.9 to 1.8 if possible). I also don't really see the value of having weak references be delegators since is very unsafe to do anything without wrapping the call in a "rescue RefError" block. The object can be reclaimed at any time so the only safe method to be sure you still have it is to create a strong reference to the object at which point you might as well use that reference instead of the delegated methods on the WeakRef. It also should be simpler for other implementations of Ruby like Jruby or Rubinius to map native weak references to a simpler interface.

Sample code with WeakReference

 orig = Object.new
 ref = WeakReference.new(orig)
 # ...
 obj = ref.object
 if obj
   # Do something
 end

I also have a version of the patch which just fixes WeakRef to work without introducing a new class, but I feel this version is the right way to go.

Also included are unit tests for weak references but the test that checks for the broken functionality is not 100% reliable since garbage collection is not deterministic. I've also included a script that shows the problem in more detail with the existing weak reference implementation.

 ruby show_bug.rb 100000     # Run 100,000 iterations on the current implementation of WeakRef and report any problems
 ruby -I. show_bug.rb 100000 # Run 100,000 iterations on the new implementation of WeakRef and report any problems

=end


Files

weakref.rb (5.2 KB) weakref.rb Replacement for weakref.rb bdurand (Brian Durand), 12/18/2010 01:57 AM
test_weakref.rb (2.39 KB) test_weakref.rb Unit tests for WeakRef bdurand (Brian Durand), 12/18/2010 01:57 AM
show_bug.rb (1.71 KB) show_bug.rb Script to reproduce the bug bdurand (Brian Durand), 12/18/2010 01:57 AM
weakref_patch.diff (3.95 KB) weakref_patch.diff Alternate patch to weakref.rb (not recommended) bdurand (Brian Durand), 12/18/2010 01:57 AM
weakref.rb (5.12 KB) weakref.rb Fixed rdoc comments bdurand (Brian Durand), 12/18/2010 02:43 AM
weakref.rb (4.97 KB) weakref.rb Replacement for weakref.rb without synchronization in the finalizers bdurand (Brian Durand), 01/06/2011 03:00 AM
weakref_patch.diff (3.99 KB) weakref_patch.diff Patch to existing weakref.rb without synchronization in the finalizers bdurand (Brian Durand), 01/06/2011 03:00 AM
weakref.rb (5.09 KB) weakref.rb Reverted replacement for weakref.rb with WeakReference bdurand (Brian Durand), 01/11/2011 12:43 AM
weakref_patch.diff (4.08 KB) weakref_patch.diff Reverted alternate patch to weakref.rb that doesn't introduce a new class bdurand (Brian Durand), 01/11/2011 12:43 AM
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0