Merge pull request #1069 from XevoInc/fix/lifecycle_manager_queue

Fix/lifecycle manager queue

2 files changed, 62 insertions, 29 deletions

diff --git a/SmartDeviceLink/SDLLifecycleManager.m b/SmartDeviceLink/SDLLifecycleManager.m
index 0f6a7d4a6..c5cc8ea5a 100644
--- a/SmartDeviceLink/SDLLifecycleManager.m
+++ b/SmartDeviceLink/SDLLifecycleManager.m
@@ -163,16 +163,16 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
     SDLLogD(@"Starting lifecycle manager");
     self.readyHandler = [readyHandler copy];
 
-    [self.lifecycleStateMachine transitionToState:SDLLifecycleStateStarted];
+    [self sdl_transitionToState:SDLLifecycleStateStarted];
 }
 
 - (void)stop {
     dispatch_sync(_lifecycleQueue, ^{
         SDLLogD(@"Lifecycle manager stopped");
         if ([self.lifecycleStateMachine isCurrentState:SDLLifecycleStateReady]) {
-            [self.lifecycleStateMachine transitionToState:SDLLifecycleStateUnregistering];
+            [self sdl_transitionToState:SDLLifecycleStateUnregistering];
         } else {
-            [self.lifecycleStateMachine transitionToState:SDLLifecycleStateStopped];
+            [self sdl_transitionToState:SDLLifecycleStateStopped];
         }
     });
 }
@@ -261,10 +261,13 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
     // Apple Bug ID #30059457
     __weak typeof(self) weakSelf = self;
     dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0.5 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
-        [weakSelf.delegate managerDidDisconnect];
+        __strong typeof(weakSelf) strongSelf = weakSelf;
+        if (!strongSelf) { return; }
+
+        [strongSelf.delegate managerDidDisconnect];
 
         if (shouldRestart) {
-            [weakSelf.lifecycleStateMachine transitionToState:SDLLifecycleStateStarted];
+            [strongSelf sdl_transitionToState:SDLLifecycleStateStarted];
         }
     });
 }
@@ -290,7 +293,7 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
                     weakSelf.readyHandler(NO, error);
 
                     if (weakSelf.lifecycleState != SDLLifecycleStateReconnecting) {
-                        [weakSelf.lifecycleStateMachine transitionToState:SDLLifecycleStateStopped];
+                        [weakSelf sdl_transitionToState:SDLLifecycleStateStopped];
                     }
 
                     return;
@@ -298,7 +301,7 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
 
                 weakSelf.registerResponse = (SDLRegisterAppInterfaceResponse *)response;
                 [SDLGlobals sharedGlobals].rpcVersion = weakSelf.registerResponse.syncMsgVersion;
-                [weakSelf.lifecycleStateMachine transitionToState:SDLLifecycleStateRegistered];
+                [weakSelf sdl_transitionToState:SDLLifecycleStateRegistered];
             });
         }];
 }
@@ -311,9 +314,9 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
     
     // language mismatch? but actual language is a supported language? and delegate has implemented method?
     if (actualLanguage != desiredLanguage && [supportedLanguages containsObject:actualLanguage] && delegateCanUpdateLifecycle) {
-        [self.lifecycleStateMachine transitionToState:SDLLifecycleStateUpdatingConfiguration];
+        [self sdl_transitionToState:SDLLifecycleStateUpdatingConfiguration];
     } else {
-        [self.lifecycleStateMachine transitionToState:SDLLifecycleStateSettingUpManagers];
+        [self sdl_transitionToState:SDLLifecycleStateSettingUpManagers];
     }
 }
 
@@ -347,7 +350,7 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
         [self sendConnectionManagerRequest:changeRegistration withResponseHandler:nil];
     }
     
-    [self.lifecycleStateMachine transitionToState:SDLLifecycleStateSettingUpManagers];
+    [self sdl_transitionToState:SDLLifecycleStateSettingUpManagers];
 }
 
 - (void)didEnterStateSettingUpManagers {
@@ -398,14 +401,14 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
     dispatch_group_notify(managerGroup, self.lifecycleQueue, ^{
         // We could have been shut down while waiting for the completion of starting file manager and permission manager.
         if (self.lifecycleState == SDLLifecycleStateSettingUpManagers) {
-            [self.lifecycleStateMachine transitionToState:SDLLifecycleStateSettingUpAppIcon];
+            [self sdl_transitionToState:SDLLifecycleStateSettingUpAppIcon];
         }
     });
 }
 
 - (void)didEnterStateSettingUpAppIcon {
     if (self.registerResponse.iconResumed.boolValue) {
-        [self.lifecycleStateMachine transitionToState:SDLLifecycleStateSettingUpHMI];
+        [self sdl_transitionToState:SDLLifecycleStateSettingUpHMI];
         return;
     }
 
@@ -415,7 +418,7 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
         dispatch_async(weakself.lifecycleQueue, ^{
             // We could have been shut down while setting up the app icon, make sure we still want to continue or we could crash
             if (weakself.lifecycleState == SDLLifecycleStateSettingUpAppIcon) {
-                [weakself.lifecycleStateMachine transitionToState:SDLLifecycleStateSettingUpHMI];
+                [weakself sdl_transitionToState:SDLLifecycleStateSettingUpHMI];
             }
         });
    }];
@@ -429,7 +432,7 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
     }
 
     // We are sure to have a HMIStatus, set state to ready
-    [self.lifecycleStateMachine transitionToState:SDLLifecycleStateReady];
+    [self sdl_transitionToState:SDLLifecycleStateReady];
 }
 
 - (void)didEnterStateReady {
@@ -470,7 +473,7 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
                 SDLLogE(@"SDL Error unregistering, we are going to hard disconnect: %@, response: %@", error, response);
             }
 
-            [weakSelf.lifecycleStateMachine transitionToState:SDLLifecycleStateStopped];
+            [weakSelf sdl_transitionToState:SDLLifecycleStateStopped];
         }];
 }
 
@@ -611,6 +614,18 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
     return YES;
 }
 
+// this is to make sure that the transition happens on the dedicated queue
+- (void)sdl_transitionToState:(SDLState *)state {
+    if (strcmp(dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL), dispatch_queue_get_label(self.lifecycleQueue)) == 0) {
+        [self.lifecycleStateMachine transitionToState:state];
+    } else {
+        // once this method returns, the transition is completed
+        dispatch_sync(self.lifecycleQueue, ^{
+            [self.lifecycleStateMachine transitionToState:state];
+        });
+    }
+}
+
 
 #pragma mark SDL notification observers
 
@@ -618,7 +633,7 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
     SDLLogD(@"Transport connected");
 
     dispatch_async(self.lifecycleQueue, ^{
-        [self.lifecycleStateMachine transitionToState:SDLLifecycleStateConnected];
+        [self sdl_transitionToState:SDLLifecycleStateConnected];
     });
 }
 
@@ -627,9 +642,9 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
 
     dispatch_async(self.lifecycleQueue, ^{
         if (self.lifecycleState == SDLLifecycleStateUnregistering || self.lifecycleState == SDLLifecycleStateStopped) {
-            [self.lifecycleStateMachine transitionToState:SDLLifecycleStateStopped];
+            [self sdl_transitionToState:SDLLifecycleStateStopped];
         } else {
-            [self.lifecycleStateMachine transitionToState:SDLLifecycleStateReconnecting];
+            [self sdl_transitionToState:SDLLifecycleStateReconnecting];
         }
     });
 }
@@ -659,7 +674,7 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
     SDLLogD(@"Audio streaming state changed from %@ to %@", oldStreamingState, self.audioStreamingState);
 
     if ([self.lifecycleStateMachine isCurrentState:SDLLifecycleStateSettingUpHMI]) {
-        [self.lifecycleStateMachine transitionToState:SDLLifecycleStateReady];
+        [self sdl_transitionToState:SDLLifecycleStateReady];
     }
 
     if (![self.lifecycleStateMachine isCurrentState:SDLLifecycleStateReady]) {
@@ -701,14 +716,14 @@ SDLLifecycleState *const SDLLifecycleStateReady = @"Ready";
     SDLLogE(@"Remote Device forced unregistration for reason: %@", appUnregisteredNotification.reason);
 
     if ([self.lifecycleStateMachine isCurrentState:SDLLifecycleStateUnregistering]) {
-        [self.lifecycleStateMachine transitionToState:SDLLifecycleStateStopped];
+        [self sdl_transitionToState:SDLLifecycleStateStopped];
     } else if ([self.lifecycleStateMachine isCurrentState:SDLLifecycleStateStopped]) {
         return;
     } else if ([appUnregisteredNotification.reason isKindOfClass:[NSString class]] && [appUnregisteredNotification.reason isEqualToEnum:SDLAppInterfaceUnregisteredReasonAppUnauthorized]) {
         // HAX: The string check is due to a core "feature" that could cause -1 to be sent as the enum value, which will crash here.
-        [self.lifecycleStateMachine transitionToState:SDLLifecycleStateStopped];
+        [self sdl_transitionToState:SDLLifecycleStateStopped];
     } else {
-        [self.lifecycleStateMachine transitionToState:SDLLifecycleStateReconnecting];
+        [self sdl_transitionToState:SDLLifecycleStateReconnecting];
     }
 }
 
diff --git a/SmartDeviceLinkTests/DevAPISpecs/SDLLifecycleManagerSpec.m b/SmartDeviceLinkTests/DevAPISpecs/SDLLifecycleManagerSpec.m
index e03c821ac..49892f366 100644
--- a/SmartDeviceLinkTests/DevAPISpecs/SDLLifecycleManagerSpec.m
+++ b/SmartDeviceLinkTests/DevAPISpecs/SDLLifecycleManagerSpec.m
@@ -39,6 +39,11 @@
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
 
+@interface SDLLifecycleManager ()
+// this private property is used for testing
+@property (copy, nonatomic) dispatch_queue_t lifecycleQueue;
+@end
+
 QuickConfigurationBegin(SendingRPCsConfiguration)
 
 + (void)configure:(Configuration *)configuration {
@@ -77,6 +82,18 @@ describe(@"a lifecycle manager", ^{
     __block id streamingManagerMock = OCMClassMock([SDLStreamingMediaManager class]);
     __block id systemCapabilityMock = OCMClassMock([SDLSystemCapabilityManager class]);
     
+    void (^transitionToState)(SDLState *) = ^(SDLState *state) {
+        dispatch_sync(testManager.lifecycleQueue, ^{
+            [testManager.lifecycleStateMachine transitionToState:state];
+        });
+    };
+
+    void (^setToStateWithEnterTransition)(SDLState *, SDLState *) = ^(SDLState *oldState, SDLState *newState) {
+        dispatch_sync(testManager.lifecycleQueue, ^{
+            [testManager.lifecycleStateMachine setToState:newState fromOldState:oldState callEnterTransition:YES];
+        });
+    };
+
     beforeEach(^{
         OCMStub([proxyMock iapProxyWithListener:[OCMArg any] secondaryTransportManager:[OCMArg any]]).andReturn(proxyMock);
         OCMStub([(SDLProxy*)proxyMock protocol]).andReturn(protocolMock);
@@ -257,7 +274,7 @@ describe(@"a lifecycle manager", ^{
 
                     // Send an RAI response & make sure we have an HMI status to move the lifecycle forward
                     testManager.hmiLevel = SDLHMILevelFull;
-                    [testManager.lifecycleStateMachine transitionToState:SDLLifecycleStateRegistered];
+                    transitionToState(SDLLifecycleStateRegistered);
                     [NSThread sleepForTimeInterval:0.3];
                 });
                 
@@ -302,7 +319,7 @@ describe(@"a lifecycle manager", ^{
                     response.resultCode = SDLResultSuccess;
                     testManager.registerResponse = response;
                     
-                    [testManager.lifecycleStateMachine setToState:SDLLifecycleStateSettingUpHMI fromOldState:nil callEnterTransition:YES];
+                    setToStateWithEnterTransition(nil, SDLLifecycleStateSettingUpHMI);
 
                     expect(@(readyHandlerSuccess)).to(equal(@NO));
                     expect(readyHandlerError).to(beNil());
@@ -319,7 +336,7 @@ describe(@"a lifecycle manager", ^{
                     response.resultCode = SDLResultSuccess;
                     testManager.registerResponse = response;
                     
-                    [testManager.lifecycleStateMachine setToState:SDLLifecycleStateSettingUpHMI fromOldState:nil callEnterTransition:YES];
+                    setToStateWithEnterTransition(nil, SDLLifecycleStateSettingUpHMI);
                     
                     testHMILevel = SDLHMILevelFull;
                     testHMIStatus.hmiLevel = testHMILevel;
@@ -340,7 +357,7 @@ describe(@"a lifecycle manager", ^{
                     response.resultCode = SDLResultSuccess;
                     testManager.registerResponse = response;
                     
-                    [testManager.lifecycleStateMachine setToState:SDLLifecycleStateReady fromOldState:nil callEnterTransition:YES];
+                    setToStateWithEnterTransition(nil, SDLLifecycleStateReady);
 
                     expect(@(readyHandlerSuccess)).toEventually(equal(@YES));
                     expect(readyHandlerError).toEventually(beNil());
@@ -354,7 +371,7 @@ describe(@"a lifecycle manager", ^{
                     response.info = @"some info";
                     testManager.registerResponse = response;
 
-                    [testManager.lifecycleStateMachine setToState:SDLLifecycleStateReady fromOldState:nil callEnterTransition:YES];
+                    setToStateWithEnterTransition(nil, SDLLifecycleStateReady);
 
                     expect(@(readyHandlerSuccess)).toEventually(equal(@YES));
                     expect(readyHandlerError).toEventuallyNot(beNil());
@@ -383,7 +400,7 @@ describe(@"a lifecycle manager", ^{
                     SDLLifecycleConfigurationUpdate *update = [[SDLLifecycleConfigurationUpdate alloc] initWithAppName:@"EnGb" shortAppName:@"E" ttsName:[SDLTTSChunk textChunksFromString:@"EnGb ttsName"] voiceRecognitionCommandNames:nil];
                     OCMStub([testManager.delegate managerShouldUpdateLifecycleToLanguage:[OCMArg any]]).andReturn(update);
 
-                    [testManager.lifecycleStateMachine setToState:SDLLifecycleStateUpdatingConfiguration fromOldState:SDLLifecycleStateRegistered callEnterTransition:YES];
+                    setToStateWithEnterTransition(SDLLifecycleStateRegistered, SDLLifecycleStateUpdatingConfiguration);
                     // Transition to StateSettingUpManagers to prevent assert error from the lifecycle machine
                     [testManager.lifecycleStateMachine setToState:SDLLifecycleStateSettingUpManagers fromOldState:SDLLifecycleStateUpdatingConfiguration callEnterTransition:NO];
 
@@ -405,7 +422,7 @@ describe(@"a lifecycle manager", ^{
 
                     OCMStub([testManager.delegate managerShouldUpdateLifecycleToLanguage:[OCMArg any]]).andReturn(nil);
 
-                    [testManager.lifecycleStateMachine setToState:SDLLifecycleStateUpdatingConfiguration fromOldState:SDLLifecycleStateRegistered callEnterTransition:YES];
+                    setToStateWithEnterTransition(SDLLifecycleStateRegistered, SDLLifecycleStateUpdatingConfiguration);
                     // Transition to StateSettingUpManagers to prevent assert error from the lifecycle machine
                     [testManager.lifecycleStateMachine setToState:SDLLifecycleStateSettingUpManagers fromOldState:SDLLifecycleStateUpdatingConfiguration callEnterTransition:NO];
 
@@ -449,6 +466,7 @@ describe(@"a lifecycle manager", ^{
                         testUnregisterResponse.success = @YES;
                         testUnregisterResponse.correlationID = @(testManager.lastCorrelationId);
                         
+                        // This should run on `com.sdl.rpcProcessingQueue` to simulate the real case.
                         [testManager.notificationDispatcher postRPCResponseNotification:SDLDidReceiveUnregisterAppInterfaceResponse response:testUnregisterResponse];
                     });