Handing threading race resolving methods.
- Don't prune the extension registry as that can lead to failures when two threads are racing. - If adding the method fails, check and see if it already is bound to decide the return result. Deals with threading races binding the methods.
This commit is contained in:
parent
a7e3b0ab01
commit
2d1c5e26cb
@ -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];
|
||||
|
@ -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;
|
||||
}
|
||||
|
@ -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
|
||||
|
@ -345,4 +345,6 @@ GPB_MESSAGE_SIGNATURE_ENTRY(int32_t, Enum)
|
||||
+ (id)getClassValue;
|
||||
@end
|
||||
|
||||
BOOL GPBClassHasSel(Class aClass, SEL sel);
|
||||
|
||||
CF_EXTERN_C_END
|
||||
|
Loading…
Reference in New Issue
Block a user