From d6590d653415c0bfacf97e7f768dd3c994cb8d26 Mon Sep 17 00:00:00 2001
From: Thomas Van Lenten <thomasvl@google.com>
Date: Thu, 17 Dec 2015 14:35:44 -0500
Subject: Drop all use of OSSpinLock

Apple engineers have pointed out that OSSpinLocks are vulnerable to live locking
on iOS in cases of priority inversion:
. http://mjtsai.com/blog/2015/12/16/osspinlock-is-unsafe/
. https://lists.swift.org/pipermail/swift-dev/Week-of-Mon-20151214/000372.html

- Use a dispatch_semaphore_t within the extension registry.
- Use a dispatch_semaphore_t for protecting autocreation within messages.
- Drop the custom/internal GPBString class since we don't have really good
  numbers to judge the locking replacements and it isn't required. We can
  always bring it back with real data in the future.
---
 objectivec/GPBRootObject.m | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

(limited to 'objectivec/GPBRootObject.m')

diff --git a/objectivec/GPBRootObject.m b/objectivec/GPBRootObject.m
index 7036723f..4570716f 100644
--- a/objectivec/GPBRootObject.m
+++ b/objectivec/GPBRootObject.m
@@ -31,7 +31,6 @@
 #import "GPBRootObject_PackagePrivate.h"
 
 #import <objc/runtime.h>
-#import <libkern/OSAtomic.h>
 
 #import <CoreFoundation/CoreFoundation.h>
 
@@ -96,13 +95,19 @@ static CFHashCode GPBRootExtensionKeyHash(const void *value) {
   return jenkins_one_at_a_time_hash(key);
 }
 
-static OSSpinLock gExtensionSingletonDictionaryLock_ = OS_SPINLOCK_INIT;
+// NOTE: OSSpinLock may seem like a good fit here but Apple engineers have
+// pointed out that they are vulnerable to live locking on iOS in cases of
+// priority inversion:
+//   http://mjtsai.com/blog/2015/12/16/osspinlock-is-unsafe/
+//   https://lists.swift.org/pipermail/swift-dev/Week-of-Mon-20151214/000372.html
+static dispatch_semaphore_t gExtensionSingletonDictionarySemaphore;
 static CFMutableDictionaryRef gExtensionSingletonDictionary = NULL;
 static GPBExtensionRegistry *gDefaultExtensionRegistry = NULL;
 
 + (void)initialize {
   // Ensure the global is started up.
   if (!gExtensionSingletonDictionary) {
+    gExtensionSingletonDictionarySemaphore = dispatch_semaphore_create(1);
     CFDictionaryKeyCallBacks keyCallBacks = {
       // See description above for reason for using custom dictionary.
       0,
@@ -134,9 +139,10 @@ static GPBExtensionRegistry *gDefaultExtensionRegistry = NULL;
 
 + (void)globallyRegisterExtension:(GPBExtensionDescriptor *)field {
   const char *key = [field singletonNameC];
-  OSSpinLockLock(&gExtensionSingletonDictionaryLock_);
+  dispatch_semaphore_wait(gExtensionSingletonDictionarySemaphore,
+                          DISPATCH_TIME_FOREVER);
   CFDictionarySetValue(gExtensionSingletonDictionary, key, field);
-  OSSpinLockUnlock(&gExtensionSingletonDictionaryLock_);
+  dispatch_semaphore_signal(gExtensionSingletonDictionarySemaphore);
 }
 
 static id ExtensionForName(id self, SEL _cmd) {
@@ -166,14 +172,24 @@ static id ExtensionForName(id self, SEL _cmd) {
   key[classNameLen] = '_';
   memcpy(&key[classNameLen + 1], selName, selNameLen);
   key[classNameLen + 1 + selNameLen] = '\0';
-  OSSpinLockLock(&gExtensionSingletonDictionaryLock_);
+
+  // NOTE: Even though this method is called from another C function,
+  // gExtensionSingletonDictionarySemaphore and gExtensionSingletonDictionary
+  // will always be initialized. This is because this call flow is just to
+  // lookup the Extension, meaning the code is calling an Extension class
+  // message on a Message or Root class. This guarantees that the class was
+  // initialized and Message classes ensure their Root was also initialized.
+  NSAssert(gExtensionSingletonDictionary, @"Startup order broken!");
+
+  dispatch_semaphore_wait(gExtensionSingletonDictionarySemaphore,
+                          DISPATCH_TIME_FOREVER);
   id extension = (id)CFDictionaryGetValue(gExtensionSingletonDictionary, key);
   if (extension) {
     // The method is getting wired in to the class, so no need to keep it in
     // the dictionary.
     CFDictionaryRemoveValue(gExtensionSingletonDictionary, key);
   }
-  OSSpinLockUnlock(&gExtensionSingletonDictionaryLock_);
+  dispatch_semaphore_signal(gExtensionSingletonDictionarySemaphore);
   return extension;
 }
 
-- 
cgit v1.2.3