Skip to content

Commit cf016a4

Browse files
committed
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_.
1 parent d570d48 commit cf016a4

File tree

3 files changed

+42
-42
lines changed

3 files changed

+42
-42
lines changed

objectivec/GPBMessage.m

+42
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,20 @@ @interface GPBMessage () {
7878
GPBMessage *autocreator_;
7979
GPBFieldDescriptor *autocreatorField_;
8080
GPBExtensionDescriptor *autocreatorExtension_;
81+
82+
// A lock to provide mutual exclusion from internal data that can be modified
83+
// by *read* operations such as getters (autocreation of message fields and
84+
// message extensions, not setting of values). Used to guarantee thread safety
85+
// for concurrent reads on the message.
86+
// NOTE: OSSpinLock may seem like a good fit here but Apple engineers have
87+
// pointed out that they are vulnerable to live locking on iOS in cases of
88+
// priority inversion:
89+
// http://mjtsai.com/blog/2015/12/16/osspinlock-is-unsafe/
90+
// https://lists.swift.org/pipermail/swift-dev/Week-of-Mon-20151214/000372.html
91+
// Use of readOnlySemaphore_ must be prefaced by a call to
92+
// GPBPrepareReadOnlySemaphore to ensure it has been created. This allows
93+
// readOnlySemaphore_ to be only created when actually needed.
94+
_Atomic(dispatch_semaphore_t) readOnlySemaphore_;
8195
}
8296
@end
8397

@@ -3272,4 +3286,32 @@ id GPBGetMessageMapField(GPBMessage *self, GPBFieldDescriptor *field) {
32723286
return GetOrCreateMapIvarWithField(self, field, syntax);
32733287
}
32743288

3289+
id GPBGetObjectIvarWithField(GPBMessage *self, GPBFieldDescriptor *field) {
3290+
NSCAssert(!GPBFieldIsMapOrArray(field), @"Shouldn't get here");
3291+
if (GPBGetHasIvarField(self, field)) {
3292+
uint8_t *storage = (uint8_t *)self->messageStorage_;
3293+
id *typePtr = (id *)&storage[field->description_->offset];
3294+
return *typePtr;
3295+
}
3296+
// Not set...
3297+
3298+
// Non messages (string/data), get their default.
3299+
if (!GPBFieldDataTypeIsMessage(field)) {
3300+
return field.defaultValue.valueMessage;
3301+
}
3302+
3303+
GPBPrepareReadOnlySemaphore(self);
3304+
dispatch_semaphore_wait(self->readOnlySemaphore_, DISPATCH_TIME_FOREVER);
3305+
GPBMessage *result = GPBGetObjectIvarWithFieldNoAutocreate(self, field);
3306+
if (!result) {
3307+
// For non repeated messages, create the object, set it and return it.
3308+
// This object will not initially be visible via GPBGetHasIvar, so
3309+
// we save its creator so it can become visible if it's mutated later.
3310+
result = GPBCreateMessageWithAutocreator(field.msgClass, self, field);
3311+
GPBSetAutocreatedRetainedObjectIvarWithField(self, field, result);
3312+
}
3313+
dispatch_semaphore_signal(self->readOnlySemaphore_);
3314+
return result;
3315+
}
3316+
32753317
#pragma clang diagnostic pop

objectivec/GPBMessage_PackagePrivate.h

-14
Original file line numberDiff line numberDiff line change
@@ -61,20 +61,6 @@ typedef struct GPBMessage_Storage *GPBMessage_StoragePtr;
6161
// GPBMessage_Storage with _has_storage__ as the first field.
6262
// Kept public because static functions need to access it.
6363
GPBMessage_StoragePtr messageStorage_;
64-
65-
// A lock to provide mutual exclusion from internal data that can be modified
66-
// by *read* operations such as getters (autocreation of message fields and
67-
// message extensions, not setting of values). Used to guarantee thread safety
68-
// for concurrent reads on the message.
69-
// NOTE: OSSpinLock may seem like a good fit here but Apple engineers have
70-
// pointed out that they are vulnerable to live locking on iOS in cases of
71-
// priority inversion:
72-
// http://mjtsai.com/blog/2015/12/16/osspinlock-is-unsafe/
73-
// https://lists.swift.org/pipermail/swift-dev/Week-of-Mon-20151214/000372.html
74-
// Use of readOnlySemaphore_ must be prefaced by a call to
75-
// GPBPrepareReadOnlySemaphore to ensure it has been created. This allows
76-
// readOnlySemaphore_ to be only created when actually needed.
77-
_Atomic(dispatch_semaphore_t) readOnlySemaphore_;
7864
}
7965

8066
// Gets an extension value without autocreating the result if not found. (i.e.

objectivec/GPBUtilities.m

-28
Original file line numberDiff line numberDiff line change
@@ -628,34 +628,6 @@ id GPBGetObjectIvarWithFieldNoAutocreate(GPBMessage *self,
628628
return *typePtr;
629629
}
630630

631-
id GPBGetObjectIvarWithField(GPBMessage *self, GPBFieldDescriptor *field) {
632-
NSCAssert(!GPBFieldIsMapOrArray(field), @"Shouldn't get here");
633-
if (GPBGetHasIvarField(self, field)) {
634-
uint8_t *storage = (uint8_t *)self->messageStorage_;
635-
id *typePtr = (id *)&storage[field->description_->offset];
636-
return *typePtr;
637-
}
638-
// Not set...
639-
640-
// Non messages (string/data), get their default.
641-
if (!GPBFieldDataTypeIsMessage(field)) {
642-
return field.defaultValue.valueMessage;
643-
}
644-
645-
GPBPrepareReadOnlySemaphore(self);
646-
dispatch_semaphore_wait(self->readOnlySemaphore_, DISPATCH_TIME_FOREVER);
647-
GPBMessage *result = GPBGetObjectIvarWithFieldNoAutocreate(self, field);
648-
if (!result) {
649-
// For non repeated messages, create the object, set it and return it.
650-
// This object will not initially be visible via GPBGetHasIvar, so
651-
// we save its creator so it can become visible if it's mutated later.
652-
result = GPBCreateMessageWithAutocreator(field.msgClass, self, field);
653-
GPBSetAutocreatedRetainedObjectIvarWithField(self, field, result);
654-
}
655-
dispatch_semaphore_signal(self->readOnlySemaphore_);
656-
return result;
657-
}
658-
659631
// Only exists for public api, no core code should use this.
660632
int32_t GPBGetMessageEnumField(GPBMessage *self, GPBFieldDescriptor *field) {
661633
GPBFileSyntax syntax = [self descriptor].file.syntax;

0 commit comments

Comments
 (0)