Skip to content

Fix #3705: Code window is empty when select a .baml and refresh #3726

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
christophwille wants to merge 1 commit into
base: master
Choose a base branch
from
+19 −0
Open

Fix #3705: Code window is empty when select a .baml and refresh #3726

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions ILSpy/AssemblyTree/AssemblyTreeModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -946,12 +946,31 @@ public void Refresh()
}

private void RefreshInternal()
{
_ = RefreshInternalAsync();
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RefreshInternal() fires RefreshInternalAsync() without observing the returned task. If RefreshInternalAsync throws (e.g., from ShowAssemblyList, SelectNode, or RefreshDecompiledView), the exception becomes unobserved and can be lost/crash later. Consider attaching the existing task error handling used elsewhere in this file (e.g., call .HandleExceptions() on the returned task, or explicitly .Catch<Exception>(...) + .IgnoreExceptions()).

Suggested change
_ = RefreshInternalAsync();
_ = RefreshInternalAsync().HandleExceptions();

Copilot uses AI. Check for mistakes.
}

private async Task RefreshInternalAsync()
{
Comment on lines +949 to 954
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because RefreshInternal() can be invoked multiple times (via refreshThrottle.Tick()), the fire-and-forget async refresh allows multiple RefreshInternalAsync executions to overlap/re-enter while awaiting GetMetadataFileAsync(). That can result in out-of-order ShowAssemblyList/SelectNode/RefreshDecompiledView updates. Consider serializing refreshes (e.g., track the currently running refresh task and skip/await it, or use a generation id/cancellation token so only the latest refresh applies).

Copilot uses AI. Check for mistakes.
using (Keyboard.FocusedElement.PreserveFocus())
{
var path = GetPathForNode(SelectedItem);

ShowAssemblyList(settingsService.AssemblyListManager.LoadList(AssemblyList.ListName));

// Ensure assembly loaded before FindNodeByPath to allow lazy-loaded resource nodes to be found
if (path?.Length > 0)
{
foreach (var asm in AssemblyList.GetAssemblies())
{
if (asm.FileName == path[0])
{
await asm.GetMetadataFileAsync().Catch<Exception>(_ => { });
break;
}
}
}

Comment on lines 955 to +973
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keyboard.FocusedElement.PreserveFocus() now spans an await. Since PreserveFocus restores focus unconditionally on Dispose (see ExtensionMethods.cs), a slow GetMetadataFileAsync() can cause focus to be restored later and steal focus from whatever the user clicked during the await. Consider only restoring focus if it hasn't changed since the refresh started (or move the PreserveFocus scope so it doesn't cover the awaited portion).

Suggested change
using (Keyboard.FocusedElement.PreserveFocus())
{
var path = GetPathForNode(SelectedItem);
ShowAssemblyList(settingsService.AssemblyListManager.LoadList(AssemblyList.ListName));
// Ensure assembly loaded before FindNodeByPath to allow lazy-loaded resource nodes to be found
if (path?.Length > 0)
{
foreach (var asm in AssemblyList.GetAssemblies())
{
if (asm.FileName == path[0])
{
await asm.GetMetadataFileAsync().Catch<Exception>(_ => { });
break;
}
}
}
var path = GetPathForNode(SelectedItem);
using (Keyboard.FocusedElement.PreserveFocus())
{
ShowAssemblyList(settingsService.AssemblyListManager.LoadList(AssemblyList.ListName));
}
// Ensure assembly loaded before FindNodeByPath to allow lazy-loaded resource nodes to be found
if (path?.Length > 0)
{
foreach (var asm in AssemblyList.GetAssemblies())
{
if (asm.FileName == path[0])
{
await asm.GetMetadataFileAsync().Catch<Exception>(_ => { });
break;
}
}
}
using (Keyboard.FocusedElement.PreserveFocus())
{

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RefreshInternalAsync captures a path and later awaits GetMetadataFileAsync(). During that await, the user can change the selection; when execution resumes, SelectNode(FindNodeByPath(path, ...)) may unexpectedly revert the selection to the stale path. Consider capturing the initial selection/path and, before calling SelectNode/RefreshDecompiledView, verifying the selection is still the same (or using a refresh generation id so stale refreshes don't apply).

Suggested change
var currentPath = GetPathForNode(SelectedItem);
var selectionChanged = SelectedItem is not null
&& !((path == null && currentPath == null)
|| (path != null && currentPath != null && path.SequenceEqual(currentPath)));
if (selectionChanged)
return;

Copilot uses AI. Check for mistakes.
SelectNode(FindNodeByPath(path, true), inNewTabPage: false);

RefreshDecompiledView();
Expand Down
Loading