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

Commit b24b202

Browse files
committed
Make sure github pane is reentrant
Fixes #282 We were getting into a state when loading the pane for the first time where `Reload` would be called once as a result of the `RepoChanged` call, which triggers when `Initialize(ServiceProvider)` runs, and then `Reload` would run again as a result of the `Initialize(ViewWithData)` call, because the Pull Requests button was clicked, which causes the pane to reset itself. These two `Reload` calls would be executing concurrently, causing a race condition that would crash randomly if the controller was stopped and restarted at just the right point. Need to make sure that nothing else can happen while `Initialize(ServiceProvider)` is running, and that `Initialize(ServiceProvider)` is run serially (no asyncs here!).
1 parent 96521e6 commit b24b202

File tree

1 file changed

+29
-23
lines changed

1 file changed

+29
-23
lines changed

src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,15 @@
1818
using NullGuard;
1919
using ReactiveUI;
2020
using System.Collections.Generic;
21-
using Microsoft.VisualStudio.Shell;
21+
using System.Threading.Tasks;
2222

2323
namespace GitHub.VisualStudio.UI.Views
2424
{
2525
[ExportViewModel(ViewType = UIViewType.GitHubPane)]
2626
[PartCreationPolicy(CreationPolicy.NonShared)]
2727
public class GitHubPaneViewModel : TeamExplorerItemBase, IGitHubPaneViewModel
2828
{
29+
bool initialized;
2930
readonly CompositeDisposable disposables = new CompositeDisposable();
3031
IUIController uiController;
3132
WindowController windowController;
@@ -37,7 +38,8 @@ public class GitHubPaneViewModel : TeamExplorerItemBase, IGitHubPaneViewModel
3738
readonly List<ViewWithData> navStack = new List<ViewWithData>();
3839
int currentNavItem = -1;
3940
bool navigatingViaArrows;
40-
OleMenuCommand back, forward, refresh;
41+
bool disabled;
42+
Microsoft.VisualStudio.Shell.OleMenuCommand back, forward, refresh;
4143

4244
[ImportingConstructor]
4345
public GitHubPaneViewModel(ISimpleApiClientFactory apiFactory, ITeamExplorerServiceHolder holder,
@@ -56,58 +58,63 @@ public override void Initialize(IServiceProvider serviceProvider)
5658
base.Initialize(serviceProvider);
5759

5860
ServiceProvider.AddCommandHandler(GuidList.guidGitHubToolbarCmdSet, PkgCmdIDList.pullRequestCommand,
59-
(s, e) => Reload(new ViewWithData { Flow = UIControllerFlow.PullRequests, ViewType = UIViewType.PRList }));
61+
(s, e) => Reload(new ViewWithData { Flow = UIControllerFlow.PullRequests, ViewType = UIViewType.PRList }).Forget());
6062

6163
back = ServiceProvider.AddCommandHandler(GuidList.guidGitHubToolbarCmdSet, PkgCmdIDList.backCommand,
6264
() => !disabled && currentNavItem > 0,
6365
() => {
6466
DisableButtons();
65-
Reload(navStack[--currentNavItem], true);
67+
Reload(navStack[--currentNavItem], true).Forget();
6668
},
6769
true);
6870

6971
forward = ServiceProvider.AddCommandHandler(GuidList.guidGitHubToolbarCmdSet, PkgCmdIDList.forwardCommand,
7072
() => !disabled && currentNavItem < navStack.Count - 1,
7173
() => {
7274
DisableButtons();
73-
Reload(navStack[++currentNavItem], true);
75+
Reload(navStack[++currentNavItem], true).Forget();
7476
},
7577
true);
7678

7779
refresh = ServiceProvider.AddCommandHandler(GuidList.guidGitHubToolbarCmdSet, PkgCmdIDList.refreshCommand,
7880
() => !disabled && navStack.Count > 0,
7981
() => {
8082
DisableButtons();
81-
Reload();
83+
Reload().Forget();
8284
},
8385
true);
86+
87+
initialized = true;
8488
}
8589

8690
public void Initialize([AllowNull] ViewWithData data)
8791
{
8892
Title = "GitHub";
89-
Reload(data);
93+
Reload(data).Forget();
9094
}
9195

92-
protected async override void RepoChanged(bool changed)
96+
protected override void RepoChanged(bool changed)
9397
{
9498
base.RepoChanged(changed);
9599

96-
if (!changed)
100+
if (!initialized)
97101
return;
98102

99-
IsGitHubRepo = await IsAGitHubRepo();
100-
if (!IsGitHubRepo)
103+
if (!changed)
101104
return;
102105

103-
Reload();
106+
IsGitHubRepo = null;
107+
Reload().Forget();
104108
}
105109

106-
async void Reload([AllowNull] ViewWithData data = null, bool navigating = false)
110+
async Task Reload([AllowNull] ViewWithData data = null, bool navigating = false)
107111
{
108112
navigatingViaArrows = navigating;
109113

110-
if (!IsGitHubRepo)
114+
if (!IsGitHubRepo.HasValue)
115+
IsGitHubRepo = await IsAGitHubRepo();
116+
117+
if (!IsGitHubRepo.Value)
111118
{
112119
if (uiController != null)
113120
{
@@ -135,6 +142,7 @@ async void Reload([AllowNull] ViewWithData data = null, bool navigating = false)
135142
var c = factory.CreateViewAndViewModel(UIViewType.LoggedOut);
136143
Control = c.View;
137144
}
145+
return;
138146
}
139147

140148
void StartFlow(UIControllerFlow controllerFlow, [AllowNull]IConnection conn, ViewWithData data = null)
@@ -268,8 +276,8 @@ public bool IsLoggedIn
268276
set { isLoggedIn = value; this.RaisePropertyChange(); }
269277
}
270278

271-
bool isGitHubRepo;
272-
public bool IsGitHubRepo
279+
bool? isGitHubRepo;
280+
public bool? IsGitHubRepo
273281
{
274282
get { return isGitHubRepo; }
275283
set { isGitHubRepo = value; this.RaisePropertyChange(); }
@@ -282,17 +290,15 @@ public bool IsGitHubRepo
282290
public bool IsShowing => true;
283291

284292
bool disposed = false;
285-
private bool disabled;
286-
287293
protected override void Dispose(bool disposing)
288294
{
289295
if (disposing)
290296
{
291-
if (!disposed)
292-
{
293-
disposables.Dispose();
294-
disposed = true;
295-
}
297+
if (disposed)
298+
return;
299+
disposed = true;
300+
DisableButtons();
301+
disposables.Dispose();
296302
}
297303
base.Dispose(disposing);
298304
}

0 commit comments

Comments
 (0)