From: Nobuyoshi Nakada Date: 2011-09-30T02:14:13+09:00 Subject: [ruby-dev:44565] Re: [Ruby 1.9 - Bug #5350] WeakRef で謎の NoMethodError なかだです。 Thu, 29 Sep 2011 20:48:38 +0900, Nobuyoshi Nakada wrote in [ruby-dev:44562]: > 第一の問題点は、weakrefのファイナライズ処理が再入不能なのに、ファイナラ > イザ自体は再入することがあり得ることです。[ruby-dev:44527]にある再現コー > ドのように、weakrefのファイナライズ処理で内部的に使っているMutexが外部 > からも容易にアクセスできてしまうことも問題といっていいでしょう。 ObjectSpace::WeakMapを追加してそれを使うようにしたパッチです。 diff --git i/gc.c w/gc.c index fad49e0..ba0cac4 100644 --- i/gc.c +++ w/gc.c @@ -402,6 +402,9 @@ int *ruby_initial_gc_stress_ptr = &rb_objspace.gc_stress; #define nonspecial_obj_id(obj) (VALUE)((SIGNED_VALUE)(obj)|FIXNUM_FLAG) static void rb_objspace_call_finalizer(rb_objspace_t *objspace); +static VALUE define_final0(VALUE obj, VALUE block); +VALUE rb_define_final(VALUE obj, VALUE block); +VALUE rb_undefine_final(VALUE obj); #if defined(ENABLE_VM_OBJSPACE) && ENABLE_VM_OBJSPACE rb_objspace_t * @@ -2829,6 +2832,12 @@ os_each_obj(int argc, VALUE *argv, VALUE os) static VALUE undefine_final(VALUE os, VALUE obj) { + return rb_undefine_final(obj); +} + +VALUE +rb_undefine_final(VALUE obj) +{ rb_objspace_t *objspace = &rb_objspace; st_data_t data = obj; rb_check_frozen(obj); @@ -2849,9 +2858,7 @@ undefine_final(VALUE os, VALUE obj) static VALUE define_final(int argc, VALUE *argv, VALUE os) { - rb_objspace_t *objspace = &rb_objspace; - VALUE obj, block, table; - st_data_t data; + VALUE obj, block; rb_scan_args(argc, argv, "11", &obj, &block); rb_check_frozen(obj); @@ -2862,6 +2869,16 @@ define_final(int argc, VALUE *argv, VALUE os) rb_raise(rb_eArgError, "wrong type argument %s (should be callable)", rb_obj_classname(block)); } + return define_final0(obj, block); +} + +static VALUE +define_final0(VALUE obj, VALUE block) +{ + rb_objspace_t *objspace = &rb_objspace; + VALUE table; + st_data_t data; + if (!FL_ABLE(obj)) { rb_raise(rb_eArgError, "cannot define finalizer for %s", rb_obj_classname(obj)); @@ -2883,6 +2900,17 @@ define_final(int argc, VALUE *argv, VALUE os) return block; } +VALUE +rb_define_final(VALUE obj, VALUE block) +{ + rb_check_frozen(obj); + if (!rb_respond_to(block, rb_intern("call"))) { + rb_raise(rb_eArgError, "wrong type argument %s (should be callable)", + rb_obj_classname(block)); + } + return define_final0(obj, block); +} + void rb_gc_copy_finalizer(VALUE dest, VALUE obj) { @@ -3656,6 +3684,157 @@ gc_profile_total_time(VALUE self) * See also GC.count, GC.malloc_allocated_size and GC.malloc_allocations */ +struct weakmap { + st_table *obj2wmap; /* obj -> [ref,...] */ + st_table *wmap2obj; /* ref -> obj */ + VALUE final; +}; + +static int +wmap_mark_map(st_data_t key, st_data_t val, st_data_t arg) +{ + FL_SET((VALUE)val, FL_MARK); + return ST_CONTINUE; +} + +static void +wmap_mark(void *ptr) +{ + struct weakmap *w = ptr; + st_foreach(w->obj2wmap, wmap_mark_map, 0); + rb_gc_mark(w->final); +} + +static int +wmap_free_map(st_data_t key, st_data_t val, st_data_t arg) +{ + rb_ary_resize((VALUE)val, 0); + return ST_CONTINUE; +} + +static void +wmap_free(void *ptr) +{ + struct weakmap *w = ptr; + st_foreach(w->obj2wmap, wmap_free_map, 0); + st_clear(w->obj2wmap); + st_clear(w->wmap2obj); +} + +size_t rb_ary_memsize(VALUE ary); +static int +wmap_memsize_map(st_data_t key, st_data_t val, st_data_t arg) +{ + *(size_t *)arg += rb_ary_memsize((VALUE)val); + return ST_CONTINUE; +} + +static size_t +wmap_memsize(const void *ptr) +{ + size_t size; + const struct weakmap *w = ptr; + if (!w) return 0; + size = sizeof(*w); + size += st_memsize(w->obj2wmap); + size += st_memsize(w->wmap2obj); + st_foreach(w->obj2wmap, wmap_memsize_map, (st_data_t)&size); + return size; +} + +static const rb_data_type_t weakmap_type = { + "weakmap", + { + wmap_mark, + wmap_free, + wmap_memsize, + } +}; + +static VALUE +wmap_allocate(VALUE klass) +{ + struct weakmap *w; + VALUE obj = TypedData_Make_Struct(klass, struct weakmap, &weakmap_type, w); + w->obj2wmap = st_init_numtable(); + w->wmap2obj = st_init_numtable(); + w->final = rb_obj_method(obj, ID2SYM(rb_intern("finalize"))); + return obj; +} + +static VALUE +wmap_finalize(VALUE self, VALUE obj) +{ + st_data_t data; + VALUE rids; + long i; + struct weakmap *w; + + TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); + obj = NUM2PTR(obj); + + data = (st_data_t)obj; + if (st_delete(w->obj2wmap, &data, &data)) { + rids = (VALUE)data; + for (i = 0; i < RARRAY_LEN(rids); ++i) { + data = (st_data_t)RARRAY_PTR(rids)[i]; + st_delete(w->wmap2obj, &data, NULL); + } + } + + data = (st_data_t)obj; + if (st_delete(w->wmap2obj, &data, &data)) { + VALUE rid = (VALUE)data; + int empty = 1; + if (st_lookup(w->obj2wmap, (st_data_t)rid, &data)) { + rb_ary_delete((VALUE)data, obj); + empty = !RARRAY_LEN((VALUE)data); + } + if (empty) { + data = (st_data_t)rid; + st_delete(w->obj2wmap, &data, &data); + } + } +} + +static VALUE +wmap_aset(VALUE self, VALUE wmap, VALUE orig) +{ + st_data_t data; + VALUE rids; + struct weakmap *w; + + TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); + rb_define_final(orig, w->final); + rb_define_final(wmap, w->final); + if (!st_lookup(w->obj2wmap, (st_data_t)orig, &data)) { + rids = rb_ary_tmp_new(1); + st_insert(w->obj2wmap, (st_data_t)orig, (st_data_t)rids); + } + else { + rids = (VALUE)data; + } + rb_ary_push(rids, orig); + st_insert(w->wmap2obj, (st_data_t)wmap, (st_data_t)orig); + return nonspecial_obj_id(orig); +} + +static VALUE +wmap_aref(VALUE self, VALUE wmap) +{ + st_data_t data; + VALUE obj; + struct weakmap *w; + rb_objspace_t *objspace = &rb_objspace; + + TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w); + if (!st_lookup(w->wmap2obj, (st_data_t)wmap, &data)) return Qnil; + obj = (VALUE)data; + if (!is_id_value(objspace, obj)) return Qnil; + if (!is_live_object(objspace, obj)) return Qnil; + return obj; +} + /* * The GC module provides an interface to Ruby's mark and * sweep garbage collection mechanism. Some of the underlying methods @@ -3710,6 +3889,14 @@ Init_GC(void) rb_define_module_function(rb_mObSpace, "count_objects", count_objects, -1); + { + VALUE rb_cWeakMap = rb_define_class_under(rb_mObSpace, "WeakMap", rb_cObject); + rb_define_alloc_func(rb_cWeakMap, wmap_allocate); + rb_define_method(rb_cWeakMap, "[]=", wmap_aset, 2); + rb_define_method(rb_cWeakMap, "[]", wmap_aref, 1); + rb_define_private_method(rb_cWeakMap, "finalize", wmap_finalize, 1); + } + #if CALC_EXACT_MALLOC_SIZE rb_define_singleton_method(rb_mGC, "malloc_allocated_size", gc_malloc_allocated_size, 0); rb_define_singleton_method(rb_mGC, "malloc_allocations", gc_malloc_allocations, 0); diff --git i/lib/weakref.rb w/lib/weakref.rb index ee5444a..1fea9a9 100644 --- i/lib/weakref.rb +++ w/lib/weakref.rb @@ -1,5 +1,4 @@ require "delegate" -require 'thread' # Weak Reference class that allows a referenced object to be # garbage-collected. A WeakRef may be used exactly like the object it @@ -16,6 +15,7 @@ require 'thread' # p foo.to_s # should raise exception (recycled) class WeakRef < Delegator + @@__map = ::ObjectSpace::WeakMap.new ## # RefError is raised when a referenced object has been recycled by the @@ -24,51 +24,17 @@ class WeakRef < Delegator class RefError < StandardError end - @@id_map = {} # obj -> [ref,...] - @@id_rev_map = {} # ref -> obj - @@mutex = Mutex.new - @@final = lambda {|id| - @@mutex.synchronize { - rids = @@id_map[id] - if rids - for rid in rids - @@id_rev_map.delete(rid) - end - @@id_map.delete(id) - end - rid = @@id_rev_map[id] - if rid - @@id_rev_map.delete(id) - @@id_map[rid].delete(id) - @@id_map.delete(rid) if @@id_map[rid].empty? - end - } - } - ## # Creates a weak reference to +orig+ def initialize(orig) - @__id = orig.object_id - ObjectSpace.define_finalizer orig, @@final - ObjectSpace.define_finalizer self, @@final - @@mutex.synchronize { - @@id_map[@__id] = [] unless @@id_map[@__id] - } - @@id_map[@__id].push self.object_id - @@id_rev_map[self.object_id] = @__id + @@__map[self] = orig super end def __getobj__ # :nodoc: - unless @@id_rev_map[self.object_id] == @__id - Kernel::raise RefError, "Invalid Reference - probably recycled", Kernel::caller(2) - end - begin - ObjectSpace._id2ref(@__id) - rescue RangeError + @@__map[self] or Kernel::raise RefError, "Invalid Reference - probably recycled", Kernel::caller(2) - end end def __setobj__(obj) # :nodoc: @@ -78,7 +44,7 @@ class WeakRef < Delegator # Returns true if the referenced object is still alive. def weakref_alive? - @@id_rev_map[self.object_id] == @__id + !!@@__map[self] end end -- --- 僕の前にBugはない。 --- 僕の後ろにBugはできる。 中田 伸悦