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

Blazor server mode: calling jsinterop to cleanup clientside before disposing component causes memory leak in page close/refresh #33535

Closed
alanma75 opened this issue Jun 15, 2021 · 3 comments
Labels
area-blazor Includes: Blazor, Razor Components ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved

Comments

@alanma75
Copy link

alanma75 commented Jun 15, 2021

Description

while developing a custom blazor component, some clientside object need to be cleared before the component being disposed. below sample is a simplified version taken from blazor-university:
https://blazor-university.com/javascript-interop/calling-dotnet-from-javascript/lifetimes-and-memory-leaks/

Dispose method on the page tries to invoke a js method to clear its clientside objects, then DotnetReference is disposed.

what we have noticed is the jsinterop call throws exception while page refresh/close, because jsruntime at that moment is already closed. this exception cause all objects on serverside from the closed circuit to stay in memory. (Take memory snapshot using diagnostic tool, and check ViewModel object, the viewmodel object created from previous circuits stays in memory after page refresh, and with jsinterop call removed from Dispose method, there is always just one Viewmodel in memory.

Configuration

refer this script block in _host.html

    <script>
        var BlazorUniversity = BlazorUniversity || {};
        BlazorUniversity.startRandomGenerator = function(dotNetObject) {
            setInterval(function () {
                let text = Math.random() * 1000;
                console.log("JS: Generated " + text);
                dotNetObject.invokeMethodAsync('AddText', text.toString());
            }, 1000);
            return 1;
        };
    </script>

index.razor

@page "/"
@inject IJSRuntime JSRuntime
@implements IDisposable

<span>Text received</span>

<ul>
    @foreach (string text in TextHistory)
    {
        <li>@text</li>
    }
</ul>

@code
{
    List<string> TextHistory = new List<string>();
    int GeneratorHandle = -1;
    DotNetObjectReference<Index> ObjectReference;
    ViewModel model = new ViewModel() {Name = "Test person"};

    protected override async Task OnAfterRenderAsync(bool firstRender)
    {
        await base.OnAfterRenderAsync(firstRender);
        if (firstRender)
        {
            ObjectReference = DotNetObjectReference.Create(this);
            GeneratorHandle = await JSRuntime.InvokeAsync<int>("BlazorUniversity.startRandomGenerator", ObjectReference);
        }
    }

    [JSInvokable("AddText")]
    public void AddTextToTextHistory(string text)
    {
        TextHistory.Add(text.ToString());
        while (TextHistory.Count > 10) 
            TextHistory.RemoveAt(0);
        StateHasChanged();
        System.Diagnostics.Debug.WriteLine("DotNet: Received " + text);
    }

    public async void Dispose()
    {
        if (GeneratorHandle != -1)
        {
    //Cancel our callback before disposing our object reference, this call cause exception in page close/refresh and block the GC
            await JSRuntime.InvokeVoidAsync("BlazorUniversity.stopRandomGenerator", GeneratorHandle);
        }

        if (ObjectReference != null)
        {
    //Now dispose our object reference so our component can be garbage collected
            ObjectReference.Dispose();
        }
    }

    public class ViewModel
    {
        public string Name { get; set; }
    }
}

Regression?

I have also tried in .net 3.1 and got the same issue.

Other information

what is the correct way to dispose client and server side resources? if this is a razor component, the dispose is called from both partial render when the component is removed from render tree (in this case, page is still active so this jsinterop call works fine), or when page closed (in this case, jsinterop call throw exception, even with try catch, we still observe the locking of all the closed page's object locked by renderer root object.

@mairaw mairaw transferred this issue from dotnet/core Jun 15, 2021
@alanma75 alanma75 changed the title calling jsinterop to cleanup clientside before disposing page causes memory leak in page close/refresh Blazor server mode: calling jsinterop to cleanup clientside before disposing page causes memory leak in page close/refresh Jun 15, 2021
@alanma75 alanma75 changed the title Blazor server mode: calling jsinterop to cleanup clientside before disposing page causes memory leak in page close/refresh Blazor server mode: calling jsinterop to cleanup clientside before disposing component causes memory leak in page close/refresh Jun 15, 2021
@javiercn javiercn added the area-blazor Includes: Blazor, Razor Components label Jun 15, 2021
@glararan
Copy link

glararan commented Jun 15, 2021

Implement use of CancellationTokenSource e.g. https://www.meziantou.net/canceling-background-tasks-when-a-user-navigates-away-from-a-blazor-component.htm

pass CTS.Token as second parameter in method InvokeVoidAsync. In Dispose method call CTS.Cancel().

Exception should no longer occur.

EDIT: Dispose method should have argument bool disposing if I am remember correct

@SteveSandersonMS
Copy link
Member

what we have noticed is the jsinterop call throws exception while page refresh/close, because jsruntime at that moment is already close

Am I right in thinking the exception occurs 1 minute after you close the page? If so, it's exactly the same issue that we recently addressed for .NET 6 in #32901

There are really two different concerns to be aware of here:

  1. In .NET Core 3.1 to 5, JS interop calls after the page is closed will not immediately fail, but rather will time out (with an exception) after 1 minute. In .NET 6 we changed this so the calls will fail immediately with an exception.
  2. You must allow for the JS call to throw an exception. You cannot have disposal logic that only cleans up server-side resources if the JS call doesn't throw, because that's a security issue. A malicious client could always choose to throw, even if your JS code doesn't contain logic that would ever throw. And then the malicious client could consume an unlimited amount of server resources.

So, to write safe disposal logic, always allow for JS interop calls to throw, and still be sure to clean up server resources before or after that. For example,

@implements IAsyncDisposable

...

@code {
    // ... other code goes here ...

    public async Task DisposeAsync()
    {
        try
        {
            await JSRuntime.InvokeVoidAsync("BlazorUniversity.stopRandomGenerator", GeneratorHandle);
        }
        finally
        {
            ObjectReference?.Dispose();
        }
    }
}

@SteveSandersonMS SteveSandersonMS added the ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. label Jun 15, 2021
@ghost ghost added the Status: Resolved label Jun 15, 2021
@ghost
Copy link

ghost commented Jun 16, 2021

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

@ghost ghost closed this as completed Jun 16, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 16, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved
Projects
None yet
Development

No branches or pull requests

4 participants