Laurent Sansonetti
lsans****@apple*****
Mon Jun 18 11:26:50 JST 2007
Committed as r1839. Sorry for the late response, I'm currently on vacation :) Laurent On Jun 14, 2007, at 1:07 AM, Satoshi Nakagawa wrote: > Hi Laurent. > >> # Though I wonder if it's really safe to replace the NSObject method >> like this. > > It's the same way as AspectCocoa's way. > So I think it's a proven technique. > > -- > Satoshi Nakagawa > > > On 2007/06/14, at 17:02, Laurent Sansonetti wrote: > >> 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 >> >> _______________________________________________ >> Rubycocoa-devel mailing list >> Rubyc****@lists***** >> http://lists.sourceforge.jp/mailman/listinfo/rubycocoa-devel > > _______________________________________________ > Rubycocoa-devel mailing list > Rubyc****@lists***** > http://lists.sourceforge.jp/mailman/listinfo/rubycocoa-devel