Prevent thread deadlock when disconnecting the iPhone from the Head Unit

2 files changed, 33 insertions, 25 deletions

diff --git a/SmartDeviceLink/SDLIAPSession.m b/SmartDeviceLink/SDLIAPSession.m
index 47047ece9..9ae28569c 100644
--- a/SmartDeviceLink/SDLIAPSession.m
+++ b/SmartDeviceLink/SDLIAPSession.m
@@ -11,7 +11,7 @@
 NS_ASSUME_NONNULL_BEGIN
 
 NSString *const IOStreamThreadName = @"com.smartdevicelink.iostream";
-NSTimeInterval const StreamThreadWaitSecs = 1.0;
+NSTimeInterval const StreamThreadWaitSecs = 10.0;
 
 @interface SDLIAPSession ()
 
@@ -80,7 +80,6 @@ NSTimeInterval const StreamThreadWaitSecs = 1.0;
 }
 
 - (void)stop {
-    // This method must be called on the main thread
     if ([NSThread isMainThread]) {
         [self sdl_stop];
     } else {
@@ -91,6 +90,7 @@ NSTimeInterval const StreamThreadWaitSecs = 1.0;
 }
 
 - (void)sdl_stop {
+    NSParameterAssert([NSThread.currentThread isMainThread]);
     if (self.isDataSession) {
         [self.ioStreamThread cancel];
 
@@ -137,6 +137,10 @@ NSTimeInterval const StreamThreadWaitSecs = 1.0;
         if (bytesWritten < 0) {
             if (ostream.streamError != nil) {
                 [self sdl_handleOutputStreamWriteError:ostream.streamError];
+            } else {
+                // The write operation failed but there is no further information about the error.
+                // This can occur when disconnecting from an external accessory.
+                SDLLogE(@"Output stream write operation failed");
             }
         } else if (bytesWritten == bytesRemaining) {
             // Remove the data from the queue
@@ -173,13 +177,10 @@ NSTimeInterval const StreamThreadWaitSecs = 1.0;
         [self startStream:self.easession.outputStream];
 
         SDLLogD(@"Starting the accessory event loop");
-        do {
-            if (self.sendDataQueue.count > 0 && !self.sendDataQueue.frontDequeued) {
-                [self sdl_dequeueAndWriteToOutputStream];
-            }
-            // The principle here is to give the event loop enough time to process stream events while also allowing it to handle new enqueued data buffers in a timely manner. We're capping the run loop CPU time at 0.25s maximum before it will return control to the rest of the loop.
+        while (!NSThread.currentThread.cancelled) {
+            // Enqueued data will be written to and read from the streams in the runloop
             [[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate dateWithTimeIntervalSinceNow:0.25f]];
-        } while (![NSThread currentThread].cancelled);
+        }
 
         SDLLogD(@"Closing the accessory session for id: %tu, name: %@", self.easession.accessory.connectionID, self.easession.accessory.name);
 
diff --git a/SmartDeviceLink/SDLIAPTransport.m b/SmartDeviceLink/SDLIAPTransport.m
index df623d980..4147c12cd 100644
--- a/SmartDeviceLink/SDLIAPTransport.m
+++ b/SmartDeviceLink/SDLIAPTransport.m
@@ -549,28 +549,33 @@ int const ProtocolIndexTimeoutSeconds = 10;
 
 - (SDLStreamEndHandler)sdl_dataStreamEndedHandler {
     __weak typeof(self) weakSelf = self;
-    
     return ^(NSStream *stream) {
+        NSAssert(![NSThread.currentThread isMainThread], @"%@ should only be called on the IO thread", NSStringFromSelector(_cmd));
         __strong typeof(weakSelf) strongSelf = weakSelf;
+
         SDLLogD(@"Data stream ended");
-        if (strongSelf.session != nil) {
-            // The handler will be called on the IO thread, but the session stop method must be called on the main thread and we need to wait for the session to stop before nil'ing it out. To do this, we use dispatch_sync() on the main thread.
-            dispatch_sync(dispatch_get_main_queue(), ^{
-                [strongSelf.session stop];
-            });
+        if (!strongSelf.session) {
+            SDLLogD(@"Data session is nil");
+            return;
+        }
+        // The handler will be called on the IO thread, but the session stop method must be called on the main thread
+        dispatch_async(dispatch_get_main_queue(), ^{
+            [strongSelf.session stop];
             strongSelf.session.streamDelegate = nil;
             strongSelf.session = nil;
-        }
-        // We don't call sdl_retryEstablishSession here because the stream end event usually fires when the accessory is disconnected
+            
+            // We don't call sdl_retryEstablishSession here because the stream end event usually fires when the accessory is disconnected
+        });
+        
+        // To prevent deadlocks the handler must return to the runloop and not block the thread
     };
 }
 
 - (SDLStreamHasBytesHandler)sdl_dataStreamHasBytesHandler {
     __weak typeof(self) weakSelf = self;
-    
     return ^(NSInputStream *istream) {
+        NSAssert(![NSThread.currentThread isMainThread], @"%@ should only be called on the IO thread", NSStringFromSelector(_cmd));
         __strong typeof(weakSelf) strongSelf = weakSelf;
-        
         uint8_t buf[[[SDLGlobals sharedGlobals] mtuSizeForServiceType:SDLServiceTypeRPC]];
         while (istream.streamStatus == NSStreamStatusOpen && istream.hasBytesAvailable) {
             // It is necessary to check the stream status and whether there are bytes available because the dataStreamHasBytesHandler is executed on the IO thread and the accessory disconnect notification arrives on the main thread, causing data to be passed to the delegate while the main thread is tearing down the transport.
@@ -597,18 +602,20 @@ int const ProtocolIndexTimeoutSeconds = 10;
 
 - (SDLStreamErrorHandler)sdl_dataStreamErroredHandler {
     __weak typeof(self) weakSelf = self;
-    
     return ^(NSStream *stream) {
+        NSAssert(![NSThread.currentThread isMainThread], @"%@ should only be called on the IO thread", NSStringFromSelector(_cmd));
         __strong typeof(weakSelf) strongSelf = weakSelf;
         SDLLogE(@"Data stream error");
-        dispatch_sync(dispatch_get_main_queue(), ^{
+        dispatch_async(dispatch_get_main_queue(), ^{
             [strongSelf.session stop];
+            strongSelf.session.streamDelegate = nil;
+            strongSelf.session = nil;
+            if (![LegacyProtocolString isEqualToString:strongSelf.session.protocol]) {
+                [strongSelf sdl_retryEstablishSession];
+            }
         });
-        strongSelf.session.streamDelegate = nil;
-        strongSelf.session = nil;
-        if (![LegacyProtocolString isEqualToString:strongSelf.session.protocol]) {
-            [strongSelf sdl_retryEstablishSession];
-        }
+        
+        // To prevent deadlocks the handler must return to the runloop and not block the thread
     };
 }