[ObjC] Use os_unfair_lock instead of dispatch_semaphore_t.
The minOS version is high enough it can be used, and it avoids the long standing
issue for priority inversion dependent on what callers did with
threading/queues.
diff --git a/objectivec/GPBRootObject.m b/objectivec/GPBRootObject.m
index 08adfcf..e11bb4c 100644
--- a/objectivec/GPBRootObject.m
+++ b/objectivec/GPBRootObject.m
@@ -31,6 +31,7 @@
#import "GPBRootObject_PackagePrivate.h"
#import <objc/runtime.h>
+#import <os/lock.h>
#import <CoreFoundation/CoreFoundation.h>
@@ -96,19 +97,24 @@
return jenkins_one_at_a_time_hash(key);
}
-// 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:
+// Long ago, this was a OSSpinLock, but then it came to light that there were issues for that on
+// iOS:
// 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;
+// So it was a dispatch_semaphore_t, but that has issues with priority inversion if developer code
+// uses queue for different things. But the minOS versions are now high enough, os_unfair_lock and
+// be used, and should provide all the support needed. For more information in the
+// concurrence/locking space see:
+// https://gist.github.com/tclementdev/6af616354912b0347cdf6db159c37057
+// https://developer.apple.com/library/archive/documentation/Performance/Conceptual/EnergyGuide-iOS/PrioritizeWorkWithQoS.html
+// https://developer.apple.com/videos/play/wwdc2017/706/
+static os_unfair_lock gExtensionSingletonDictionaryLock = OS_UNFAIR_LOCK_INIT;
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,
@@ -139,9 +145,9 @@
+ (void)globallyRegisterExtension:(GPBExtensionDescriptor *)field {
const char *key = [field singletonNameC];
- dispatch_semaphore_wait(gExtensionSingletonDictionarySemaphore, DISPATCH_TIME_FOREVER);
+ os_unfair_lock_lock(&gExtensionSingletonDictionaryLock);
CFDictionarySetValue(gExtensionSingletonDictionary, key, field);
- dispatch_semaphore_signal(gExtensionSingletonDictionarySemaphore);
+ os_unfair_lock_unlock(&gExtensionSingletonDictionaryLock);
}
static id ExtensionForName(id self, SEL _cmd) {
@@ -173,20 +179,20 @@
key[classNameLen + 1 + selNameLen] = '\0';
// NOTE: Even though this method is called from another C function,
- // gExtensionSingletonDictionarySemaphore and gExtensionSingletonDictionary
+ // gExtensionSingletonDictionaryLock 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);
+ os_unfair_lock_lock(&gExtensionSingletonDictionaryLock);
id extension = (id)CFDictionaryGetValue(gExtensionSingletonDictionary, key);
// We can't remove the key from the dictionary here (as an optimization),
// two threads could have gone into +resolveClassMethod: for the same method,
// and ended up here; there's no way to ensure both return YES without letting
// both try to wire in the method.
- dispatch_semaphore_signal(gExtensionSingletonDictionarySemaphore);
+ os_unfair_lock_unlock(&gExtensionSingletonDictionaryLock);
return extension;
}
