Skip to content

Commit 130c166

Browse files
committed
Remove the use of dispatch_once that is heap backed.
Apple recently updated the docs on dispatch_once to point out that the storage for the dispatch_once_t must be static or global, but not something that was ever used before as the implementation doesn't use a memory barrier. So we drop the use and create the semaphore when needed and use an atomic swap deal with any threading races.
1 parent ba3fa41 commit 130c166

File tree

2 files changed

+20
-17
lines changed

2 files changed

+20
-17
lines changed

objectivec/GPBMessage.m

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,25 @@ void GPBClearMessageAutocreator(GPBMessage *self) {
738738
self->autocreatorExtension_ = nil;
739739
}
740740

741+
// Call this before using the readOnlySemaphore_. This ensures it is created only once.
742+
void GPBPrepareReadOnlySemaphore(GPBMessage *self) {
743+
#pragma clang diagnostic push
744+
#pragma clang diagnostic ignored "-Wdirect-ivar-access"
745+
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
746+
747+
// Create the semaphore on demand (rather than init) as developers might not cause them
748+
// to be needed, and the heap usage can add up. The atomic swap is used to avoid needing
749+
// another lock around creating it.
750+
if (self->readOnlySemaphore_ == nil) {
751+
dispatch_semaphore_t worker = dispatch_semaphore_create(1);
752+
if (!OSAtomicCompareAndSwapPtrBarrier(NULL, worker, (void * volatile *)&(self->readOnlySemaphore_))) {
753+
dispatch_release(worker);
754+
}
755+
}
756+
757+
#pragma clang diagnostic pop
758+
}
759+
741760
static GPBUnknownFieldSet *GetOrMakeUnknownFields(GPBMessage *self) {
742761
if (!self->unknownFields_) {
743762
self->unknownFields_ = [[GPBUnknownFieldSet alloc] init];

objectivec/GPBMessage_PackagePrivate.h

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ typedef struct GPBMessage_Storage *GPBMessage_StoragePtr;
7070
// Use of readOnlySemaphore_ must be prefaced by a call to
7171
// GPBPrepareReadOnlySemaphore to ensure it has been created. This allows
7272
// readOnlySemaphore_ to be only created when actually needed.
73-
dispatch_once_t readOnlySemaphoreCreationOnce_;
7473
dispatch_semaphore_t readOnlySemaphore_;
7574
}
7675

@@ -105,22 +104,7 @@ CF_EXTERN_C_BEGIN
105104

106105

107106
// Call this before using the readOnlySemaphore_. This ensures it is created only once.
108-
NS_INLINE void GPBPrepareReadOnlySemaphore(GPBMessage *self) {
109-
#pragma clang diagnostic push
110-
#pragma clang diagnostic ignored "-Wdirect-ivar-access"
111-
112-
// Starting on Xcode 8.3, the static analyzer complains that the dispatch_once_t
113-
// variable passed to dispatch_once should not be allocated on the heap or
114-
// stack. Given that the semaphore is also an instance variable of the message,
115-
// both variables are cleared at the same time, so this is safe.
116-
#if !defined(__clang_analyzer__)
117-
dispatch_once(&self->readOnlySemaphoreCreationOnce_, ^{
118-
self->readOnlySemaphore_ = dispatch_semaphore_create(1);
119-
});
120-
#endif // !defined(__clang_analyzer__)
121-
122-
#pragma clang diagnostic pop
123-
}
107+
void GPBPrepareReadOnlySemaphore(GPBMessage *self);
124108

125109
// Returns a new instance that was automatically created by |autocreator| for
126110
// its field |field|.

0 commit comments

Comments
 (0)