Fixed: Launcher freezing on startup when fetching mod update metadata#919
Merged
Sewer56 merged 5 commits intoJul 25, 2026
Merged
Conversation
Closed
WalkthroughThe update summary now provides asynchronous metadata retrieval with cached results and a synchronous compatibility wrapper. The launcher fetches this metadata before opening the update dialog. A new dialog view model constructor accepts the precomputed updates and initializes summary, size, selection, and download state fields. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
source/Reloaded.Mod.Loader.Update/Structures/ModUpdateSummary.cs (1)
70-92: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider
ConfigureAwait(false)for library-internal awaits.This is reusable library code (
Reloaded.Mod.Loader.Update) whose async method may be awaited directly (Update.cs) or blocked-on via the sync wrapper. Per general async guidance, In library code, you can never use the context, so you should always use ConfigureAwait(false) on every await to avoid needless continuation marshaling back to a captured context.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/Reloaded.Mod.Loader.Update/Structures/ModUpdateSummary.cs` around lines 70 - 92, Update every await in the affected ModUpdateSummary async flow, including GetDownloadFileSizeAsync, GetReleaseMetadataAsync, and DownloadNuspecReaderAsync, to use ConfigureAwait(false). Apply this consistently to all library-internal awaits in the containing method without changing exception handling or result-processing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@source/Reloaded.Mod.Launcher.Lib/Models/ViewModel/Dialog/ModUpdateDialogViewModel.cs`:
- Around line 44-56: Update the public ModUpdateDialogViewModel constructor to
handle an empty updateInfo array before accessing UpdateInfo[0]. Preserve the
existing initialization for non-empty arrays and establish an appropriate
no-selection state when no updates are provided.
In `@source/Reloaded.Mod.Loader.Update/Structures/ModUpdateSummary.cs`:
- Around line 64-95: Guard the NuGet changelog fallback in the update-info
computation with its own try/catch, covering resolver settings access,
repository creation, nuspec download, and release-notes extraction so failures
leave changelog unset and do not abort processing other mods. Also replace the
direct IPackageResolverDownloadSize cast with a safe capability check and only
fetch download size when the resolver implements that interface, preserving the
existing default size when unsupported or failing.
---
Nitpick comments:
In `@source/Reloaded.Mod.Loader.Update/Structures/ModUpdateSummary.cs`:
- Around line 70-92: Update every await in the affected ModUpdateSummary async
flow, including GetDownloadFileSizeAsync, GetReleaseMetadataAsync, and
DownloadNuspecReaderAsync, to use ConfigureAwait(false). Apply this consistently
to all library-internal awaits in the containing method without changing
exception handling or result-processing behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e1b69d7c-05c6-4f7c-87a6-c4195c701234
📒 Files selected for processing (3)
source/Reloaded.Mod.Launcher.Lib/Models/ViewModel/Dialog/ModUpdateDialogViewModel.cssource/Reloaded.Mod.Launcher.Lib/Update.cssource/Reloaded.Mod.Loader.Update/Structures/ModUpdateSummary.cs
Sewer56
reviewed
Jul 23, 2026
Member
|
Aight, I see the LLM shenanigans going on here. Though it remains a curious case why an update check deadlocked in the first place. |
This is internal code, so no need for a backwards compatible overload" Also cleaned up the doc comment on the remaining constructor (the "Prefer this overload" note was stale after the removal).
…teSummary.GetUpdateInfoAsync - Replaced direct IPackageResolverDownloadSize cast with is pattern check so resolvers not implementing it no longer throw InvalidCastException; download size remains default (0) when unsupported or failing, matching the existing safe pattern in UpdateDownloadablePackage. - Wrapped NuGet nuspec-based changelog extraction in its own try/catch so failures in resolver-settings access, repository creation, nuspec download, or release-notes parsing leave changelog unset and do not abort the per-mod iteration.
…eInfo array Replace unconditional UpdateInfo[0] access with a length check. Empty arrays now set SelectedUpdate to null and CanDownload to false, preventing an IndexOutOfRangeException when no updates are provided.
Library-internal async calls should not capture the synchronization context under the default assumption of library usage. Applied ConfigureAwait(false) to GetDownloadFileSizeAsync, GetReleaseMetadataAsync, and DownloadNuspecReaderAsync consistently within the method. Exception handling (empty catch blocks) and result processing remain unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/Reloaded.Mod.Loader.Update/Structures/ModUpdateSummary.cs (1)
53-54: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInvalidate the cached update array when update pairs change.
After the first metadata fetch,
RemoveByModIdreplacesManagerModResultPairsbut_updatesremains populated. Subsequent calls return removed mods, so the dialog can calculate download size and availability from stale entries.🛠️ Proposed fix
public void RemoveByModId(IEnumerable<string> disabledModIds) { + _updates = null; var idToItemDict = ManagerModResultPairs.ToDictionary(pair => pair.ModTuple.Config.ModId);Also applies to: 107-108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/Reloaded.Mod.Loader.Update/Structures/ModUpdateSummary.cs` around lines 53 - 54, Invalidate the cached _updates array whenever ManagerModResultPairs changes, including the update paths in RemoveByModId and the corresponding change around the second referenced location. Ensure subsequent metadata fetches rebuild the update summary from the current pairs instead of returning entries removed by mod ID, while preserving the existing cache behavior when the pairs are unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/Reloaded.Mod.Loader.Update/Structures/ModUpdateSummary.cs`:
- Line 72: Update CheckForModUpdatesAsync and its GetUpdateInfoAsync
metadata-task flow to enforce a bounded timeout or cancellation deadline,
ensuring slow requests cannot keep the UI pending indefinitely. When the
deadline is exceeded, return the available best-effort update results instead of
propagating the timeout, while preserving normal completion behavior.
---
Outside diff comments:
In `@source/Reloaded.Mod.Loader.Update/Structures/ModUpdateSummary.cs`:
- Around line 53-54: Invalidate the cached _updates array whenever
ManagerModResultPairs changes, including the update paths in RemoveByModId and
the corresponding change around the second referenced location. Ensure
subsequent metadata fetches rebuild the update summary from the current pairs
instead of returning entries removed by mod ID, while preserving the existing
cache behavior when the pairs are unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 934d8ec3-3e4d-4448-8815-6310455ea4f9
📒 Files selected for processing (2)
source/Reloaded.Mod.Launcher.Lib/Models/ViewModel/Dialog/ModUpdateDialogViewModel.cssource/Reloaded.Mod.Loader.Update/Structures/ModUpdateSummary.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- source/Reloaded.Mod.Launcher.Lib/Models/ViewModel/Dialog/ModUpdateDialogViewModel.cs
Member
|
Cleaned up and ready to go. Thanks for contributing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #910
Root cause:
ModUpdateSummary.GetUpdateInfo()performs per-mod network requests(download sizes, changelogs, nuspec fetches) with no timeouts, wrapped in
Task.Run(...).Wait(). It was called from theModUpdateDialogViewModelconstructor,which runs on the UI thread inside
SynchronizationContext.Send— so any slow orunresponsive package endpoint froze the launcher indefinitely on startup.
Full WinDbg analysis of a hang dump is in #910.
Change:
ModUpdateSummary: addedGetUpdateInfoAsync(); the syncGetUpdateInfo()remainsas a wrapper for compatibility.
Update.CheckForModUpdatesAsync: awaits the metadata fetch on the background threadbefore marshaling to the UI thread.
ModUpdateDialogViewModel: new overload accepts precomputed update info; the dialogconstructor now does no I/O.
Verified: Full solution builds (VS Community, v142 toolset for the bootstrapper).
Launched the built Reloaded-II.exe on the previously-freezing setup (Win11 26100,
mods with pending updates): startup completes and the update dialog appears normally.
All build warnings are pre-existing on master.
Note: the underlying requests still lack timeouts — a hung endpoint now stalls the
background check silently rather than the UI. Left out of this PR to keep it minimal;
happy to add a cancellation ceiling as a follow-up if wanted.