From ca25388b378d8cefa32a493c359d059e397eb420 Mon Sep 17 00:00:00 2001 From: theraven Date: Sat, 12 Jan 2013 16:29:20 +0000 Subject: [PATCH] Fix a lock-order reversal where one thread calls some dtable-manipulating function while another is initializing. --- dtable.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/dtable.c b/dtable.c index 744d88a..cbac716 100644 --- a/dtable.c +++ b/dtable.c @@ -680,6 +680,12 @@ PRIVATE void objc_send_initialize(id object) return; } + // 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_RUNTIME(); LOCK_OBJECT_FOR_SCOPE((id)meta); LOCK(&initialize_lock); if (objc_test_class_flag(class, objc_class_flag_initialized)) @@ -696,6 +702,14 @@ PRIVATE void objc_send_initialize(id object) dtable_t class_dtable = create_dtable_for_class(class, uninstalled_dtable); dtable_t dtable = skipMeta ? 0 : create_dtable_for_class(meta, class_dtable); + // Now we've finished doing things that may acquire the runtime lock, so we + // can hold onto the initialise lock to make anything doing + // dtable_for_class block until we've finished updating temporary dtable + // lists. + // If another thread holds the runtime lock, it can now proceed until it + // gets into a dtable_for_class call, and then block there waiting for us + // to finish setting up the temporary dtable. + UNLOCK_RUNTIME(); static SEL initializeSel = 0; if (0 == initializeSel) @@ -730,7 +744,12 @@ PRIVATE void objc_send_initialize(id object) __attribute__((cleanup(remove_dtable))) InitializingDtable meta_buffer = { meta, dtable, &buffer }; temporary_dtables = &meta_buffer; + // We now release the initialize lock. We'll reacquire it later when we do + // the cleanup, but at this point we allow other threads to get the + // temporary dtable and call +initialize in other threads. UNLOCK(&initialize_lock); + // We still hold the class lock at this point. dtable_for_class will block + // there after acquiring the temporary dtable. checkARCAccessors(class);