Avoid trying to use _hotReloadAgent variable before it is initialized #46916
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,11 +26,6 @@ public static class WebAssemblyHotReload | |
|
|
||
| internal static async Task InitializeAsync() | ||
| { | ||
| if (Environment.GetEnvironmentVariable("__ASPNETCORE_BROWSER_TOOLS") == "true") | ||
| { | ||
| // Attempt to read previously applied hot reload deltas if the ASP.NET Core browser tools are available (indicated by the presence of the Environment variable). | ||
|
|
@@ -50,14 +45,19 @@ internal static async Task InitializeAsync() | |
| [JSInvokable(nameof(ApplyHotReloadDelta))] | ||
| public static void ApplyHotReloadDelta(string moduleIdString, byte[] metadataDelta, byte[] ilDelta, byte[] pdbBytes) | ||
| { | ||
| // Analyzer has a bug where it doesn't handle ConditionalAttribute: https://github.com/dotnet/roslyn/issues/63464 | ||
| #pragma warning disable IDE0200 // Remove unnecessary lambda expression | ||
| Interlocked.CompareExchange(ref _hotReloadAgent, new HotReloadAgent(m => Debug.WriteLine(m)), null); | ||
|
There was a problem hiding this comment. Why does the initialization in the There was a problem hiding this comment. As the InitializeAsync is an async method what was happening is that in a race condition before _hotReloadAgent is initialized on InitializeAsync it was trying to be used on ApplyHotReloadDelta method and throwing the NRE exception. There was a problem hiding this comment. That sounds like a bug in the consuming code then, rather than here, isn't it? So the consumder then should just make sure they wait for the initialization before trying to apply changes. There was a problem hiding this comment. I'm not sure if it's possible to check if InitializeAsync is done before calling the ApplyHotReloadDelta as it's called by dotnet watch.
There was a problem hiding this comment. It is possible that there is an issue with initialization. I am not sure what exactly the initialization sequence for WASM is supposed to do. It'd be great if anyone knows and can review the code paths. There was a problem hiding this comment. Here is where InitializeAsync is called: And here is where it's calling the applyHotReload function on JS from dotnet watch: Which will call the ApplyHotReloadDelta on C# side: I think there is no way to guarantee that InitializeAsync is done before calling the ApplyHotReloadDelta, I guess that this is the correct fix. |
||
| #pragma warning restore IDE0200 // Remove unnecessary lambda expression | ||
|
|
||
| var moduleId = Guid.Parse(moduleIdString, CultureInfo.InvariantCulture); | ||
|
|
||
| _updateDeltas[0].ModuleId = moduleId; | ||
| _updateDeltas[0].MetadataDelta = metadataDelta; | ||
| _updateDeltas[0].ILDelta = ilDelta; | ||
| _updateDeltas[0].PdbBytes = pdbBytes; | ||
|
|
||
| _hotReloadAgent.ApplyDeltas(_updateDeltas); | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will a new HotReloadAgent be initialized even in case if the value shouldn't be replaced here? I assume the compiler is smart enough not to do that, but I'm not sure, hence my question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interlocked.CompareExchange is thread safe and will avoid creating two HotReloadAgent in two different threads.
It compares two values for equality and, if they are equal, replaces the first value.
https://learn.microsoft.com/en-us/dotnet/api/system.threading.interlocked.compareexchange?view=net-7.0#system-threading-interlocked-compareexchange(system-object@-system-object-system-object)