mickeyl / LTSupportAutomotive

An iOS / watchOS / macOS support library for OBD2, VIN-Decoding, and more.
MIT License
212 stars 59 forks source link

LTOBD2Adapter: Race Condition in Command Queue Access #22

Open Skyb0rg opened 5 years ago

Skyb0rg commented 5 years ago
-(void)asyncEnqueueInternalCommand:(LTOBD2AdapterInternalCommand*)internalCommand
{
    **[_commandQueue addObject:internalCommand];**
    [self asyncProcessCommandQueue];
}
*** Terminating app due to uncaught exception 'NSRangeException', reason: '*** -[__NSArrayM insertObject:atIndex:]: index 7191 beyond bounds [0 .. 7189]'

Can you help me to figure out what has happend. I record over 30 Minutes rpm and speed and 4 Other values.

Skyb0rg commented 5 years ago

Tested this in debug and _commandQueue is between 1 and 6 after adding commands. i dont know why it can go up to 7000. And why it is possible to get this Error: index beyond bounds

i will try it with this solution:

if ( [_commandQueue count] > 200 )
{
    NSLog( @"cancelPendingCommands : Has more than 200" );
    [self cancelPendingCommands];
}

after: asyncProcessCommandQueue

mickeyl commented 5 years ago

Thanks for this report. The actual problem looks like a multithreading race condition – I should check whether access of the command queue is a) coordinated or b) make the command queue thread-safe.

That said, if you're checking values in a loop, you have to take care that you're not adding commands faster than they are handled, so instead of adding based on a timer it would be better to use the completion handler to schedule the next iteration.

Last but not least, there are several optimizations that I want to tackle in 2019, namely adding facilities to group multiple PID queries (in order to save bandwidth), and also something to ease repeating queries.

Skyb0rg commented 5 years ago

created an outlet for the queue count and slow down the new requests if needed. for me its currently ok. Thank you for detailed answer 👍

Skyb0rg commented 5 years ago

Have you any idea how to fix this ? Yesterday it was 0...4 [index 6 beyond bounds]

I start the "transmitMultipleCommands" function in the main queue but it doesn´t help.

Skyb0rg commented 5 years ago

This Error only happens all >50.000 readings so its hard to find the error but i think this might be help.

-(void)asyncEnqueueInternalCommand:(LTOBD2AdapterInternalCommand*)internalCommand
{
    @synchronized(self) {
        [_commandQueue addObject:internalCommand];
        [self asyncProcessCommandQueue];
    }
}
Skyb0rg commented 5 years ago

@synchronized(self) is not the solution. Now i test with the serial dispatchqueue.

Skyb0rg commented 5 years ago

created pull request for the solution. #26