Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 3cebf10

Browse files
committed
Fix race condition
The `ListenToCompletionState` subscription was triggering a navigation request on the `Next` callback. Depending on how fast VS responds, this can cause the Sync section to be disposed, along with the `UIController` instance, before the handler returns, which would cause `UIController` to crash. Make sure we only navigate away once the `ListenToCompletionState` observable is completed, and also tweak the way `Dispose` happens in `UIController` to minimize races.
1 parent ead03f2 commit 3cebf10

File tree

2 files changed

+15
-7
lines changed

2 files changed

+15
-7
lines changed

src/GitHub.App/Controllers/UIController.cs

+10-5
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ void End(bool success)
152152
{
153153
uiProvider.RemoveService(typeof(IConnection));
154154
transition.OnCompleted();
155+
completion?.OnNext(success);
155156
completion?.OnCompleted();
156157
completion = null;
157158
}
@@ -291,15 +292,19 @@ protected virtual void Dispose(bool disposing)
291292
if (disposing)
292293
{
293294
if (disposed) return;
295+
disposed = true;
294296

295-
Debug.WriteLine("Disposing ({0})", GetHashCode());
296-
disposables.Dispose();
297-
transition?.Dispose();
298-
completion?.Dispose();
299297
if (connectionAdded != null)
300298
connectionManager.Connections.CollectionChanged -= connectionAdded;
301299
connectionAdded = null;
302-
disposed = true;
300+
301+
var tr = transition;
302+
var cmp = completion;
303+
transition = null;
304+
completion = null;
305+
disposables.Dispose();
306+
tr?.Dispose();
307+
cmp?.Dispose();
303308
}
304309
}
305310

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,13 @@ void ShowPublish()
117117
disposable = uiflow;
118118
var ui = uiflow.Value;
119119
var creation = ui.SelectFlow(UIControllerFlow.Publish);
120-
ui.ListenToCompletionState().Subscribe(_ => { }, () =>
120+
bool success = false;
121+
ui.ListenToCompletionState().Subscribe(s => success = s, () =>
121122
{
123+
// there's no real cancel button in the publish form, but if support a back button there, then we want to hide the form
122124
IsVisible = false;
123-
ServiceProvider.TryGetService<ITeamExplorer>()?.NavigateToPage(new Guid(TeamExplorerPageIds.Home), null);
125+
if (success)
126+
ServiceProvider.TryGetService<ITeamExplorer>()?.NavigateToPage(new Guid(TeamExplorerPageIds.Home), null);
124127
});
125128

126129
creation.Subscribe(c =>

0 commit comments

Comments
 (0)