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

Commit a3447e2

Browse files
committed
Avoid a stack overflow on ICommand executions
The implementation for OleMenuCommand apparently doesn't protect against reentrant calls of the canEnable callbacks (same bug ReactiveUI has, but at least there the class is private...). When executing a command, if the execution triggers a xaml refresh, it will call canEnable, re-enable the buttons, then execute all over again! As expected, this doesn't work well... CanEnable callbacks need to return false until the execution is fully done.
1 parent 6819bea commit a3447e2

File tree

1 file changed

+7
-3
lines changed

1 file changed

+7
-3
lines changed

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,23 +58,23 @@ public override void Initialize(IServiceProvider serviceProvider)
5858
(s, e) => Reload(new ViewWithData { Flow = UIControllerFlow.PullRequests, ViewType = UIViewType.PRList }));
5959

6060
back = ServiceProvider.AddCommandHandler(GuidList.guidGitHubToolbarCmdSet, PkgCmdIDList.backCommand,
61-
() => currentNavItem > 0,
61+
() => !disabled && currentNavItem > 0,
6262
() => {
6363
DisableButtons();
6464
Reload(navStack[--currentNavItem], true);
6565
},
6666
true);
6767

6868
forward = ServiceProvider.AddCommandHandler(GuidList.guidGitHubToolbarCmdSet, PkgCmdIDList.forwardCommand,
69-
() => currentNavItem < navStack.Count - 1,
69+
() => !disabled && currentNavItem < navStack.Count - 1,
7070
() => {
7171
DisableButtons();
7272
Reload(navStack[++currentNavItem], true);
7373
},
7474
true);
7575

7676
refresh = ServiceProvider.AddCommandHandler(GuidList.guidGitHubToolbarCmdSet, PkgCmdIDList.refreshCommand,
77-
() => navStack.Count > 0,
77+
() => !disabled && navStack.Count > 0,
7878
() => {
7979
DisableButtons();
8080
Reload();
@@ -224,10 +224,12 @@ void UpdateToolbar()
224224
back.Enabled = currentNavItem > 0;
225225
forward.Enabled = currentNavItem < navStack.Count - 1;
226226
refresh.Enabled = navStack.Count > 0;
227+
disabled = false;
227228
}
228229

229230
void DisableButtons()
230231
{
232+
disabled = true;
231233
back.Enabled = false;
232234
forward.Enabled = false;
233235
refresh.Enabled = false;
@@ -279,6 +281,8 @@ public bool IsGitHubRepo
279281
public bool IsShowing => true;
280282

281283
bool disposed = false;
284+
private bool disabled;
285+
282286
protected override void Dispose(bool disposing)
283287
{
284288
if (disposing)

0 commit comments

Comments
 (0)