Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CFRunLoopPerformBlock never executes #531

Closed
aquigriffin opened this issue Nov 11, 2016 · 10 comments
Closed

CFRunLoopPerformBlock never executes #531

aquigriffin opened this issue Nov 11, 2016 · 10 comments
Assignees

Comments

@aquigriffin
Copy link

In ARTHttp.m, starting at line 175. CFRunLoopRef fails to perform block if CurrentRunLoopRef ignores wakeup. Reproduced with performing token request on application startup.

@tcard tcard self-assigned this Nov 14, 2016
@ricardopereira
Copy link
Contributor

@aquigriffin Could you add a short runnable sample to reproduce this issue?

@ricardopereira
Copy link
Contributor

Even so, I prefer using GCD instead of run loops. It's much safer. No need to deal with run loop management and we can avoid issues like this.

@aquigriffin
Copy link
Author

aquigriffin commented Nov 15, 2016

@ricardopereira Here is an example of my ClientOptions authCallback:

__weak typeof(self) weakSelf = self;
   ARTClientOptions *clientOptions = [[ARTClientOptions alloc] init];
   clientOptions.authCallback = ^(ARTTokenParams *params, void(^callback)(id<ARTTokenDetailsCompatible>, NSError*)) {
       [_interaction startTaskOnSuccess:^(OUPAblyTokenRequestResponseDTO *_Nullable responseDTO) {
           [weakSelf updateChannelNamesWithResponseDTO:responseDTO];
           ARTTokenRequest *tokenRequest = [self tokenRequestFromTokenRequestResponseDTO:responseDTO];
           callback(tokenRequest,nil);
       } onFailure:^(NSError *_Nullable error){
           callback(nil,error);
       }];
   };

where _interaction startTaskOnSuccess is a call to our services for the token_request parameters.

The client is being created on AppDelegate applicationDidLaunchWithOptions: with the following code:

`- (void)configureClient {
_client = [[ARTRealtime alloc] initWithOptions:[self clientOptions]];

[_client.auth.logger debugMode];
[_client.auth.logger errorMode];
[_client.auth.logger verboseMode];

__weak typeof(self) weakSelf = self;
[_client.connection on:^(ARTConnectionStateChange *stateChange) {
    [weakSelf handleConnectionStateChange:stateChange];
}];

}`

If I force the callback(ARTTokenRequest *, NSError *) in clientOptions.authCallback to be run on the main thread, the callback will perform the block at line 180 in ARTHttp.m, and connect the client. If I am not running on main the block will not fire. There may be an issue with the connection retry happening before the callback can fire if not on the main thread, but I have not been able to verify this. I have verified I can repro this on device and simulator ios9.0+, iphone6/6s.

@ricardopereira
Copy link
Contributor

@aquigriffin Thanks for the example. It makes sense what you said. The API reference makes this clear. It says that we shouldn't call the methods of an NSRunLoop object running in a different thread.

The NSRunLoop class is generally not considered to be thread-safe and its methods should only be called within the context of the current thread. You should never try to call the methods of an NSRunLoop object running in a different thread, as doing so might cause unexpected results.

I will have a look into this.

@ricardopereira
Copy link
Contributor

@aquigriffin Can you test it using pod 'Ably', :git => 'https://github.com/ably/ably-ios.git', :branch => 'fix-531'? Thanks.

@ricardopereira
Copy link
Contributor

ricardopereira commented Nov 18, 2016

Basically, if the problem is the authCallback isn't transition the error or UI updates doesn't work in the callback, then the #536 solves that problem. If not, then the problem is related with a race condition: the client.connection.on listener is not ready when the authCallback is called. Try to set the autoConnect to false and see if it works.

@implementation AppDelegate {
    ARTClientOptions *_clientOptions;
    ARTRealtime *_client;
}

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions {
    _clientOptions = [[ARTClientOptions alloc] init];
    _clientOptions.autoConnect = false;
    _clientOptions.authCallback = ^(ARTTokenParams *params, void(^callback)(id<ARTTokenDetailsCompatible>, NSError*)) {
        callback(nil, [NSError errorWithDomain:@"localhost" code:0 userInfo:nil]);
    };
    _client = [[ARTRealtime alloc] initWithOptions:_clientOptions];
    [_client.connection on:^(ARTConnectionStateChange *stateChange) {
        NSLog(@"%@", stateChange);
    }];
    [_client connect];
    return YES;
}

@aquigriffin
Copy link
Author

@ricardopereira I checked out fix-531 and tested connection with and without the auto-connect setting 'on'. I found I was getting an exception EXC_BAD_ACCESS on

if (*((int*)dispatch_get_specific((__bridge const void *)(ARTGCDMainQueueKey))) == 99)

Reading the description of dispatch_get_specific() says:
*@discussion

  • When called from a block executing on a queue, returns the context for the
  • specified key if it has been set on the queue, otherwise returns the result
  • of dispatch_get_specific() executed on the queue's target queue or NULL
  • if the current queue is a global concurrent queue.

If we get NULL this would cause the error I'm seeing. You should check for null before checking equality. i.e.

int* result = ((int*)dispatch_get_specific((__bridge const void *)(ARTGCDMainQueueKey))); if (result != NULL && *result == 99) {

That said, using this new code with my minor change does indeed ensure the authCallback's callback gets fired.

@ricardopereira
Copy link
Contributor

@aquigriffin Damn 😅 I forgot the NULL case. I'll fix it right away.

@ricardopereira
Copy link
Contributor

@aquigriffin Fixed. Try again please and don't forget the pod update.

@aquigriffin
Copy link
Author

@ricardopereira works fine, thanks for the quick response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants