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

Crash in -[RZBPeripheral cancelAllCommands] due to infinite loop #98

Open
joshuatbrown opened this issue Jan 21, 2019 · 4 comments
Open

Comments

@joshuatbrown
Copy link

Issue

In the completion block of -[RZBPeripheral connectWithCompletion:], call -[RZBPeripheral cancelConnectionWithCompletion:]. It'll create an infinite loop and eventually crash with an EXC_BAD_ACCESS.

For example:

- (void)testCancelConnectionInConnectCompletion {
    RZBPeripheral *peripheral = [self.centralManager peripheralForUUID:self.connection.identifier];

    [peripheral connectWithCompletion:^(NSError * _Nullable error) {
        [peripheral cancelConnectionWithCompletion:nil];
    }];
    
    [self waitForQueueFlush];
}
@joshuatbrown
Copy link
Author

Unfortunately, our fix for this causes a different RZBluetooth test to fail. Here's our fix in RZBPeripheral:

- (void)cancelAllCommands
{
    NSError *error = [NSError errorWithDomain:RZBluetoothErrorDomain
                                         code:RZBluetoothConnectionCancelled
                                     userInfo:@{}];

    NSArray *commands = self.dispatch.commands.copy;
    for (RZBCommand *command in commands) {
        if (command.isExecuted == YES && command.isCompleted == NO) {
            // We are in the completion handler of this command. Do not complete it or we will loop infinitely.
            continue;
        }

        [self.dispatch completeCommand:command
                            withObject:nil error:error];
    }
}

This fixes the test case above, but it causes -[RZBSimulatedTests testConnectionAndCancelWhileNotConnectable] to fail, and I'm not sure why. Can someone explain the purpose of that test and why it fails with this fix?

@cpatterson-lilly
Copy link
Contributor

@joshuatbrown It seems that every access to RZBCommandDispatch.commands must be @synchronized.... Try replacing self.dispatch.commands.copy with a call to the (currently internal) method RZBDispatch.synchronizedCommandsCopy(), or wrapping that line in an @synchronized block?

@joshuatbrown
Copy link
Author

@cpatterson-lilly I replaced my NSArray *commands = ... line above with the following:

NSArray *commands;
@synchronized(self.dispatch.commands) {
    commands = [self.dispatch.commands copy];
}

The same test still fails for the same reason. I can leave the @synchronized if that's the right way, but it looks like we still need to figure out how to make the test pass.

joshuatbrown added a commit to e-gineering/RZBluetooth that referenced this issue Jan 23, 2019
@joshuatbrown
Copy link
Author

You can see my branch/commit above where I added the unit test and fixed the crash. But I'm still seeing the failure in -[RZBSimulatedTests testConnectionAndCancelWhileNotConnectable]. Any idea what's going on?

joshuatbrown pushed a commit to e-gineering/RZBluetooth that referenced this issue Feb 4, 2019
joshuatbrown pushed a commit to e-gineering/RZBluetooth that referenced this issue Feb 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants