Laurent Sansonetti
lsans****@apple*****
Thu Jun 14 17:02:00 JST 2007
Hi Satoshi-san, This seems indeed better to remove the instance from the cache within dealloc. # Though I wonder if it's really safe to replace the NSObject method like this. But at a glance all tests seem to pass, and you wrote that the previous problem you discovered is also fix by this change. I will commit the patch then, thank you! Laurent On Jun 13, 2007, at 7:39 AM, Satoshi Nakagawa wrote: > Hi. > >> + at implementation NSObject (__DeallocHook) >> +- (void)__clearCacheAndDealloc >> +{ >> + remove_from_oc2rb_cache(self); >> + [self __clearCacheAndDealloc]; >> +} >> + at end > > I have reconsidered this part. > It could seem to be a code of infinite recursion. > But actually [self __clearCacheAndDealloc] will invoke the original > version of dealloc. > It is a bit hard to read. > > I wrote another patch, more readable. > > -- > Satoshi Nakagawa > > Index: framework/src/objc/RBRuntime.m > =================================================================== > --- framework/src/objc/RBRuntime.m (revision 1837) > +++ framework/src/objc/RBRuntime.m (working copy) > @@ -228,6 +228,32 @@ > return RBNotifyException(api_name, err); > } > + at implementation NSObject (__DeallocHook) > + > +- (void) __dealloc > +{ > +} > + > +- (void) __clearCacheAndDealloc > +{ > + remove_from_oc2rb_cache(self); > + [self __dealloc]; > +} > + > + at end > + > +static void install_dealloc_hook() > +{ > + Method dealloc_method, aliased_dealloc_method, > cache_aware_dealloc_method; > + > + dealloc_method = class_getInstanceMethod([NSObject class], > @selector(dealloc)); > + aliased_dealloc_method = class_getInstanceMethod([NSObject > class], @selector(__dealloc)); > + cache_aware_dealloc_method = class_getInstanceMethod([NSObject > class], @selector(__clearCacheAndDealloc)); > + > + aliased_dealloc_method->method_imp = dealloc_method->method_imp; > + dealloc_method->method_imp = cache_aware_dealloc_method- > >method_imp; > +} > + > static int rubycocoa_initialized_flag = 0; > static int rubycocoa_initialized_p() > @@ -243,6 +269,7 @@ > if (! rubycocoa_initialized_flag) { > init_rb2oc_cache(); // initialize the Ruby->ObjC internal cache > init_oc2rb_cache(); // initialize the ObjC->Ruby internal cache > + install_dealloc_hook(); > initialize_mdl_osxobjc(); // initialize an objc part of rubycocoa > initialize_mdl_bundle_support(); > init_ovmix(); > Index: framework/src/objc/ocdata_conv.m > =================================================================== > --- framework/src/objc/ocdata_conv.m (revision 1837) > +++ framework/src/objc/ocdata_conv.m (working copy) > @@ -709,13 +709,11 @@ > result = rbobj_get_ocid(context_obj) == ocid ? context_obj : > ocobj_s_new(ocid); > } > - if (context_obj != Qfalse) { > - CACHE_LOCK(&oc2rbCacheLock); > - // Check out that the hash is still empty for us, to avoid a > race condition. > - if (!st_lookup(oc2rbCache, (st_data_t)ocid, (st_data_t > *)&result)) > - st_insert(oc2rbCache, (st_data_t)ocid, (st_data_t)result); > - CACHE_UNLOCK(&oc2rbCacheLock); > - } > + CACHE_LOCK(&oc2rbCacheLock); > + // Check out that the hash is still empty for us, to avoid a > race condition. > + if (!st_lookup(oc2rbCache, (st_data_t)ocid, (st_data_t > *)&result)) > + st_insert(oc2rbCache, (st_data_t)ocid, (st_data_t)result); > + CACHE_UNLOCK(&oc2rbCacheLock); > } > return result; > > _______________________________________________ > Rubycocoa-devel mailing list > Rubyc****@lists***** > http://lists.sourceforge.jp/mailman/listinfo/rubycocoa-devel
