Support async main #17673

SteveSandersonMS · 2019-12-07T01:16:03Z

It's possible that Mono might add support for async main in the future (what do you think, @lewing?) but in the meantime this should meet our requirements for @rynowak's proposed improved startup APIs.

This also fixes the issue where the gold error bar wasn't appearing for synchronous startup exceptions. It now does that for both sync and async entrypoint exceptions. Also if you use Blazor.start in JS, the promise it returns reflects the "async main" result, so you can suppress or display the errors in whatever way you want.

If anyone knows a more official way to detect the async version of the entrypoint, please let me know, though I doubt there is one.

I'll add E2E tests later (before merging this).

pranavkm · 2019-12-07T03:02:49Z

src/Components/Blazor/Blazor/src/Hosting/EntrypointInvoker.cs

+
+        private static MethodBase CreateMethodBase(IntPtr methodHandleValue)
+        {
+            var methodHandleCtor = typeof(RuntimeMethodHandle).GetConstructor(BindingFlags.NonPublic | BindingFlags.Instance, null, new[] { typeof(IntPtr) }, null);


This gives me pause since RuntimeMethodHandle doesn't have any public constructors. This seems like Mono should give us an API

This gives me pause since RuntimeMethodHandle doesn't have any public constructors. This seems like Mono should give us an API

If this wasn't so low-level, I'd also be concerned. However I see this as being a polyfill in place of native async-main support, so use of private reflection feels reasonable and proportionate. If at some point Mono can offer this feature natively we'll switch to that, and in the meantime all we need is for this not to get broken, which I'm pretty sure will not be a problem for Mono.

Let's raise this with @lewing and get an agreement about whether (a) Mono could do this natively in the near future, or (b) the Mono team can at least be aware we're doing this and agree it's acceptable and won't break it, including when switching to CoreFx.

won't break it, including when switching to CoreFx.

This seems exceptionally unlikely to be remembered and dealt with 😆

Thanks to a suggestion from @vargaz, this private reflection approach is now eliminated in favour of a regular public API solution, so I think the key problem is gone.

pranavkm · 2019-12-07T03:05:46Z

src/Components/Blazor/Blazor/src/Hosting/EntrypointInvoker.cs

+{
+    internal static class EntrypointInvoker
+    {
+        public static Task InvokeEntrypointAsync(IntPtr entrypointMethodHandleValue, string[] args)


Do you think it's easy to write a unit test for this?

I'm going to bet the answer is no. We need to have a pointer to a method. I'm sure that's not going to behave the same way on CoreCLR and Mono-WASM

pranavkm · 2019-12-07T03:08:14Z

src/Components/Blazor/Blazor/src/Hosting/EntrypointInvoker.cs

+
+            // We're not handling any errors here, synchronous or asynchronous. That means they'll
+            // bubble up to the caller on the JS side, which can decide what to do with them.
+            return entrypointMethodBase.Invoke(null, new object[] { args }) as Task;


Does this work fine for a no-args Main e.g. public int Main()

also if we ever care about the return value. I don't think we do today.

Good point about no-args. I'll fix that.

As for return value, I don't think that's relevant since Blazor apps aren't console apps. It's hard to see how exit codes could be useful. Even in the edge case where someone really wants this, they could achieve something equivalent through JS interop before exiting.

In the longer term if Mono adds native support for this, they can take care of supplying a Promise<exit code> back to the JS side for more general use cases.

rynowak · 2019-12-08T04:16:17Z

src/Components/Blazor/Blazor/src/Hosting/EntrypointInvoker.cs

+                if (origNameLength > 2)
+                {
+                    var candidateMethodName = origName.Substring(1, origNameLength - 2);
+                    var candidateMethod = entrypointMethodBase.DeclaringType.GetMethod(


Is there anything else encoded in the metadata we could use? (don't have ILSpy handy). Usually the compiler uses attributes for such things.

I don't spot anything useful, unfortunately

rynowak · 2019-12-08T04:21:09Z

src/Components/Web.JS/src/Platform/Mono/MonoPlatform.ts

@@ -263,11 +273,13 @@ function createEmscriptenModuleInstance(loadAssemblyUrls: string[], onReady: ()
      'number',
    ]);

-    mono_call_assembly_entry_point = Module.mono_call_assembly_entry_point;


You can image instead that Module.mono_call_assembly_entry_point would just return a value, which could a Task, int, or Task<int>. That seems problematic because from the JS side we don't know the value's type - which leads back to us wanting to call the entry point ourselves.

If Mono adds native support for async main, I think they should create a new entrypoint API called something like mono_call_assembly_entry_point_async, which returns a Promise that:

resolves with either null/undefined in the case of a void or Task main

or resolves with a number for a main of type intor Task<int>

or rejects with some representation of an exception for either sync or async failures.

I don't think there's any problem on the JS side with needing to know the type in advance.

Its possible to do this more reliably by calling the 'mono_assembly_get_object' mono API function which returns an
Assembly object, then calling its EntryPoint property from c#. mono_assembly_get_object () is not currently exposed to js, but we can add it if needed.

MonoReflectionAssembly* mono_assembly_get_object (MonoDomain *domain, MonoAssembly *assembly);

Thanks @vargaz - that's a great suggestion!

I realised we don't need mono_assembly_get_object either, as we can use Assembly.Load(string) on the .NET side to get the same result. If you think mono_assembly_get_object would be faster for any reason, then yes it would be great to have that exposed to JS. Would it be correct to expect that it wouldn't really be significantly faster, since Assembly.Load(string) is returning the Assembly instance already cached in memory anyway (since we already loaded it inside the bootup js)?

mono_assembly_get_object () would be a bit faster. Will look into adding it.

rynowak · 2019-12-08T04:21:23Z

src/Components/Blazor/Blazor/src/Hosting/EntrypointInvoker.cs

+
+        private static MethodBase CreateMethodBase(IntPtr methodHandleValue)
+        {
+            var methodHandleCtor = typeof(RuntimeMethodHandle).GetConstructor(BindingFlags.NonPublic | BindingFlags.Instance, null, new[] { typeof(IntPtr) }, null);


This gives me pause since RuntimeMethodHandle doesn't have any public constructors. This seems like Mono should give us an API

rynowak · 2019-12-08T04:22:35Z

src/Components/Blazor/Blazor/src/Hosting/EntrypointInvoker.cs

+
+            // For "async Task Main", the C# compiler generates a method called "<Main>"
+            // that is marked as the assembly entrypoint. Detect this case, and instead of
+            // calling "<Whatever>", call the sibling "Whatever".


What's the reason why we need to call Main instead of <Main> ? Is this because <Main> (generated wrapper) contains a Task.GetAwaiter.GetResult()?

rynowak · 2019-12-08T04:23:24Z

src/Components/Blazor/Blazor/src/Hosting/EntrypointInvoker.cs

+
+            // We're not handling any errors here, synchronous or asynchronous. That means they'll
+            // bubble up to the caller on the JS side, which can decide what to do with them.
+            return entrypointMethodBase.Invoke(null, new object[] { args }) as Task;


also if we ever care about the return value. I don't think we do today.

rynowak · 2019-12-08T04:24:58Z

src/Components/Web.JS/src/Platform/Mono/MonoPlatform.ts

+
+    // Since no .NET code has run yet, manually initialize JS interop
+    const ensureJsInteropInitializedMethod = this.findMethod('Microsoft.AspNetCore.Blazor', 'Microsoft.AspNetCore.Blazor.Services', 'WebAssemblyJSRuntime', 'EnsureInitialized');
+    this.callMethod(ensureJsInteropInitializedMethod, null, []);


Not that we allow it today, because this will make it impossible for users to do anything that configures JS Interop in their main method (before "blazor startup"). Does it make sense for us to use unmarshalled interop for these kinds of things?

I realize the reason you probably went this route is for the async stuff 😆

Yes, I did do this with unmarshalled interop originally, but then switched to JSInterop when it seemed necessary to get all the async-Promise mapping stuff.

However, as per my comments on your startup APIs gist, I'd now like to change this design. If the main method becomes async and runs for the lifetime of the app, then it no longer makes sense for the Promise here to reflect the lifetime of the main method.

Instead, Blazor.start should only represent the startup process and not the execution of the main method. This makes it useful for things like "stop showing a progress bar once the .NET code starts running". In the case where someone does want the JS side to response once the Blazor app completes entirely (e.g., due to an unhandled exception), that's unusual but can be achieved quite conveniently through JS interop.

So with the updated design it won't be necessary to use JS interop in the code here, and I'll go back to the simpler unmarshalled interop.

…fers only to the startup process, not the async execution of Main.

SteveSandersonMS added area-blazor Includes: Blazor, Razor Components feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly labels Dec 7, 2019

SteveSandersonMS requested review from pranavkm, rynowak and javiercn December 7, 2019 01:16

pranavkm approved these changes Dec 7, 2019

View reviewed changes

rynowak reviewed Dec 8, 2019

View reviewed changes

SteveSandersonMS added 12 commits December 10, 2019 13:41

Support async main

5cfaa09

Call entrypoint via unmarshalled interop. The returned promise now re…

6f4f69a

…fers only to the startup process, not the async execution of Main.

Clear up error cases

d606b5f

Also support zero-arg main

f34e287

Clean up usings

f1ff236

Add missing test project to sln

2738411

Simplify by finding entrypoint on .NET side

a0cc4c3

Unit tests for EntrypointInvoker

293d547

Make error notification test file names/layout consistent with others

8ded549

Make WebAssembly E2E tests all use async main

b8e0cb2

E2E tests for startup error notification

17e9790

Make E2E tests actually prove something

f8b30c8

SteveSandersonMS force-pushed the stevesa/support-async-main branch from e445117 to f8b30c8 Compare December 10, 2019 13:48

Fix E2E tests

3522a06

SteveSandersonMS merged commit 3a93704 into blazor-wasm Dec 12, 2019

SteveSandersonMS deleted the stevesa/support-async-main branch December 12, 2019 15:05

SteveSandersonMS mentioned this pull request Dec 12, 2019

Support async main #17823

Closed

SteveSandersonMS mentioned this pull request Jan 3, 2020

Async initialization in Main not working #18104

Closed

Support async main #17673

Support async main #17673

Uh oh!

Conversation

SteveSandersonMS commented Dec 7, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveSandersonMS Dec 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveSandersonMS Dec 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vargaz Dec 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SteveSandersonMS Dec 9, 2019 •

edited

Loading

SteveSandersonMS Dec 9, 2019 •

edited

Loading

vargaz Dec 9, 2019 •

edited

Loading