From 20220c48f5e29fdd2e5bd4a74916cef3fec56283 Mon Sep 17 00:00:00 2001 From: Richard Frith-MacDonald Date: Fri, 20 Aug 2010 11:07:33 +0000 Subject: [PATCH] fixes for bug #29338 git-svn-id: svn+ssh://svn.gna.org/svn/gnustep/libs/base/trunk@31188 72102866-910b-0410-8b05-ffd578937521 --- ChangeLog | 9 + Source/ObjectiveC2/GNUmakefile | 2 +- Source/ObjectiveC2/runtime.c | 58 +++++- Source/ObjectiveC2/sync.m | 328 ++++++++++++++++++++++----------- 4 files changed, 276 insertions(+), 121 deletions(-) diff --git a/ChangeLog b/ChangeLog index 07592f857..57f62dfb5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2010-08-20 Richard Frith-Macdonald + + * Source/ObjectiveC2/runtime.c: Made objc_disposeClassPair() a no-op + if the class pair is already registered .. to prevent it from causing + crashes. Made objc_registerClassPair() log error if there is already + a class of the name. + * Source/ObjectiveC2/sync.m: Replace with Greg's implementation + (reported as working though I haven't tested it). + 2010-08-19 Richard Frith-Macdonald * configure.ac: Check alignment of pthread types diff --git a/Source/ObjectiveC2/GNUmakefile b/Source/ObjectiveC2/GNUmakefile index 862f511bc..97fabd762 100644 --- a/Source/ObjectiveC2/GNUmakefile +++ b/Source/ObjectiveC2/GNUmakefile @@ -37,7 +37,7 @@ ObjectiveC2_OBJC_FILES = \ NSBlocks.m -ifeq ($(OBJCSYNC), 0) +ifneq ($(OBJCSYNC), 1) ObjectiveC2_OBJC_FILES += sync.m endif diff --git a/Source/ObjectiveC2/runtime.c b/Source/ObjectiveC2/runtime.c index c042580d7..7701607f4 100644 --- a/Source/ObjectiveC2/runtime.c +++ b/Source/ObjectiveC2/runtime.c @@ -858,14 +858,37 @@ safe_remove_from_subclass_list(Class cls) void objc_disposeClassPair(Class cls) { - Class meta = ((id) cls)->isa; + Class meta; - // Remove from the runtime system so nothing tries updating the dtable - // while we are freeing the class. - objc_mutex_lock(__objc_runtime_mutex); - safe_remove_from_subclass_list(meta); - safe_remove_from_subclass_list(cls); - objc_mutex_unlock(__objc_runtime_mutex); + if (cls == 0) + { + return; + } + meta = ((id) cls)->isa; + + if (objc_lookUpClass (class_getName (cls)) == (id)cls) + { + fprintf(stderr, "*** ERROR *** function objc_disposeClassPair() called " + "on registered class pair '%s'\n", class_getName(cls)); + return; + /* + The runtime provides no mechanism to remove a class. + The following code essentially frees the memory used by a class without + fully removing it ... which obviously tends to cause random crashes + later on if anything tries to use the class or to traverse data + structures containing the class. + Indeed, it's hard to see how this function could ever be made to work + (what if there are subclasses of the class being removed, or if + there are instances of the class?) even with changes to the runtime. + + // Remove from the runtime system so nothing tries updating the dtable + // while we are freeing the class. + objc_mutex_lock(__objc_runtime_mutex); + safe_remove_from_subclass_list(meta); + safe_remove_from_subclass_list(cls); + objc_mutex_unlock(__objc_runtime_mutex); + */ + } // Free the method and ivar lists. freeMethodLists(cls); @@ -981,9 +1004,28 @@ void __objc_resolve_class_links(void); void objc_registerClassPair(Class cls) { - Class metaClass = cls->class_pointer; + Class metaClass; + Class existing; + + if (Nil == cls) + { + fprintf(stderr, "*** ERROR *** function objc_registerClassPair() called " + "on Nil class pair '%s'\n", class_getName(cls)); + } + existing = (Class)objc_lookUpClass (class_getName (cls)); + if (existing == cls) + { + return; // Already registered + } + if (Nil == existing) + { + fprintf(stderr, "*** ERROR *** function objc_registerClassPair() called " + "for class pair with name ('%s') of existing class.\n", + class_getName(cls)); + } // Initialize the dispatch table for the class and metaclass. + metaClass = cls->class_pointer; __objc_update_dispatch_table_for_class(metaClass); __objc_update_dispatch_table_for_class(cls); __objc_add_class_to_hash(cls); diff --git a/Source/ObjectiveC2/sync.m b/Source/ObjectiveC2/sync.m index 90645b27f..cce854370 100644 --- a/Source/ObjectiveC2/sync.m +++ b/Source/ObjectiveC2/sync.m @@ -1,135 +1,239 @@ +/* + The implementation of synchronization primitives for Objective-C. + Copyright (C) 2008 Free Software Foundation, Inc. -#include "config.h" -#include "ObjectiveC2/runtime.h" + This file is part of GCC. + + Gregory Casamento + + GCC is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by the + Free Software Foundation; either version 2, or (at your option) any + later version. + + GCC is distributed in the hope that it will be useful, but WITHOUT + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public + License for more details. + + You should have received a copy of the GNU General Public License + along with GCC; see the file COPYING. If not, write to + the Free Software Foundation, 51 Franklin Street, Fifth Floor, + Boston, MA 02110-1301, USA. +*/ + +/* + As a special exception, if you link this library with files compiled + with GCC to produce an executable, this does not cause the resulting + executable to be covered by the GNU General Public License. This + exception does not however invalidate any other reasons why the + executable file might be covered by the GNU General Public License. +*/ -#include -#include #include +#include "objc/objc.h" +#include "objc/objc-api.h" +#include "objc/thr.h" -@interface Fake -+ (void) dealloc; -@end +/* + * Node structure... + */ +typedef struct lock_node { + id obj; + objc_mutex_t lock; + struct lock_node *next; + struct lock_node *prev; +} lock_node_t; -static pthread_mutex_t at_sync_init_lock = PTHREAD_MUTEX_INITIALIZER; -static unsigned long long lockClassId; +/* + * Return types for the locks... + */ +typedef enum { + OBJC_SYNC_SUCCESS = 0, + OBJC_SYNC_NOT_OWNING_THREAD_ERROR = -1, + OBJC_SYNC_TIMED_OUT = -2, + OBJC_SYNC_NOT_INITIALIZED = -3 +} sync_return_t; -IMP objc_msg_lookup(id, SEL); - -static void deallocLockClass(id obj, SEL _cmd); - -static inline Class -findLockClass(id obj) -{ - struct objc_object object = { obj->isa }; - SEL dealloc = @selector(dealloc); - Class lastClass; - - // Find the first class where this lookup is correct - if (objc_msg_lookup((id)&object, dealloc) != (IMP)deallocLockClass) - { - do { - object.isa = class_getSuperclass(object.isa); - } while (Nil != object.isa - && objc_msg_lookup((id)&object, dealloc) != (IMP)deallocLockClass); - } - if (Nil == object.isa) - { - return Nil; - } - /* object->isa is now either the lock class, or a class which inherits from - * the lock class - */ - do { - lastClass = object.isa; - object.isa = class_getSuperclass(object.isa); - } while (Nil != object.isa - && objc_msg_lookup((id)&object, dealloc) == (IMP)deallocLockClass); - return lastClass; -} - -static inline Class -initLockObject(id obj) -{ - Class lockClass; - const char *types; - pthread_mutex_t *lock; - pthread_mutexattr_t attr; - - if (class_isMetaClass(obj->isa)) - { - lockClass = objc_allocateMetaClass(obj, sizeof(pthread_mutex_t)); - } - else - { - char nameBuffer[40]; - - snprintf(nameBuffer, 39, "hiddenlockClass%lld", lockClassId++); - lockClass = objc_allocateClassPair(obj->isa, nameBuffer, - sizeof(pthread_mutex_t)); - } - - types = method_getTypeEncoding(class_getInstanceMethod(obj->isa, - @selector(dealloc))); - class_addMethod(lockClass, @selector(dealloc), (IMP)deallocLockClass, types); - - if (!class_isMetaClass(obj->isa)) - { - objc_registerClassPair(lockClass); - } - - lock = object_getIndexedIvars(lockClass); - pthread_mutexattr_init(&attr); - pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); - pthread_mutex_init(lock, &attr); - pthread_mutexattr_destroy(&attr); - - obj->isa = lockClass; - return lockClass; -} +static lock_node_t *lock_list = NULL; +static objc_mutex_t table_lock = NULL; +/** + * Initialize the table lock. + */ static void -deallocLockClass(id obj, SEL _cmd) +sync_lock_init() { - Class lockClass = findLockClass(obj); - Class realClass = class_getSuperclass(lockClass); - // Free the lock - pthread_mutex_t *lock = object_getIndexedIvars(lockClass); - - pthread_mutex_destroy(lock); - // Free the class -#ifndef __MINGW32__ - objc_disposeClassPair(lockClass); -#endif - // Reset the class then call the real -dealloc - obj->isa = realClass; - [obj dealloc]; + if (table_lock == NULL) + { + table_lock = objc_mutex_allocate(); + } } -void +/** + * Find the node in the list. + */ +static lock_node_t* +sync_find_node(id obj) +{ + lock_node_t *current = lock_list; + + if (lock_list != NULL) + { + // iterate over the list looking for the end... + while (current != NULL) + { + // if the current object is the one, breal and + // return that node. + if (current->obj == obj) + { + break; + } + + // get the next one... + current = current->next; + } + } + return current; +} + +/** + * Add a node for the object, if one doesn't already exist. + */ +static lock_node_t* +sync_add_node(id obj) +{ + lock_node_t *current = NULL; + + // get the lock... + sync_lock_init(); + + // if the list hasn't been initialized, initialize it. + if (lock_list == NULL) + { + // instantiate the new node and set the list... + lock_list = malloc(sizeof(lock_node_t)); + + // set the current node to the last in the list... + current = lock_list; + + // set next and prev... + current->prev = NULL; + current->next = NULL; + } + else + { + lock_node_t *new_node = NULL; + current = lock_list; + + // look for the end of the list. + while (current->next) + { + current = current->next; + } + + // instantiate the new node... + new_node = malloc(sizeof(lock_node_t)); + + if (new_node != NULL) + { + // set next and prev... + current->next = new_node; + new_node->prev = current; + new_node->next = NULL; + + // set the current node to the last in the list... + current = new_node; + } + } + + if (current != NULL) + { + // add the object and it's lock + current->obj = obj; + current->lock = objc_mutex_allocate(); + } + + return current; +} + +/** + * Add a lock for the object. + */ +int +__attribute__((weak)) objc_sync_enter(id obj) { - Class lockClass = findLockClass(obj); - pthread_mutex_t *lock; + lock_node_t *node = NULL; + int status = 0; - if (Nil == lockClass) + // lock access to the table until we're done.... + sync_lock_init(); + objc_mutex_lock(table_lock); + + node = sync_find_node(obj); + if (node == NULL) { - pthread_mutex_lock(&at_sync_init_lock); - // Test again in case two threads call objc_sync_enter at once - lockClass = findLockClass(obj); - if (Nil == lockClass) + node = sync_add_node(obj); + if (node == NULL) { - lockClass = initLockObject(obj); + // unlock the table.... + objc_mutex_unlock(table_lock); + return OBJC_SYNC_NOT_INITIALIZED; } - pthread_mutex_unlock(&at_sync_init_lock); } - lock = object_getIndexedIvars(lockClass); - pthread_mutex_lock(lock); + + // unlock the table.... + objc_mutex_unlock(table_lock); + + status = objc_mutex_lock(node->lock); + + // if the status is more than one, then another thread + // has this section locked, so we abort. A status of -1 + // indicates that an error occurred. + if (status > 1 || status == -1) + { + return OBJC_SYNC_NOT_OWNING_THREAD_ERROR; + } + + return OBJC_SYNC_SUCCESS; } -void +/** + * Remove a lock for the object. + */ +int +__attribute__((weak)) objc_sync_exit(id obj) { - Class lockClass = findLockClass(obj); - pthread_mutex_t *lock = object_getIndexedIvars(lockClass); - pthread_mutex_unlock(lock); + lock_node_t *node = NULL; + int status = 0; + + // lock access to the table until we're done.... + sync_lock_init(); + objc_mutex_lock(table_lock); + + node = sync_find_node(obj); + if (node == NULL) + { + // unlock the table.... + objc_mutex_unlock(table_lock); + return OBJC_SYNC_NOT_INITIALIZED; + } + + status = objc_mutex_unlock(node->lock); + + // unlock the table.... + objc_mutex_unlock(table_lock); + + // if the status is not zero, then we are not the sole + // owner of this node. Also if -1 is returned, this indicates and error + // condition. + if (status > 0 || status == -1) + { + return OBJC_SYNC_NOT_OWNING_THREAD_ERROR; + } + + return OBJC_SYNC_SUCCESS; } +