1 files changed, 61 insertions, 29 deletions

diff --git a/objectivec/GPBMessage.m b/objectivec/GPBMessage.m
index 4a6f0612..90485bd1 100644
--- a/objectivec/GPBMessage.m
+++ b/objectivec/GPBMessage.m
@@ -130,7 +130,7 @@ static NSError *ErrorFromException(NSException *exception) {
 
 static void CheckExtension(GPBMessage *self,
                            GPBExtensionDescriptor *extension) {
-  if ([self class] != extension.containingMessageClass) {
+  if (![self isKindOfClass:extension.containingMessageClass]) {
     [NSException
          raise:NSInvalidArgumentException
         format:@"Extension %@ used on wrong class (%@ instead of %@)",
@@ -738,6 +738,25 @@ void GPBClearMessageAutocreator(GPBMessage *self) {
   self->autocreatorExtension_ = nil;
 }
 
+// Call this before using the readOnlySemaphore_. This ensures it is created only once.
+void GPBPrepareReadOnlySemaphore(GPBMessage *self) {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdirect-ivar-access"
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+
+  // Create the semaphore on demand (rather than init) as developers might not cause them
+  // to be needed, and the heap usage can add up.  The atomic swap is used to avoid needing
+  // another lock around creating it.
+  if (self->readOnlySemaphore_ == nil) {
+    dispatch_semaphore_t worker = dispatch_semaphore_create(1);
+    if (!OSAtomicCompareAndSwapPtrBarrier(NULL, worker, (void * volatile *)&(self->readOnlySemaphore_))) {
+      dispatch_release(worker);
+    }
+  }
+
+#pragma clang diagnostic pop
+}
+
 static GPBUnknownFieldSet *GetOrMakeUnknownFields(GPBMessage *self) {
   if (!self->unknownFields_) {
     self->unknownFields_ = [[GPBUnknownFieldSet alloc] init];
@@ -1023,9 +1042,11 @@ static GPBUnknownFieldSet *GetOrMakeUnknownFields(GPBMessage *self) {
       if (arrayOrMap) {
         if (field.fieldType == GPBFieldTypeRepeated) {
           if (GPBFieldDataTypeIsObject(field)) {
-            GPBAutocreatedArray *autoArray = arrayOrMap;
-            if (autoArray->_autocreator == self) {
-              autoArray->_autocreator = nil;
+            if ([arrayOrMap isKindOfClass:[GPBAutocreatedArray class]]) {
+              GPBAutocreatedArray *autoArray = arrayOrMap;
+              if (autoArray->_autocreator == self) {
+                autoArray->_autocreator = nil;
+              }
             }
           } else {
             // Type doesn't matter, it is a GPB*Array.
@@ -1037,9 +1058,11 @@ static GPBUnknownFieldSet *GetOrMakeUnknownFields(GPBMessage *self) {
         } else {
           if ((field.mapKeyDataType == GPBDataTypeString) &&
               GPBFieldDataTypeIsObject(field)) {
-            GPBAutocreatedDictionary *autoDict = arrayOrMap;
-            if (autoDict->_autocreator == self) {
-              autoDict->_autocreator = nil;
+            if ([arrayOrMap isKindOfClass:[GPBAutocreatedDictionary class]]) {
+              GPBAutocreatedDictionary *autoDict = arrayOrMap;
+              if (autoDict->_autocreator == self) {
+                autoDict->_autocreator = nil;
+              }
             }
           } else {
             // Type doesn't matter, it is a GPB*Dictionary.
@@ -2349,17 +2372,11 @@ static void MergeRepeatedNotPackedFieldFromCodedInputStream(
         // zero signals EOF / limit reached
         return;
       } else {
-        if (GPBPreserveUnknownFields(syntax)) {
-          if (![self parseUnknownField:input
-                     extensionRegistry:extensionRegistry
-                                   tag:tag]) {
-            // it's an endgroup tag
-            return;
-          }
-        } else {
-          if (![input skipField:tag]) {
-            return;
-          }
+        if (![self parseUnknownField:input
+                   extensionRegistry:extensionRegistry
+                                 tag:tag]) {
+          // it's an endgroup tag
+          return;
         }
       }
     }  // if(!merged)
@@ -2574,13 +2591,14 @@ static void MergeRepeatedNotPackedFieldFromCodedInputStream(
   if (other == self) {
     return YES;
   }
-  if (![other isKindOfClass:[self class]] &&
-      ![self isKindOfClass:[other class]]) {
+  if (![other isKindOfClass:[GPBMessage class]]) {
     return NO;
   }
-
   GPBMessage *otherMsg = other;
   GPBDescriptor *descriptor = [[self class] descriptor];
+  if ([[otherMsg class] descriptor] != descriptor) {
+    return NO;
+  }
   uint8_t *selfStorage = (uint8_t *)messageStorage_;
   uint8_t *otherStorage = (uint8_t *)otherMsg->messageStorage_;
 
@@ -2980,7 +2998,10 @@ typedef struct ResolveIvarAccessorMethodResult {
   SEL encodingSelector;
 } ResolveIvarAccessorMethodResult;
 
-static void ResolveIvarGet(GPBFieldDescriptor *field,
+// |field| can be __unsafe_unretained because they are created at startup
+// and are essentially global. No need to pay for retain/release when
+// they are captured in blocks.
+static void ResolveIvarGet(__unsafe_unretained GPBFieldDescriptor *field,
                            ResolveIvarAccessorMethodResult *result) {
   GPBDataType fieldDataType = GPBGetFieldDataType(field);
   switch (fieldDataType) {
@@ -3022,7 +3043,8 @@ static void ResolveIvarGet(GPBFieldDescriptor *field,
   }
 }
 
-static void ResolveIvarSet(GPBFieldDescriptor *field,
+// See comment about __unsafe_unretained on ResolveIvarGet.
+static void ResolveIvarSet(__unsafe_unretained GPBFieldDescriptor *field,
                            GPBFileSyntax syntax,
                            ResolveIvarAccessorMethodResult *result) {
   GPBDataType fieldDataType = GPBGetFieldDataType(field);
@@ -3060,15 +3082,16 @@ static void ResolveIvarSet(GPBFieldDescriptor *field,
 + (BOOL)resolveInstanceMethod:(SEL)sel {
   const GPBDescriptor *descriptor = [self descriptor];
   if (!descriptor) {
-    return NO;
+    return [super resolveInstanceMethod:sel];
   }
 
   // NOTE: hasOrCountSel_/setHasSel_ will be NULL if the field for the given
   // message should not have has support (done in GPBDescriptor.m), so there is
   // no need for checks here to see if has*/setHas* are allowed.
-
   ResolveIvarAccessorMethodResult result = {NULL, NULL};
-  for (GPBFieldDescriptor *field in descriptor->fields_) {
+
+  // See comment about __unsafe_unretained on ResolveIvarGet.
+  for (__unsafe_unretained GPBFieldDescriptor *field in descriptor->fields_) {
     BOOL isMapOrArray = GPBFieldIsMapOrArray(field);
     if (!isMapOrArray) {
       // Single fields.
@@ -3148,8 +3171,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];
@@ -3157,7 +3189,7 @@ static void ResolveIvarSet(GPBFieldDescriptor *field,
 
 + (BOOL)resolveClassMethod:(SEL)sel {
   // Extensions scoped to a Message and looked up via class methods.
-  if (GPBResolveExtensionClassMethod(self, sel)) {
+  if (GPBResolveExtensionClassMethod([self descriptor].messageClass, sel)) {
     return YES;
   }
   return [super resolveClassMethod:sel];