Revision: 3947 http://trac.macosforge.org/projects/ruby/changeset/3947 Author: lsansonetti@apple.com Date: 2010-04-20 19:22:39 -0700 (Tue, 20 Apr 2010) Log Message: ----------- fix bugs in the way objc method types were generated + when we acquire the core global lock, make sure we also release it in case an exception happens Modified Paths: -------------- MacRuby/trunk/vm.cpp Modified: MacRuby/trunk/vm.cpp =================================================================== --- MacRuby/trunk/vm.cpp 2010-04-20 23:14:49 UTC (rev 3946) +++ MacRuby/trunk/vm.cpp 2010-04-21 02:22:39 UTC (rev 3947) @@ -64,6 +64,34 @@ VALUE rb_cTopLevel = 0; +// A simple class that acquires the core global lock in its constructor and +// releases it in its destructor. It is used to make sure the lock will be +// released in case an exception happens inside the scope (since C++ exceptions +// call object destructors). +class RoxorCoreLock { + private: + bool locked; + + public: + RoxorCoreLock() { + GET_CORE()->lock(); + locked = true; + } + + ~RoxorCoreLock() { + if (locked) { + GET_CORE()->unlock(); + locked = false; + } + } + + void unlock(void) { + assert(locked); + GET_CORE()->unlock(); + locked = false; + } +}; + class RoxorFunction { public: // Information retrieved from JITManager. @@ -1110,23 +1138,21 @@ rb_vm_set_default_random(VALUE random) { RoxorCore *core = GET_CORE(); + RoxorCoreLock lock; - core->lock(); if (core->get_default_random() != random) { GC_RELEASE(core->get_default_random()); GC_RETAIN(random); core->set_default_random(random); } - core->unlock(); } VALUE RoxorCore::trap_cmd_for_signal(int signal) { - lock(); - VALUE trap = trap_cmd[signal]; - unlock(); - return trap; + RoxorCoreLock lock; + + return trap_cmd[signal]; } extern "C" @@ -1139,10 +1165,9 @@ int RoxorCore::trap_level_for_signal(int signal) { - lock(); - VALUE trap = trap_level[signal]; - unlock(); - return trap; + RoxorCoreLock lock; + + return trap_level[signal]; } extern "C" @@ -1155,7 +1180,8 @@ void RoxorCore::set_trap_for_signal(VALUE trap, int level, int signal) { - lock(); + RoxorCoreLock lock; + VALUE oldtrap = trap_cmd[signal]; if (oldtrap != trap) { GC_RELEASE(oldtrap); @@ -1163,7 +1189,6 @@ trap_cmd[signal] = trap; trap_level[signal] = level; } - unlock(); } extern "C" @@ -1838,9 +1863,9 @@ #endif } -static inline void +static void resolve_method_type(char *buf, const size_t buflen, Class klass, Method m, - SEL sel, const unsigned int oc_arity) + SEL sel, const unsigned int types_count) { bs_element_method_t *bs_method = GET_CORE()->find_bs_method(klass, sel); @@ -1855,12 +1880,10 @@ // informal protocol method. const char *informal_type_str = informal_type->c_str(); strncpy(buf, informal_type_str, buflen); - const unsigned int type_arity = TypeArity(informal_type_str); - if (oc_arity > type_arity) { - for (unsigned int i = type_arity; i < oc_arity; i++) { - strlcat(buf, "@", buflen); - } - } + for (unsigned int i = TypeArity(informal_type_str); + i < types_count; i++) { + strlcat(buf, "@", buflen); + } } else { // Generate an automatic signature. We do check for KVO selectors @@ -1887,31 +1910,32 @@ else if (kvo_sel(klass, selname, selsize, "replaceObjectIn", "AtIndex:withObject:")) { strncpy(buf, "v@:i@", buflen); - } #if 0 // TODO else if (kvo_sel(klass, selname, selsize, "get", ":range:")) { } #endif else { - assert(oc_arity < buflen); + assert(types_count < buflen); + // retval, self and sel. buf[0] = strncmp(selname, "set", 3) == 0 ? 'v' : '@'; buf[1] = '@'; buf[2] = ':'; - for (unsigned int i = 3; i < oc_arity; i++) { + + // Arguments. + for (unsigned int i = 3; i < types_count; i++) { buf[i] = '@'; } - buf[oc_arity] = '\0'; + buf[types_count] = '\0'; } } } else { - const unsigned int m_argc = method_getNumberOfArguments(m); - if (m_argc < oc_arity) { - for (unsigned int i = m_argc; i < oc_arity; i++) { - strcat(buf, "@"); - } + assert(strlen(buf) >= 3); + for (unsigned int i = method_getNumberOfArguments(m) + 1; + i < types_count; i++) { + strlcat(buf, "@", buflen); } } } @@ -1920,7 +1944,7 @@ RoxorCore::retype_method(Class klass, rb_vm_method_node_t *node, const char *types) { - lock(); + RoxorCoreLock lock; // TODO: 1) don't reinstall method in case the types didn't change // 2) free LLVM machine code from old objc IMP @@ -1935,8 +1959,6 @@ node = add_method(klass, node->sel, node->objc_imp, node->ruby_imp, node->arity, node->flags, types); - unlock(); - return node; } @@ -1951,9 +1973,9 @@ } // Resolve Objective-C signature. - const int oc_arity = arity.real + 3; + const int types_count = arity.real + 3; // retval, self and sel char types[100]; - resolve_method_type(types, sizeof types, klass, m, sel, oc_arity); + resolve_method_type(types, sizeof types, klass, m, sel, types_count); // Generate Objective-C stub if needed. std::map<IMP, IMP>::iterator iter = objc_to_ruby_stubs.find(imp); @@ -2022,7 +2044,7 @@ return false; } - GET_CORE()->lock(); + RoxorCoreLock lock; bool status = false; @@ -2052,7 +2074,6 @@ status = GET_CORE()->resolve_methods(map, klass, sel); bails: - GET_CORE()->unlock(); return status; } @@ -2576,7 +2597,7 @@ const char *sel_name = sel_getName(sel); const bool genuine_selector = sel_name[strlen(sel_name) - 1] == ':'; - int oc_arity = genuine_selector ? arity.real : 0; + int types_count = genuine_selector ? arity.real + 3 : 3; bool redefined = direct; rb_vm_method_node_t *node; @@ -2584,7 +2605,7 @@ Method method = class_getInstanceMethod(klass, sel); char types[100]; - resolve_method_type(types, sizeof types, klass, method, sel, oc_arity); + resolve_method_type(types, sizeof types, klass, method, sel, types_count); node = GET_CORE()->add_method(klass, sel, objc_imp, ruby_imp, arity, flags, types); @@ -2594,7 +2615,7 @@ char buf[100]; snprintf(buf, sizeof buf, "%s:", sel_name); sel = sel_registerName(buf); - oc_arity = arity.real; + types_count = arity.real + 3; redefined = true; goto define_method; @@ -2604,7 +2625,7 @@ strlcpy(buf, sel_name, sizeof buf); buf[strlen(buf) - 1] = 0; // remove the ending ':' sel = sel_registerName(buf); - oc_arity = 0; + types_count = 3; redefined = true; goto define_method; @@ -2888,7 +2909,8 @@ void * RoxorCore::gen_large_arity_stub(int argc, bool is_block) { - lock(); + RoxorCoreLock lock; + std::map<int, void *> &stubs = is_block ? rb_large_arity_bstubs : rb_large_arity_rstubs; std::map<int, void *>::iterator iter = stubs.find(argc); @@ -2902,7 +2924,6 @@ else { stub = iter->second; } - unlock(); return stub; } @@ -2911,7 +2932,7 @@ RoxorCore::gen_stub(std::string types, bool variadic, int min_argc, bool is_objc) { - lock(); + RoxorCoreLock lock; #if ROXOR_VM_DEBUG printf("gen Ruby -> %s stub with types %s\n", is_objc ? "ObjC" : "C", @@ -2931,8 +2952,6 @@ stub = iter->second; } - unlock(); - return stub; } @@ -3453,6 +3472,7 @@ __vm_raise(void) { VALUE rb_exc = GET_VM()->current_exception(); + // DTrace probe: raise if (MACRUBY_RAISE_ENABLED()) { char *classname = (char *)rb_class2name(CLASS_OF(rb_exc)); @@ -4364,21 +4384,21 @@ void RoxorCore::register_finalizer(rb_vm_finalizer_t *finalizer) { - lock(); + RoxorCoreLock lock; + finalizers.push_back(finalizer); - unlock(); } void RoxorCore::unregister_finalizer(rb_vm_finalizer_t *finalizer) { - lock(); + RoxorCoreLock lock; + std::vector<rb_vm_finalizer_t *>::iterator i = std::find(finalizers.begin(), finalizers.end(), finalizer); if (i != finalizers.end()) { finalizers.erase(i); } - unlock(); } static void @@ -4440,9 +4460,9 @@ void RoxorCore::register_thread(VALUE thread) { - lock(); + RoxorCoreLock lock; + rb_ary_push(threads, thread); - unlock(); rb_vm_thread_t *t = GetThreadPtr(thread); pthread_assert(pthread_setspecific(RoxorVM::vm_thread_key, t->vm)); @@ -4456,7 +4476,8 @@ void RoxorCore::unregister_thread(VALUE thread) { - lock(); + RoxorCoreLock lock; + // We do not call #delete because it might trigger #== in case it has been // overriden on the thread object, and therefore cause a deadlock if the // new method tries to acquire the RoxorCore GIL. @@ -4474,8 +4495,9 @@ (void *)thread); abort(); } - unlock(); + lock.unlock(); + rb_vm_thread_t *t = GetThreadPtr(thread); const int code = pthread_mutex_destroy(&t->sleep_mutex); @@ -5103,7 +5125,7 @@ rb_vm_unregister_current_alien_thread(void) { // Check if the current pthread has been registered. - GET_CORE()->lock(); + RoxorCoreLock lock; pthread_t self = pthread_self(); VALUE ary = GET_CORE()->get_threads(); bool need_to_unregister = false; @@ -5113,7 +5135,7 @@ need_to_unregister = true; } } - GET_CORE()->unlock(); + lock.unlock(); // If yes, appropriately unregister it. if (need_to_unregister) {