aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar Thomas Van Lenten <thomasvl@google.com>2017-03-02 17:14:52 -0500
committerGravatar GitHub <noreply@github.com>2017-03-02 17:14:52 -0500
commit352526c2149f88c229e229013184a136b1414ab0 (patch)
tree1486c2e0c6ffa3e5121f00096b27b52450f5a395
parenta7e3b0ab016312f7427426d2cb88b0ab77210b79 (diff)
parent2d1c5e26cb291193ac4d1f4a5179d8f6e1906229 (diff)
Merge pull request #2785 from thomasvl/threading_race
Handing threading race resolving methods.
-rw-r--r--objectivec/GPBMessage.m13
-rw-r--r--objectivec/GPBRootObject.m21
-rw-r--r--objectivec/GPBUtilities.m19
-rw-r--r--objectivec/GPBUtilities_PackagePrivate.h2
4 files changed, 46 insertions, 9 deletions
diff --git a/objectivec/GPBMessage.m b/objectivec/GPBMessage.m
index 9660f1ed..58a10fdb 100644
--- a/objectivec/GPBMessage.m
+++ b/objectivec/GPBMessage.m
@@ -3152,8 +3152,17 @@ static void ResolveIvarSet(GPBFieldDescriptor *field,
if (result.impToAdd) {
const char *encoding =
GPBMessageEncodingForSelector(result.encodingSelector, YES);
- BOOL methodAdded = class_addMethod(descriptor.messageClass, sel,
- result.impToAdd, encoding);
+ Class msgClass = descriptor.messageClass;
+ BOOL methodAdded = class_addMethod(msgClass, sel, result.impToAdd, encoding);
+ // class_addMethod() is documented as also failing if the method was already
+ // added; so we check if the method is already there and return success so
+ // the method dispatch will still happen. Why would it already be added?
+ // Two threads could cause the same method to be bound at the same time,
+ // but only one will actually bind it; the other still needs to return true
+ // so things will dispatch.
+ if (!methodAdded) {
+ methodAdded = GPBClassHasSel(msgClass, sel);
+ }
return methodAdded;
}
return [super resolveInstanceMethod:sel];
diff --git a/objectivec/GPBRootObject.m b/objectivec/GPBRootObject.m
index 4570716f..585d205a 100644
--- a/objectivec/GPBRootObject.m
+++ b/objectivec/GPBRootObject.m
@@ -184,11 +184,10 @@ static id ExtensionForName(id self, SEL _cmd) {
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);
- }
+ // 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);
return extension;
}
@@ -212,9 +211,17 @@ BOOL GPBResolveExtensionClassMethod(Class self, SEL sel) {
#pragma unused(obj)
return extension;
});
- if (class_addMethod(metaClass, sel, imp, encoding)) {
- return YES;
+ BOOL methodAdded = class_addMethod(metaClass, sel, imp, encoding);
+ // class_addMethod() is documented as also failing if the method was already
+ // added; so we check if the method is already there and return success so
+ // the method dispatch will still happen. Why would it already be added?
+ // Two threads could cause the same method to be bound at the same time,
+ // but only one will actually bind it; the other still needs to return true
+ // so things will dispatch.
+ if (!methodAdded) {
+ methodAdded = GPBClassHasSel(metaClass, sel);
}
+ return methodAdded;
}
return NO;
}
diff --git a/objectivec/GPBUtilities.m b/objectivec/GPBUtilities.m
index 1843478c..5029ec73 100644
--- a/objectivec/GPBUtilities.m
+++ b/objectivec/GPBUtilities.m
@@ -1893,6 +1893,25 @@ NSString *GPBDecodeTextFormatName(const uint8_t *decodeData, int32_t key,
#pragma clang diagnostic pop
+BOOL GPBClassHasSel(Class aClass, SEL sel) {
+ // NOTE: We have to use class_copyMethodList, all other runtime method
+ // lookups actually also resolve the method implementation and this
+ // is called from within those methods.
+
+ BOOL result = NO;
+ unsigned int methodCount = 0;
+ Method *methodList = class_copyMethodList(aClass, &methodCount);
+ for (unsigned int i = 0; i < methodCount; ++i) {
+ SEL methodSelector = method_getName(methodList[i]);
+ if (methodSelector == sel) {
+ result = YES;
+ break;
+ }
+ }
+ free(methodList);
+ return result;
+}
+
#pragma mark - GPBMessageSignatureProtocol
// A series of selectors that are used solely to get @encoding values
diff --git a/objectivec/GPBUtilities_PackagePrivate.h b/objectivec/GPBUtilities_PackagePrivate.h
index 274351b7..16859d48 100644
--- a/objectivec/GPBUtilities_PackagePrivate.h
+++ b/objectivec/GPBUtilities_PackagePrivate.h
@@ -345,4 +345,6 @@ GPB_MESSAGE_SIGNATURE_ENTRY(int32_t, Enum)
+ (id)getClassValue;
@end
+BOOL GPBClassHasSel(Class aClass, SEL sel);
+
CF_EXTERN_C_END