Fix race condition when finishing ui flows

Let's decide that the `IUIController.ListenToCompletionState` observable
finishes first, and then the `IUIController.SelectFlow` observable. Once
the `SelectFlow` observable is completed, it's a fair bet that the
`IUIController` is disposed and it isn't safe to access it for anything
else.

Fixes #203 properly

Author: shana

src/GitHub.App/Controllers/UIController.cs

@@ -153,9 +153,9 @@ public IObservable<bool> ListenToCompletionState()
         void End(bool success)
         {
             uiProvider.RemoveService(typeof(IConnection));
-            transition.OnCompleted();
             completion?.OnNext(success);
             completion?.OnCompleted();
+            transition.OnCompleted();
             completion = null;
         }
 

src/GitHub.Exports/Services/IUIProvider.cs

@@ -24,5 +24,6 @@ public interface IUIProvider : IServiceProvider
         IObservable<UserControl> SetupUI(UIControllerFlow controllerFlow, IConnection connection);
         void RunUI();
         void RunUI(UIControllerFlow controllerFlow, IConnection connection);
+        IObservable<bool> ListenToCompletionState();
     }
 }

src/GitHub.VisualStudio/Services/UIProvider.cs

@@ -31,6 +31,7 @@ public class UIProvider : IUIProvider, IDisposable
         CompositionContainer tempContainer;
         readonly Dictionary<string, ComposablePart> tempParts;
         ExportLifetimeContext<IUIController> currentUIFlow;
+        IUIController currentUI;
         readonly Version currentVersion;
         bool initializingLogging = false;
 
@@ -201,9 +202,9 @@ public IObservable<UserControl> SetupUI(UIControllerFlow controllerFlow, [AllowN
 
             var factory = GetService<IExportFactoryProvider>();
             currentUIFlow = factory.UIControllerFactory.CreateExport();
+            currentUI = currentUIFlow.Value;
             var disposable = currentUIFlow;
-            var ui = currentUIFlow.Value;
-            var creation = ui.SelectFlow(controllerFlow);
+            var creation = currentUI.SelectFlow(controllerFlow);
             windowController = new UI.WindowController(creation);
             windowController.WindowStartupLocation = System.Windows.WindowStartupLocation.CenterOwner;
             windowController.Closed += StopUIFlowWhenWindowIsClosedByUser;
@@ -216,10 +217,16 @@ public IObservable<UserControl> SetupUI(UIControllerFlow controllerFlow, [AllowN
                 else
                     StopUI();
             });
-            ui.Start(connection);
+            currentUI.Start(connection);
             return creation;
         }
 
+        public IObservable<bool> ListenToCompletionState()
+        {
+            Debug.Assert(currentUI != null, "Cannot call ListenToCompletionState without calling SetupUI first");
+            return currentUI?.ListenToCompletionState();
+        }
+
         public void RunUI()
         {
             if (!Initialized)
@@ -272,6 +279,7 @@ public void StopUI()
             }
 
             StopUI(currentUIFlow);
+            currentUI = null;
             currentUIFlow = null;
         }
 

src/GitHub.VisualStudio/TeamExplorer/Connect/GitHubConnectSection.cs

@@ -323,14 +323,18 @@ void StartFlow(UIControllerFlow controllerFlow)
         {
             var uiProvider = ServiceProvider.GetExportedValue<IUIProvider>();
             uiProvider.GitServiceProvider = ServiceProvider;
-            var ret = uiProvider.SetupUI(controllerFlow, SectionConnection);
-            ret.Subscribe(_ => {}, () =>
-            {
-                if (controllerFlow == UIControllerFlow.Clone)
-                    isCloning = true;
-                else if (controllerFlow == UIControllerFlow.Create)
-                    isCreating = true;
-            });
+            uiProvider.SetupUI(controllerFlow, SectionConnection);
+            uiProvider.ListenToCompletionState()
+                .Subscribe(success =>
+                {
+                    if (success)
+                    {
+                        if (controllerFlow == UIControllerFlow.Clone)
+                            isCloning = true;
+                        else if (controllerFlow == UIControllerFlow.Create)
+                            isCreating = true;
+                    }
+                });
             uiProvider.RunUI();
         }
 

src/GitHub.VisualStudio/TeamExplorer/Sync/GitHubPublishSection.cs

@@ -118,19 +118,20 @@ void ShowPublish()
             var ui = uiflow.Value;
             var creation = ui.SelectFlow(UIControllerFlow.Publish);
             bool success = false;
-            ui.ListenToCompletionState().Subscribe(s => success = s, () =>
-            {
-                // there's no real cancel button in the publish form, but if support a back button there, then we want to hide the form
-                IsVisible = false;
-                if (success)
-                    ServiceProvider.TryGetService<ITeamExplorer>()?.NavigateToPage(new Guid(TeamExplorerPageIds.Home), null);
-            });
+            ui.ListenToCompletionState().Subscribe(s => success = s);
 
             creation.Subscribe(c =>
             {
                 SectionContent = c;
                 c.DataContext = this;
                 ((IView)c).IsBusy.Subscribe(x => IsBusy = x);
+            },
+            () =>
+            {
+                // there's no real cancel button in the publish form, but if support a back button there, then we want to hide the form
+                IsVisible = false;
+                if (success)
+                    ServiceProvider.TryGetService<ITeamExplorer>()?.NavigateToPage(new Guid(TeamExplorerPageIds.Home), null);
             });
             ui.Start(null);
         }
@@ -142,9 +143,8 @@ protected override void Dispose(bool disposing)
             {
                 if (!disposed)
                 {
-                    if (disposable != null)
-                        disposable.Dispose();
                     disposed = true;
+                    disposable?.Dispose();
                 }
             }
             base.Dispose(disposing);