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

Tasks returned from JSRuntime.InvokeAsync during OnInitAsync never finishes in preview3 #8274

Closed
kvantetore opened this issue Mar 7, 2019 · 40 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed

Comments

@kvantetore
Copy link

kvantetore commented Mar 7, 2019

Describe the bug

Tasks returned from JSRuntime.InvokeAsync during second call to OnInitAsync (after prerendering) never finishes.

To Reproduce

Create a new razorcomponents project.

Add a js interop function to Index.cshtml and disable prerendering

    <script>
        window.interopDuringOnInit = function() {
            return "Hello World";
        }
    </script>

    <app></app>

    <script src="_framework/components.server.js"></script>

Call js interop during OnAsyncInit in Index.razor

<h1>@Greeting!</h1>

Welcome to your new app.

@functions {
    string Greeting;

    protected override async Task OnInitAsync()
    {
        try
        {
            Console.WriteLine("Calling browser");
            Greeting = await JSRuntime.InvokeAsync<string>("interopDuringOnInit");
            Console.WriteLine("Returned from browser");
        }
        catch (Exception ex)
        {
            Console.WriteLine("Could not invoke interop during OnInit, " + ex.ToString());
        }
    }
}

When OnInitAsync is called as part of an initial load (prerendering disabled) the task never finishes, This leaves the circuit hanging, and no further events are processed.

If this component is loaded from a IUriHelper.OnLocationChange, the above code works as expected.

@muratg muratg added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Mar 7, 2019
@mkArtakMSFT mkArtakMSFT added the area-blazor Includes: Blazor, Razor Components label Mar 7, 2019
@springy76
Copy link

springy76 commented Mar 11, 2019

I just switched an existing app which worked fine to the new version and got the same problem: IJSRuntime.InvokeAsync() neither returns nor throws an error on call paths from OnInitAsync().

Is there a workaround or did I do something fundamentally wrong? (which I don't believe because the same app worked since Blazor 0.3 and did every version upgrade, got converted to server side Razor Components weeks ago.)

@DNF-SaS
Copy link

DNF-SaS commented Mar 11, 2019

Same probleme here - the await for JSRuntime.InvokeAsync() in an component's OnInitAsync() does block forever.

@Knudel
Copy link

Knudel commented Mar 11, 2019

I migrated my code to preview 3 and I also ran into this problem.
Please fix this!

@DNF-SaS
Copy link

DNF-SaS commented Mar 12, 2019

Does anybody has news / a workaround for this issue?

@mkArtakMSFT
Copy link
Member

@javiercn, can you please look into this? Thanks!

@KP-WhatEver
Copy link

I also got that problem. Anything worked fine before.

@kvantetore
Copy link
Author

I have hacked around it by wrapping wrapping the router component in an if (Started), where Started is set to true after a round trip to the client. It's not exactly beautiful (and it breaks prerendering), but it unblocks the issue for me until the underlying issue is fixed.

In App.razor, delay loading the router until we have received a message from the client

@if (Started)
{
    <Router AppAssembly="typeof(Startup).Assembly" />
}
else
{
    <text>Waiting for Asp.Net Core 3.0 Preview3 Workaround...</text>
}

@functions {
    bool Started = false;

    [JSInvokable]
    public void Start()
    {
        Started = true;
        StateHasChanged();
    }

    protected override void OnInit()
    {
        _ = JSRuntime.InvokeAsync<object>("StartMe", new DotNetObjectRef(this));
    }
}

And adding window.StartMe to index.cshtml, calling back to the App component

<script>
    window.StartMe = function (dotnetHelper) {
        dotnetHelper.invokeMethodAsync('Start');
    }
</script>

@vertonghenb
Copy link
Contributor

vertonghenb commented Mar 14, 2019

Not only does this happen on OnInitAsync but also on OnParametersSetAsync.
Using it on a ButtonClick does return.

@plasticalligator
Copy link

I've confirmed that it happens on all of the lifecycle events; the task never returns.
Is there a work around that doesn't break prerendering?

@vertonghenb
Copy link
Contributor

vertonghenb commented Mar 14, 2019

@Honkmother I'm afraid that using any JS interop is not allowed in prerendering. (not related to the bug though)

@plasticalligator
Copy link

I am not calling it during prerendering.

https://github.com/SQL-MisterMagoo/PreRenderComponent allows us to detect when the prerendering is active and avoid making JS Interop calls during it.

@plasticalligator
Copy link

@kvantetore I found an alternative work-around. It would appear if you call the InvokeAsync inside of the body of a component rather than the lifecycle methods it gets properly executed. I don't know exactly when it is executed, but I imagine it is right prior to OnAfterRender.

ex. at the end of Component.razor, prior to @functions { ... }

@ { if (IsPreRendering == false) await JSRuntime.InvokeAsync<string>("interopDuringOnInit"); }

@mkArtakMSFT mkArtakMSFT added bug This issue describes a behavior which is not expected - a bug. and removed investigate labels Mar 19, 2019
@mkArtakMSFT
Copy link
Member

@javiercn can you please confirm whether this is fixed already?

@mkArtakMSFT mkArtakMSFT added this to the 3.0.0-preview4 milestone Mar 19, 2019
@javiercn
Copy link
Member

@mkArtakMSFT It is not.

@chassq
Copy link
Contributor

chassq commented Mar 24, 2019

Jus to add we see the issue as well. The javascript methods below just do console.log. When we use await the second method does not run but if we remove the await then they both run.

In our LayoutMain.cshtml file

    protected override async Task OnInitAsync()
    {
        base.OnInit();

        //Call any js event listener setup. Do this here because need to make sure the .NET side is loaded in the browser.
        await jsRuntime.InvokeAsync<bool>("initScreenFull");
        await jsRuntime.InvokeAsync<bool>("initializeListeners");
        
    }

@SteveSandersonMS
Copy link
Member

Actually the right fix might just be to guarantee we never pause the SignalR message loop.

@rynowak
Copy link
Member

rynowak commented Mar 27, 2019

Since we have our own sync context I agree with all of the above.

The reason to pause the message loop is to guarantee ordering. We implement ordering ourselves.

@davidfowl
Copy link
Member

I was going to comment on this but I was on vacation 😄. Sorry you had to spend your time investigating this @SteveSandersonMS. There's an issue in SignalR to allow a maximum number of concurrent invocations (it should really be more than 1), that should help solve this issue.

cc @anurse @BrennanConroy @halter73

@javiercn
Copy link
Member

Proposed solution: StartCircuit should not wait for quiescence. It should only wait for the initial render.

Agree. It should not wait at all, and deal with errors in the way we do in other places. We should just wrap the task to get notified and that's it.

Once StartCircuit has been called the connection must be up and running already so UpdateDisplay should work just fine.

@analogrelay
Copy link
Contributor

There's an issue in SignalR to allow a maximum number of concurrent invocations (it should really be more than 1), that should help solve this issue.

#5351

If this is important for Razor Components, please let us know so we can prioritize appropriately.

@SteveSandersonMS
Copy link
Member

Turns out that in general we don't block the SignalR message loop. In all cases except one we just hand off to the circuit's dispatcher and the call is synchronous from SignalR's perspective.

The one bad case was StartCircuit which did not do this. I've fixed this in #8863 by making CircuitHost.Initialize synchronous. The upstream caller is free to continue handling other incoming messages as soon as CircuitHost.Initialize has dispatched the "init" logic to the renderer's sync context, because that's then guaranteed to run and complete before anything else gets handled.

@SteveSandersonMS
Copy link
Member

@davidfowl Thanks for the info. I wasn't regarding this as a SignalR issue since, even if SignalR did allow concurrent hub method invocations, we'd have had a different problem where you could have received incoming JSInterop calls before the circuit was flagged as started. The underlying false assumption is now fixed in #8863 so neither problem occurs.

Totally agree it would be good for SignalR to have the flexibility to handle parallel messages in general though.

@SteveSandersonMS SteveSandersonMS added Done This issue has been fixed and removed 2 - Working labels Mar 28, 2019
@mkArtakMSFT mkArtakMSFT removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels May 9, 2019
@Bernardo-Castilho
Copy link

Has this been fixed yet?
I can reproduce this using the latest public version.

This returns a Task that never ends:
var cnt = JSRuntime.InvokeAsync("interop.primes", n);

This does not return at all, no matter where I call it from.
var cnt = await JSRuntime.InvokeAsync("interop.primes", n);

Unless I am missing something, this means I can't get results back from JavaScript at all.

@cosXsinX
Copy link

cosXsinX commented Nov 18, 2019

Actually to understand what happen when trying to launch a task on Blazor you should understand JavaScript execution lifecycle.

To sum up, any task in a JavaScript environment in the context of the browser (Safari, Chrome , Edge, ...) cannot be executed when the current task is running. That means you are dead locking when trying to wait the initiated task by JSRuntime.InvokeAsync<...>(“functionName”,...).AsTask().Wait() in a “synchronous” Blazor function.

The solution is to always perform those task calls in an async containing function and call StateIsChanged() function to say to the browser that the model was updated and that it should refresh the DOM

@gbukauskas
Copy link

gbukauskas commented Dec 6, 2019

I got the same problem calling Google Maps API in component's Dispose method (I need to remove markers on map). I solved the problem setting limit of waiting time:

public void Dispose()
{
	try
	{
		var tasks = new Task[2]
		{
			Task.Run(async () => await GoogleMapInterop.RemoveAllMarkers()),
			Task.Run(async () => await GoogleMapInterop.RemoveAllPaths())
		};
		Task.WaitAll(tasks, TimeSpan.FromSeconds(1));
	}
	catch (Exception)
	{
		// Ignore any bugs
	}
}

Error occurs after Refresh page (F5). There is no workaround thus I had to hide it.

@springy76
Copy link

Doesn't Blazor support IAsyncDisposable? Task.WaitAll feels so aged.

@gbukauskas
Copy link

This method is called from Dispose: I need to call GooleMapsAPI for removing markers and paths from map. All works fine except page refresh (F5). Do you know another way? Please tell it me.

@chucker
Copy link

chucker commented Dec 17, 2019

Doesn't Blazor support IAsyncDisposable? Task.WaitAll feels so aged.

Not yet.

@gbukauskas
Copy link

gbukauskas commented Dec 17, 2019 via email

@pranavkm
Copy link
Contributor

Hi, it looks like you are posting on a closed issue/PR/commit!

We're very likely to lose track of your bug/feedback/question unless you:

  1. Open a new issue
  2. Explain very clearly what you need help with
  3. If you think you have found a bug, include detailed repro steps so that we can investigate the problem

Thanks!

@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed
Projects
None yet
Development

No branches or pull requests