Skip to content

Commit

Permalink
Synchronise access to shared ARTLocalDevice variable
Browse files Browse the repository at this point in the history
We currently read / write the variable that stores the shared
ARTLocalDevice variable from the internal queue of the ARTRest instance
through which the device is being accessed. But, since each ARTRest
instance has its own queue, this means that access to this variable
isn’t currently synchronised between multiple ARTRest instances.

So, here we do that.

This also allows us to remove our usage of dispatch_once. This is a good
thing, because we were using it incorrectly before. From an Apple
engineer on https://stackoverflow.com/a/19845164:

> The implementation of dispatch_once() requires that the
> dispatch_once_t is zero, and has never been non-zero.

That is, you can’t reset a dispatch_once_t variable and expect
dispatch_once to still work with it, which is what we were doing.
  • Loading branch information
lawrence-forooghian committed May 4, 2022
1 parent 2e7b8b0 commit fbd5645
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 9 deletions.
1 change: 1 addition & 0 deletions Source/ARTRest+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ NS_ASSUME_NONNULL_BEGIN
- (nullable NSObject<ARTCancellable> *)internetIsUp:(void (^)(BOOL isUp))cb;

#if TARGET_OS_IOS
// This is only intended to be called from test code.
- (void)resetDeviceSingleton;
#endif

Expand Down
37 changes: 28 additions & 9 deletions Source/ARTRest.m
Original file line number Diff line number Diff line change
Expand Up @@ -724,10 +724,28 @@ - (ARTLocalDevice *)device {
return ret;
}

// Store address of once_token to access it in debug function.
static dispatch_once_t *device_once_token;

- (ARTLocalDevice *)device_nosync {
__block ARTLocalDevice *ret;
dispatch_sync(ARTRestInternal.deviceAccessQueue, ^{
ret = [self deviceWithClientId_onlyCallOnDeviceAccessQueue:self.auth.clientId_nosync];
});
return ret;
}

+ (dispatch_queue_t)deviceAccessQueue {
static dispatch_queue_t queue;
static dispatch_once_t onceToken;

dispatch_once(&onceToken, ^{
queue = dispatch_queue_create("io.ably.deviceAccess", DISPATCH_QUEUE_SERIAL);
});

return queue;
}

static BOOL sharedDeviceNeedsLoading_onlyAccessOnDeviceAccessQueue = YES;

- (ARTLocalDevice *)deviceWithClientId_onlyCallOnDeviceAccessQueue:(NSString *)clientId {
// The device is shared in a static variable because it's a reflection
// of what's persisted. Having a device instance per ARTRest instance
// could leave some instances in a stale state, if, through another
Expand All @@ -736,17 +754,18 @@ - (ARTLocalDevice *)device_nosync {
// As a side effect, the first instance "wins" at setting the device's
// client ID.

static dispatch_once_t once;
device_once_token = &once;
static id device;
dispatch_once(&once, ^{
device = [ARTLocalDevice load:self.auth.clientId_nosync storage:self.storage];
});
if (sharedDeviceNeedsLoading_onlyAccessOnDeviceAccessQueue) {
device = [ARTLocalDevice load:clientId storage:self.storage];
sharedDeviceNeedsLoading_onlyAccessOnDeviceAccessQueue = NO;
}
return device;
}

- (void)resetDeviceSingleton {
if (device_once_token) *device_once_token = 0;
dispatch_sync([ARTRestInternal deviceAccessQueue], ^{
sharedDeviceNeedsLoading_onlyAccessOnDeviceAccessQueue = YES;
});
}
#endif

Expand Down

0 comments on commit fbd5645

Please sign in to comment.