Skip to content

Remove Entrypoint Invoker #31769

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.

Sign up for GitHub

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

Jump to bottom
Merged
8 commits merged into from
Apr 15, 2021
Merged

Remove Entrypoint Invoker #31769

8 commits merged into from
Apr 15, 2021

Conversation

TanayParikh
Copy link
Contributor

@TanayParikh TanayParikh commented Apr 13, 2021 •
edited
Loading

Replaces the custom EntrypointInvoker in favor of runtime's call_assembly_entrypoint which now supports MainAsync.

Fixes: #30600
Fixes: #31148

@TanayParikh TanayParikh requested a review from a team as a code owner April 13, 2021 18:12
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Apr 13, 2021
TanayParikh
TanayParikh commented Apr 13, 2021
View reviewed changes
javiercn
javiercn approved these changes Apr 13, 2021
View reviewed changes
Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

only some minor feedback

pranavkm
pranavkm reviewed Apr 13, 2021
View reviewed changes
@pranavkm
Copy link
Contributor

LoadingApp_DynamicallySetLanguageThrows looks like an actual test failure.

pranavkm
pranavkm approved these changes Apr 14, 2021
View reviewed changes
@ghost
Copy link

ghost commented Apr 15, 2021

Hello @TanayParikh!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

lewing
lewing approved these changes Apr 15, 2021
View reviewed changes
Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
nice work!

@ghost ghost merged commit 28a0a53 into main Apr 15, 2021
@ghost ghost deleted the taparik/removeEntryPointInvoker branch April 15, 2021 01:30
SteveSandersonMS
SteveSandersonMS reviewed Apr 19, 2021
View reviewed changes
src/Components/Web.JS/src/Boot.WebAssembly.ts
@@ -119,7 +119,7 @@ async function boot(options?: Partial<WebAssemblyStartOptions>): Promise<void> {
}

// Start up the application
platform.callEntryPoint(resourceLoader.bootConfig.entryAssembly);
await platform.callEntryPoint(resourceLoader.bootConfig.entryAssembly);
Copy link
Member

@SteveSandersonMS SteveSandersonMS Apr 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TanayParikh @pranavkm Does this change the semantics of app startup? It depends on how BINDING.call_assembly_entry_point behaves, which I can't guess from reading the diff.

I thought that, previously, calling Blazor.start would return a promise that resolves when the .NET code begins running and yields the thread (e.g., on an await). Is that still how it behaves, or does it now resolve only when MainAsync's task completes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be related to the functionality of BINDING.call_assembly_entry_point specifically. Pinging @lewing for more context on this given dotnet/runtime#44045.

Copy link
Member

@lewing lewing Apr 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SteveSandersonMS is correct, successfully awaiting that task should give you the exit code, control will return as soon as it can, given that the runtime is currently singly threaded. If your model expects control to yield here before the runtime has exited, don't await the entry point promise. That said, any expectations that the entry point will yield at a specific point are probably weak.

Copy link
Member

@SteveSandersonMS SteveSandersonMS Apr 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lewing

In this case, this PR is a breaking change we should fix before preview 4 ships. @TanayParikh I'll follow up separately to coordinate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#31971

@SteveSandersonMS SteveSandersonMS mentioned this pull request Apr 20, 2021
Closed
@TanayParikh TanayParikh mentioned this pull request Apr 20, 2021
Merged
10 tasks
mkArtakMSFT pushed a commit that referenced this pull request Apr 21, 2021
@TanayParikh
Remove await for WASM platform.callEntryPoint (#31978)
ba7142e 
## Description

Regression introduced via: #31769

* On Blazor Server, `Blazor.start` returns a promise that resolves when the .NET code starts up and the app becomes interactive
* On WebAssembly:
  * Prior to #31769, `Blazor.start` behaves the same as server (the promise resolves when the app becomes interactive)
  * Since #31769, `Blazor.start` returns a promise that doesn’t resolve until the .NET app exits, which typically means never, and hence prevents its usage for triggering post-.NET startup logic

This PR is meant to revert that specific bit of logic, so that we no longer await the `platform.callEntryPoint` function and thus `Blazor.start` returns a promise that resolves when the .NET code starts up and the app becomes interactive. This now matches the behavior from 6.0-preview3.

## Customer Impact
Without this PR we inadvertently introduce a breaking change where the `Blazor.start` promise doesn't resolve until the .NET app exits. This prevents its usage for triggering post-.NET startup logic.

## Regression?
- [x] Yes
- [ ] No

[If yes, specify the version the behavior has regressed from]: 

Regression from 6.0-preview3 
Regressed by: #31769

## Risk
- [ ] High
- [ ] Medium
- [x] Low

[Justify the selection above]
Returning the `await` logic to what it was prior to #31769 which was introduced in Preview 4.

## Verification
- [x] Manual (required)
- [x] Automated: #31997

## Packaging changes reviewed?
- [ ] Yes
- [ ] No
- [x] N/A

Addresses #31971
@TanayParikh TanayParikh mentioned this pull request Nov 16, 2021
Closed
3dots pushed a commit to 3dots/aspnetcore-Web.JS that referenced this pull request Feb 19, 2024
@TanayParikh
Remove await for WASM platform.callEntryPoint (#31978)
1e5db3e 
## Description

Regression introduced via: #31769

* On Blazor Server, `Blazor.start` returns a promise that resolves when the .NET code starts up and the app becomes interactive
* On WebAssembly:
  * Prior to #31769, `Blazor.start` behaves the same as server (the promise resolves when the app becomes interactive)
  * Since #31769, `Blazor.start` returns a promise that doesn’t resolve until the .NET app exits, which typically means never, and hence prevents its usage for triggering post-.NET startup logic

This PR is meant to revert that specific bit of logic, so that we no longer await the `platform.callEntryPoint` function and thus `Blazor.start` returns a promise that resolves when the .NET code starts up and the app becomes interactive. This now matches the behavior from 6.0-preview3.

## Customer Impact
Without this PR we inadvertently introduce a breaking change where the `Blazor.start` promise doesn't resolve until the .NET app exits. This prevents its usage for triggering post-.NET startup logic.

## Regression?
- [x] Yes
- [ ] No

[If yes, specify the version the behavior has regressed from]: 

Regression from 6.0-preview3 
Regressed by: #31769

## Risk
- [ ] High
- [ ] Medium
- [x] Low

[Justify the selection above]
Returning the `await` logic to what it was prior to dotnet/aspnetcore#31769 which was introduced in Preview 4.

## Verification
- [x] Manual (required)
- [x] Automated: dotnet/aspnetcore#31997

## Packaging changes reviewed?
- [ ] Yes
- [ ] No
- [x] N/A

Addresses dotnet/aspnetcore#31971
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lewing lewing lewing approved these changes

@SteveSandersonMS SteveSandersonMS SteveSandersonMS left review comments

@HaoK HaoK HaoK left review comments

@javiercn javiercn javiercn approved these changes

@eerhardt eerhardt eerhardt left review comments

@pranavkm pranavkm pranavkm approved these changes

Assignees
No one assigned
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove dependency on System.Console in default Blazor WASM app Replace EntryPointInvoker in Blazor WebAssembly with code in the runtime
7 participants
@TanayParikh @pranavkm @lewing @SteveSandersonMS @HaoK @javiercn @eerhardt