Skip to content
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.

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

Avoid null refs in HotReloadAgent when accessing Assembly.Modules #33263

Merged
merged 1 commit into from
Jun 4, 2021

Conversation

pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Jun 3, 2021

Accessing Assembly.Modules for arbitrary loaded assemblies may throw which currently results
in the hot reload agent to produce an unhandled exception crashing the app. This change attempts
to perform a targeted fix for that issue.

Fixes #33152

Accessing Assembly.Modules for arbitrary loaded assemblies may throw which currently results
in the hot reload agent to produce an unhandled exception crashing the app. This change attempts
to perform a targeted fix for that issue.

Fixes #33152
@pranavkm pranavkm requested a review from a team as a code owner June 3, 2021 21:13
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Jun 3, 2021
Copy link
Contributor

@TanayParikh TanayParikh left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -39,7 +39,7 @@ private void OnAssemblyLoad(object? _, AssemblyLoadEventArgs eventArgs)
if (_deltas.TryGetValue(moduleId.Value, out var updateDeltas) && _appliedAssemblies.TryAdd(loadedAssembly, loadedAssembly))
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we use a ConcurrentDictionary<Assembly, Assembly> instead of a ConcurrentBag to protect against duplicate entries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We really need a ConcurrentSet here. ConcurrentBags have no lookup semantics, it gives you an item with no good way of telling you if a specific instance exists. ConcurrentDictionary with a throw away value acts as a reasonable subsitute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
3 participants