Actually wait for +initialize to finish before allowing any thread to send a message to a class (I broke this when I rewrote the dtable code to use a per-class lock - the lookup forgot to acquire the correct mutex.)

Test/RuntimeTest.m

@@ -252,6 +252,32 @@ void testRegisterAlias()
   printf("testRegisterAlias() ran\n");
 }
 
+@interface SlowInit1
++ (void)doNothing;
+@end
+@interface SlowInit2
++ (void)doNothing;
+@end
+@implementation SlowInit1
++ (void)initialize
+{
+	sleep(1);
+	[SlowInit2 doNothing];
+}
++ (void)doNothing {}
+@end
+static int initCount;
+@implementation SlowInit1
++ (void)initialize
+{
+	sleep(1);
+	__sync_fetch_and_add(&initCount, 1);
+}
++ (void)doNothing {}
+@end
+
+
+
 int main (int argc, const char * argv[])
 {
   testInvalidArguments();

category_loader.c

@@ -2,6 +2,7 @@
 #include "objc/runtime.h"
 #include "visibility.h"
 #include "loader.h"
+#include "dtable.h"
 
 #define BUFFER_TYPE struct objc_category
 #include "buffer.h"
@@ -17,10 +18,16 @@ static void register_methods(struct objc_class *cls, struct objc_method_list *l)
 	// Add the method list at the head of the list of lists.
 	l->next = cls->methods;
 	cls->methods = l;
-	// Update the dtable to catch the new methods.
+	// Update the dtable to catch the new methods, if the dtable has been
-	// FIXME: We can make this more efficient by simply passing the new method
+	// created (don't bother creating dtables for classes when categories are
-	// list to the dtable and telling it only to update those methods.
+	// loaded if the class hasn't received any messages yet.
-	objc_update_dtable_for_class(cls);
+	if (classHasDtable(cls))
+	{
+		fprintf(stderr, "Updating dtable for %s\n", class_getName(cls));
+		// FIXME: We can make this more efficient by simply passing the new method
+		// list to the dtable and telling it only to update those methods.
+		objc_update_dtable_for_class(cls);
+	}
 }
 
 static void load_category(struct objc_category *cat, struct objc_class *class)

dtable.h

@@ -46,6 +46,8 @@ static inline int classHasInstalledDtable(struct objc_class *cls)
 	return (cls->dtable != uninstalled_dtable);
 }
 
+int objc_sync_enter(id object);
+int objc_sync_exit(id object);
 /**
  * Returns the dtable for a given class.  If we are currently in an +initialize
  * method then this will block if called from a thread other than the one
@@ -58,29 +60,43 @@ static inline dtable_t dtable_for_class(Class cls)
 		return cls->dtable;
 	}
 
-	LOCK_FOR_SCOPE(&initialize_lock);
-	if (classHasInstalledDtable(cls))
-	{
-		return cls->dtable;
-	}
-	/* This is a linear search, and so, in theory, could be very slow.  It is
-	* O(n) where n is the number of +initialize methods on the stack.  In
-	* practice, this is a very small number.  Profiling with GNUstep showed that
-	* this peaks at 8. */
 	dtable_t dtable = uninstalled_dtable;
-	InitializingDtable *buffer = temporary_dtables;
+
-	while (NULL != buffer)
 	{
-		if (buffer->class == cls)
+		LOCK_FOR_SCOPE(&initialize_lock);
+		if (classHasInstalledDtable(cls))
+		{
+			return cls->dtable;
+		}
+		/* This is a linear search, and so, in theory, could be very slow.  It
+		 * is O(n) where n is the number of +initialize methods on the stack.
+		 * In practice, this is a very small number.  Profiling with GNUstep
+		 * showed that this peaks at 8. */
+		InitializingDtable *buffer = temporary_dtables;
+		while (NULL != buffer)
 		{
-			dtable = buffer->dtable;
+			if (buffer->class == cls)
-			break;
+			{
+				dtable = buffer->dtable;
+				break;
+			}
+			buffer = buffer->next;
 		}
-		buffer = buffer->next;
 	}
-	if (dtable == 0)
+
+	if (dtable != uninstalled_dtable)
 	{
-		dtable = uninstalled_dtable;
+		// Make sure that we block if +initialize is still running.  We do this
+		// after we've released the initialize lock, so that the real dtable
+		// can be installed.  This acquires / releases a recursive mutex, so if
+		// this mutex is already held by this thread then this will proceed
+		// immediately.  If it's held by another thread (i.e. the one running
+		// +initialize) then we block here until it's run.  We don't need to do
+		// this if the dtable is the uninstalled dtable, because that means
+		// +initialize has not yet been sent, so we can wait until something
+		// triggers it before needing any synchronisation.
+		objc_sync_enter((id)cls);
+		objc_sync_exit((id)cls);
 	}
 	return dtable;
 }