Skip to content

Commit

Permalink
Remove the use of dispatch_once that is heap backed.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
thomasvl authored and TeBoring committed Apr 4, 2017
1 parent 4f44a0f commit 7d758c0
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 17 deletions.
19 changes: 19 additions & 0 deletions objectivec/GPBMessage.m
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
18 changes: 1 addition & 17 deletions objectivec/GPBMessage_PackagePrivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ typedef struct GPBMessage_Storage *GPBMessage_StoragePtr;
// 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.
dispatch_once_t readOnlySemaphoreCreationOnce_;
dispatch_semaphore_t readOnlySemaphore_;
}

Expand Down Expand Up @@ -105,22 +104,7 @@ CF_EXTERN_C_BEGIN


// Call this before using the readOnlySemaphore_. This ensures it is created only once.
NS_INLINE void GPBPrepareReadOnlySemaphore(GPBMessage *self) {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdirect-ivar-access"

// Starting on Xcode 8.3, the static analyzer complains that the dispatch_once_t
// variable passed to dispatch_once should not be allocated on the heap or
// stack. Given that the semaphore is also an instance variable of the message,
// both variables are cleared at the same time, so this is safe.
#if !defined(__clang_analyzer__)
dispatch_once(&self->readOnlySemaphoreCreationOnce_, ^{
self->readOnlySemaphore_ = dispatch_semaphore_create(1);
});
#endif // !defined(__clang_analyzer__)

#pragma clang diagnostic pop
}
void GPBPrepareReadOnlySemaphore(GPBMessage *self);

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

0 comments on commit 7d758c0

Please sign in to comment.