The problem

Things can quickly go wrong if a hot reload occurs mid-frame. Imagine half of a frame running with old code, and the other half running with new code.

while(applicationIsRunning)
{
    // The less rest time between each frame, the higher the probablity 
    // of a hot-reload occurring mid-frame, possibly resulting in invalid application state.
    RenderFrame(); 
    Rest();
}

Invalid application state can easily cause any number of issues. Everything from hard crashes, to mysterious bugs that send developers on goose chases, debugging problems that could have never occurred in the first place.

All applications are subject to these kinds of problems, but there is an easy solution, and one that is especially easy to implement in software that has a notion of a "frame" or a "tick", as shown in the example above.

Proposed Solution

We need to be able to tell the Hot Reload agent, not to patch our application while a frame is being computed. There are many ways to achieve this, so this is simply meant to get the ball rolling.

I propose you provide the ability to acquire a handle, from which you can interact with the Hot Reload agent. Upon requesting it, it is then ensured that only the first host requesting it gets the right to use it. That way, the API makes it clear that specifying sync-points is not something that should be done arbitrary places, without considering other sync-points, as they would quickly start conflicting with each other. If it is possible for a process with multiple threads to sync up with one global sync-point, it would still be possible for the developer to ensure that manually.

The handle itself can be acquired in many different ways, but since you're already providing hooks via MetadataUpdateHandlerAttribute, I assume we can also get some kind of handle going forward, the question then becomes, what does the handle need to do?

Option 1) (Bad) Let us pause and continue the hot reload agent

I wanted to mention this solution because it might be a solution one could dream up, but it has problems.

while(applicationIsRunning)
{
    hotReloadHandle.Pause();
    RenderFrame();
    hotReloadHandle.Continue();
    Rest();
}

Why is it bad?

While I'm sure that there are cases where a sync-point would happen so rarely that specifying a period where hot reloading should not occur would make more sense, a sync-point is much more flexible, and you get a lot more control, and while achieving a sync-point effect using the API above is possible, it would look kind of silly, and it wouldn't even be obvious that it would work:

hotReloadHandle.Pause();
while(applicationIsRunning)
{
    RenderFrame();
    Rest();
    hotReloadHandle.Continue();
    hotReloadHandle.Pause();
}

Option 2) Let us introduce a sync point

This is how Live++ (Hot Reloading for c++) solves this issue: https://liveplusplus.tech/docs/documentation.html#enabling_livepp

while(applicationIsRunning)
{
    hotReloadHandle.SyncPoint(out var types);

    bool didPatch = types?.Length != 0; // And perhaps the syncPoint could return a bool.

    RenderFrame();
}

The downside of this approach is that you do not get to clear cashes before the hot reload takes place. So that is why I'd go straight to something like option 3.

Option 3) Give us full control

If you agree that providing a way to solve this problem is a good idea, I would encourage you to consider a more explicit implementation. It would both open up many more possibilities and encourage developers to consider them and provide a great hotload experience. With this approach, developers can profile things, provide user feedback, and so on..

while(applicationIsRunning)
{
    RenderFrame();

    // Returns true when types has been queued up for patching.
    if (hotReloadHandle.RequireHotReload()) 
    {
        // Lock in on what is about to be hotreloaded, **but do not apply the patch yet** !!!!!!!
        hotReloadHandle.BeginHotReload(out var types);

        // Do what needs to be done with the **old code** before the hotload takes place
        ClearCachesAndWhatNotUsingOldCode();

        // Trigger all MetadataUpdateHandlerAttribute events and 
        // Patch assemblies / perform the hot reload
        hotReloadHandle.EndHotReload(); 
    }
}

Other suggestions to further improve the hot relaoding experience:

Add the ability to control if a full reload is required or not.

I feel like this one is self explanitory. Some code changes can only, and should only, have an effect with a full reload.

[assembly: MetadataUpdateHandler(typeof(UpdateHandler))]    
internal static class UpdateHandler
{
        public static bool ShouldPerformFullReload(Type[] types)
        {
            if (types.Contains(typeof(Startup)))
            {
                return true;
            }

            return false;
        }
}

Or / and: one might imagine an attribute, that makes specifying this behaviour more easy:

[System.Reflection.Metadata.NotHotReloadable] // Or:
[System.Reflection.Metadata.HotReloading.RequireFullReload]
public class Startup
{

}

A big problem with UpdateApplication and ClearCache

https://docs.microsoft.com/en-us/dotnet/api/system.reflection.metadata.metadataupdatehandlerattribute?view=net-6.0

UpdateApplication and ClearCache do not block the Hot Reload! Both of them should be running before the patch takes place, and perhaps that is when they are invoked?

Maybe this was done deliberately because something was slow and you wanted to speed up hot-reloading? If not blocking is the intended behaviour for this. At least getting a OnBeboreHotReload(Type[] types) where we know that it is the old code clearing doing the work, would be appreciated.

(Of course, if the ability to add a sync point is added in a good way, we can clear caches safely there too)

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

Slightly related to this is also the way dotnet watch (at least) restarts the process on rude edits. It seems that it plainly just kills the process? I'd like to see that this aspect was a bit more cooperative, too.

I'm not aware of the platform specifics, but would it be possible to first call CloseMainWindow on the process, and only after a timeout kill it? This would allow the application to save its state for the next run, or do any special cleanup it needs to.

Slightly related to this is also the way dotnet watch (at least) restarts the process on rude edits. It seems that it plainly just kills the process? I'd like to see that this aspect was a bit more cooperative, too.

I'm not aware of the platform specifics, but would it be possible to first call CloseMainWindow on the process, and only after a timeout kill it? This would allow the application to save its state for the next run, or do any special cleanup it needs to.

I've been wanting this too (I believe it closes gracefully through VS btw). In my case, I only want to save and recover state, if the window was closed and opened by a watch run session, and not when opened from scratch. Perhaps you can somehow go through the parent process and figure that out, but they could also introduce an OnBeforeReload event or something equivalent, where we could do that work.

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

Tagging subscribers to this area: @tommcdon See info in area-owners.md if you want to be subscribed.

Issue Details

# The problem Things can quickly go wrong if a hot reload occurs mid-frame. Imagine half of a frame running with old code, and the other half running with new code. ```cs while(applicationIsRunning) { // The less rest time between each frame, the higher the probablity // of a hot-reload occurring mid-frame, possibly resulting in invalid application state. RenderFrame(); Rest(); } ``` Invalid application state can easily cause any number of issues. Everything from hard crashes, to mysterious bugs that send developers on goose chases, debugging problems that could have never occurred in the first place. All applications are subject to these kinds of problems, but there is an easy solution, and one that is especially easy to implement in software that has a notion of a "frame" or a "tick", as shown in the example above. # Proposed Solution We need to be able to tell the Hot Reload agent, _not_ to patch our application while a frame is being computed. There are many ways to achieve this, so this is simply meant to get the ball rolling. I propose you provide the ability to acquire a handle, from which you can interact with the Hot Reload agent. Upon requesting it, it is then ensured that only the first host requesting it gets the right to use it. That way, the API makes it clear that specifying sync-points is not something that should be done arbitrary places, without considering other sync-points, as they would quickly start conflicting with each other. If it is possible for a process with multiple threads to sync up with one global sync-point, it would still be possible for the developer to ensure that manually. The handle itself can be acquired in many different ways, but since you're already providing hooks via MetadataUpdateHandlerAttribute, I assume we can also get some kind of handle going forward, the question then becomes, what does the handle need to do? ## Option 1) (Bad) Let us pause and continue the hot reload agent I wanted to mention this solution because it might be a solution one could dream up, but it has problems. ```cs while(applicationIsRunning) { hotReloadHandle.Pause(); RenderFrame(); hotReloadHandle.Continue(); Rest(); } ``` **Why is it bad?** While I'm sure that there are cases where a sync-point would happen so rarely that specifying a period where hot reloading should not occur would make more sense, a sync-point is much more flexible, and you get a lot more control, and while achieving a sync-point effect using the API above is possible, it would look kind of silly, and it wouldn't even be obvious that it would work: ```cs hotReloadHandle.Pause(); while(applicationIsRunning) { RenderFrame(); Rest(); hotReloadHandle.Continue(); hotReloadHandle.Pause(); } ``` ## Option 2) Let us introduce a sync point This is how Live++ (Hot Reloading for c++) solves this issue: [https://liveplusplus.tech/docs/documentation.html#enabling_livepp](https://liveplusplus.tech/docs/documentation.html#enabling_livepp) ```cs while(applicationIsRunning) { hotReloadHandle.SyncPoint(out var types); bool didPatch = types?.Length != 0; // And perhaps the syncPoint could return a bool. RenderFrame(); } The downside of this approach is that you do not get to clear cashes before the hot reload takes place. So that is why I'd go straight to something like option 3. ``` ## Option 3) Give us full control If you agree that providing a way to solve this problem is a good idea, I would encourage you to consider a more explicit implementation. It would both open up many more possibilities and encourage developers to consider them and provide a great hotload experience. With this approach, developers can profile things, provide user feedback, and so on.. ```cs while(applicationIsRunning) { RenderFrame(); // Returns true when types has been queued up for patching. if (hotReloadHandle.RequireHotReload()) { // Lock in on what is about to be hotreloaded, **but do not apply the patch yet** !!!!!!! hotReloadHandle.BeginHotReload(out var types); // Do what needs to be done with the **old code** before the hotload takes place ClearCachesAndWhatNotUsingOldCode(); // Trigger all MetadataUpdateHandlerAttribute events and // Patch assemblies / perform the hot reload hotReloadHandle.EndHotReload(); } } ``` # Other suggestions to further improve the hot relaoding experience: ## Add the ability to control if a full reload is required or not. I feel like this one is self explanitory. Some code changes can only, and should only, have an effect with a full reload. ```cs [assembly: MetadataUpdateHandler(typeof(UpdateHandler))] internal static class UpdateHandler { public static bool ShouldPerformFullReload(Type[] types) { if (types.Contains(typeof(Startup))) { return true; } return false; } } ``` Or / and: one might imagine an attribute, that makes specifying this behaviour more easy: ```cs [System.Reflection.Metadata.NotHotReloadable] // Or: [System.Reflection.Metadata.HotReloading.RequireFullReload] public class Startup { } ``` ## A big problem with UpdateApplication and ClearCache [https://docs.microsoft.com/en-us/dotnet/api/system.reflection.metadata.metadataupdatehandlerattribute?view=net-6.0](https://docs.microsoft.com/en-us/dotnet/api/system.reflection.metadata.metadataupdatehandlerattribute?view=net-6.0) UpdateApplication and ClearCache do not block the Hot Reload! Both of them _should_ be running _before_ the patch takes place, and perhaps that is when they are invoked? Maybe this was done deliberately because something was slow and you wanted to speed up hot-reloading? If not blocking is the intended behaviour for this. At least getting a `OnBeboreHotReload(Type[] types)` where we know that it is the old code clearing doing the work, would be appreciated. (Of course, if the ability to add a sync point is added in a good way, we can clear caches safely there too)

Author:	BjarkeCK
Assignees:	-
Labels:	`area-Diagnostics-coreclr`, `untriaged`
Milestone:	-

Extending @BjarkeCK's point, it'd be useful for various live-development scenarios to know when a rude edit happens.

Option 3 could be extended as follows. On code edit:

The runtime signals to application whether an unpatched edit has been made and whether a hot-reload can be applied
The application can request the runtime continue executing (or alternatively, has the responsibility to request the reload).

class HotReloadHandle
{
        // APIs for polling hot reload state
    bool IsUnpatchedEditMade { get; }
    bool IsUnpatchedRudeEditMade { get; }
        bool IsEditPatchable { get; }
        Type[] EditedTypes { get; }
        Type[] PatchableTypes { get; }

        // APIs for applying hot reloads
        // returns true if hot reload begins, outputs patchableTypes if so.
        // returns false if hot reload doesn't begin.
        //     - Either RudeTypes is empty (no changes available, so no hotreload happened) or
        //        RudeTypes is populated (changes were available but couldn't be applied).
        bool TryBeginHotReload(out Type[] patchableTypes, out Type[] rudeTypes);
        void EndHotReload();
        void CancelHotReload();
}

Here's the modified example:

while(applicationIsRunning)
{
    RenderFrame();

        // assume the below collection tostring expands to a select + string.join
    Console.WriteLine($"Hot Reload Edited {hotReloadHandle.IsUnpatchedEditMade} Patchable {hotReloadHandle.IsEditPatchable} Rude {hotReloadHandle.IsUnpatchedRudeEditMade} Types {hotReloadHandle.EditedTypes} {hotReloadHandle.PatchableTypes}");

    if (hotReloadHandle.TryBeginHotReload(out var patchableTypes, out var rudeTypes)) 
    {
        DoWhatever();

        // Trigger all MetadataUpdateHandlerAttribute events and 
        // Patch assemblies / perform the hot reload
        hotReloadHandle.EndHotReload(); // vs CancelHotReload()
    } else {
            // Either a rude edit or no change made
                if (rudeTypes.Length > 0) {
            // Do things like Application.Restart() or DisplayWarning()
                }
    }
}

dotnet / runtime

Improve the overall stability of Hot Reloading by allowing us to declare sync points for the hot reload agent. #64299