From cd50e72f81628fb2a76da54a5ddfa98bf5ced6ab Mon Sep 17 00:00:00 2001
From: Earl Robsham <55839601+ERobsham@users.noreply.github.com>
Date: Wed, 25 Jan 2023 12:01:15 -0500
Subject: [PATCH] Fix lock-ordering during init (#237)

Reorders how locking is handled in `objc_send_initialize()` to prevent a deadlock.
Previously, contention on the low level spinlocks could cause a very intermittent deadlock:
  - Thread A : `objc_send_initialize()` holds the runtime lock, then tries to acquire the object lock on the metaclass, which needs to initialize the mutex for the new metaclass inside `referenceListForObject()`, so it tries to lock the `lock_for_pointer()` / `lock_spinlock()`
  - Thread B : `referenceListForObject()` holds a spinlock for an unrelated object while running `initHiddenClassForObject()` -> `allocateHiddenClass()`, which tries to acquire the runtime lock
If the metaclass object pointer in Thread A hashes to the same spinlock as the object in thread B, the runtime lock ends up deadlocked forever.
---
 dtable.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/dtable.c b/dtable.c
index cd09f35..be928b7 100644
--- a/dtable.c
+++ b/dtable.c
@@ -683,11 +683,11 @@ PRIVATE void objc_send_initialize(id object)
 		objc_send_initialize((id)class->super_class);
 	}
 
-	// Lock the runtime while we're creating dtables and before we acquire any
-	// other locks.  This prevents a lock-order reversal when
-	// dtable_for_class is called from something holding the runtime lock while
-	// we're still holding the initialize lock.  We should ensure that we never
-	// acquire the runtime lock after acquiring the initialize lock.
+	// Lock the runtime while we're creating dtables and before we acquire the
+	// init lock.  This prevents a lock-order reversal when dtable_for_class is
+	// called from something holding the runtime lock while we're still holding
+	// the initialize lock.  We should ensure that we never acquire the runtime
+	// lock after acquiring the initialize lock.
 	LOCK_RUNTIME();
 
 	// Superclass +initialize might possibly send a message to this class, in
@@ -708,7 +708,16 @@ PRIVATE void objc_send_initialize(id object)
 		return;
 	}
 
+	// We should try to acquire the class lock before any runtime/init locks.
+	// If another thread is in the middle of running `allocateHiddenClass()` it 
+	// has acquired a spinlock and will be trying to acquire the runtime lock. 
+	// When this happens there is a small chance we could hit the same spinlock
+	// and deadlock the process (as any further attempts to acquire the runtime 
+	// will also block forever).
+	UNLOCK_RUNTIME();
+
 	LOCK_OBJECT_FOR_SCOPE((id)meta);
+	LOCK_RUNTIME();
 	LOCK(&initialize_lock);
 	if (objc_test_class_flag(class, objc_class_flag_initialized))
 	{