ソースを参照

Work around strange error with atomic and swift under Xcode 8.3.3.

Haven't been able to make a repo case, but this should "fix" the problem
by avoid it completely.

- Move readOnlySemaphore_ into the .m file so it isn't exposed in any
  header.
- Move GPBGetObjectIvarWithField() also to go with the new limited
  visibility on the readOnlySemaphore_.
Thomas Van Lenten 7 年 前
コミット
cf016a42e6
3 ファイル変更42 行追加42 行削除
  1. 42 0
      objectivec/GPBMessage.m
  2. 0 14
      objectivec/GPBMessage_PackagePrivate.h
  3. 0 28
      objectivec/GPBUtilities.m

+ 42 - 0
objectivec/GPBMessage.m

@@ -78,6 +78,20 @@ static NSString *const kGPBDataCoderKey = @"GPBData";
   GPBMessage *autocreator_;
   GPBFieldDescriptor *autocreatorField_;
   GPBExtensionDescriptor *autocreatorExtension_;
+
+  // A lock to provide mutual exclusion from internal data that can be modified
+  // by *read* operations such as getters (autocreation of message fields and
+  // message extensions, not setting of values). Used to guarantee thread safety
+  // for concurrent reads on the message.
+  // 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
+  // Use of readOnlySemaphore_ must be prefaced by a call to
+  // GPBPrepareReadOnlySemaphore to ensure it has been created. This allows
+  // readOnlySemaphore_ to be only created when actually needed.
+  _Atomic(dispatch_semaphore_t) readOnlySemaphore_;
 }
 @end
 
@@ -3272,4 +3286,32 @@ id GPBGetMessageMapField(GPBMessage *self, GPBFieldDescriptor *field) {
   return GetOrCreateMapIvarWithField(self, field, syntax);
 }
 
+id GPBGetObjectIvarWithField(GPBMessage *self, GPBFieldDescriptor *field) {
+  NSCAssert(!GPBFieldIsMapOrArray(field), @"Shouldn't get here");
+  if (GPBGetHasIvarField(self, field)) {
+    uint8_t *storage = (uint8_t *)self->messageStorage_;
+    id *typePtr = (id *)&storage[field->description_->offset];
+    return *typePtr;
+  }
+  // Not set...
+
+  // Non messages (string/data), get their default.
+  if (!GPBFieldDataTypeIsMessage(field)) {
+    return field.defaultValue.valueMessage;
+  }
+
+  GPBPrepareReadOnlySemaphore(self);
+  dispatch_semaphore_wait(self->readOnlySemaphore_, DISPATCH_TIME_FOREVER);
+  GPBMessage *result = GPBGetObjectIvarWithFieldNoAutocreate(self, field);
+  if (!result) {
+    // For non repeated messages, create the object, set it and return it.
+    // This object will not initially be visible via GPBGetHasIvar, so
+    // we save its creator so it can become visible if it's mutated later.
+    result = GPBCreateMessageWithAutocreator(field.msgClass, self, field);
+    GPBSetAutocreatedRetainedObjectIvarWithField(self, field, result);
+  }
+  dispatch_semaphore_signal(self->readOnlySemaphore_);
+  return result;
+}
+
 #pragma clang diagnostic pop

+ 0 - 14
objectivec/GPBMessage_PackagePrivate.h

@@ -61,20 +61,6 @@ typedef struct GPBMessage_Storage *GPBMessage_StoragePtr;
   // GPBMessage_Storage with _has_storage__ as the first field.
   // Kept public because static functions need to access it.
   GPBMessage_StoragePtr messageStorage_;
-
-  // A lock to provide mutual exclusion from internal data that can be modified
-  // by *read* operations such as getters (autocreation of message fields and
-  // message extensions, not setting of values). Used to guarantee thread safety
-  // for concurrent reads on the message.
-  // 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
-  // Use of readOnlySemaphore_ must be prefaced by a call to
-  // GPBPrepareReadOnlySemaphore to ensure it has been created. This allows
-  // readOnlySemaphore_ to be only created when actually needed.
-  _Atomic(dispatch_semaphore_t) readOnlySemaphore_;
 }
 
 // Gets an extension value without autocreating the result if not found. (i.e.

+ 0 - 28
objectivec/GPBUtilities.m

@@ -628,34 +628,6 @@ id GPBGetObjectIvarWithFieldNoAutocreate(GPBMessage *self,
   return *typePtr;
 }
 
-id GPBGetObjectIvarWithField(GPBMessage *self, GPBFieldDescriptor *field) {
-  NSCAssert(!GPBFieldIsMapOrArray(field), @"Shouldn't get here");
-  if (GPBGetHasIvarField(self, field)) {
-    uint8_t *storage = (uint8_t *)self->messageStorage_;
-    id *typePtr = (id *)&storage[field->description_->offset];
-    return *typePtr;
-  }
-  // Not set...
-
-  // Non messages (string/data), get their default.
-  if (!GPBFieldDataTypeIsMessage(field)) {
-    return field.defaultValue.valueMessage;
-  }
-
-  GPBPrepareReadOnlySemaphore(self);
-  dispatch_semaphore_wait(self->readOnlySemaphore_, DISPATCH_TIME_FOREVER);
-  GPBMessage *result = GPBGetObjectIvarWithFieldNoAutocreate(self, field);
-  if (!result) {
-    // For non repeated messages, create the object, set it and return it.
-    // This object will not initially be visible via GPBGetHasIvar, so
-    // we save its creator so it can become visible if it's mutated later.
-    result = GPBCreateMessageWithAutocreator(field.msgClass, self, field);
-    GPBSetAutocreatedRetainedObjectIvarWithField(self, field, result);
-  }
-  dispatch_semaphore_signal(self->readOnlySemaphore_);
-  return result;
-}
-
 // Only exists for public api, no core code should use this.
 int32_t GPBGetMessageEnumField(GPBMessage *self, GPBFieldDescriptor *field) {
   GPBFileSyntax syntax = [self descriptor].file.syntax;