fixes for bug #29338

git-svn-id: svn+ssh://svn.gna.org/svn/gnustep/libs/base/trunk@31188 72102866-910b-0410-8b05-ffd578937521

ChangeLog

@@ -1,3 +1,12 @@
+2010-08-20  Richard Frith-Macdonald <rfm@gnu.org>
+
+	* 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 <rfm@gnu.org>
 
 	* configure.ac: Check alignment of pthread types

Source/ObjectiveC2/GNUmakefile

@@ -37,7 +37,7 @@ ObjectiveC2_OBJC_FILES = \
 	NSBlocks.m
 
 
-ifeq ($(OBJCSYNC), 0)
+ifneq ($(OBJCSYNC), 1)
 ObjectiveC2_OBJC_FILES += sync.m
 endif
 

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
+  if (cls == 0)
-  // while we are freeing the class.
+    {
-  objc_mutex_lock(__objc_runtime_mutex);
+      return;
-  safe_remove_from_subclass_list(meta);
+    }
-  safe_remove_from_subclass_list(cls);
+  meta = ((id) cls)->isa;
-  objc_mutex_unlock(__objc_runtime_mutex);
+
+  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);

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"
+   This file is part of GCC.
-#include "ObjectiveC2/runtime.h"
+
+   Gregory Casamento <greg.casamento@gmail.com>
+   
+   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 <pthread.h>
-#include <stdio.h>
 #include <stdlib.h>
+#include "objc/objc.h"
+#include "objc/objc-api.h"
+#include "objc/thr.h"
 
-@interface Fake
+/*
-+ (void) dealloc;
+ * Node structure...
-@end
+ */
+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 lock_node_t *lock_list = NULL;
-
+static objc_mutex_t table_lock = NULL; 
-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;
-}
 
+/**
+ * Initialize the table lock.
+ */
 static void
-deallocLockClass(id obj, SEL _cmd)
+sync_lock_init()
 {
-  Class lockClass = findLockClass(obj);
+  if (table_lock == NULL)
-  Class realClass = class_getSuperclass(lockClass);
+    {
-  // Free the lock
+      table_lock = objc_mutex_allocate();
-  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];
 }
 
-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);
+  lock_node_t *node = NULL;
-  pthread_mutex_t *lock;
+  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);
+      node = sync_add_node(obj);
-      // Test again in case two threads call objc_sync_enter at once
+      if (node == NULL)
-      lockClass = findLockClass(obj);
-      if (Nil == lockClass)
 	{
-	  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);
+  lock_node_t *node = NULL;
-  pthread_mutex_t *lock = object_getIndexedIvars(lockClass);
+  int status = 0;
-  pthread_mutex_unlock(lock);
+
+  // 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;  
 }
+