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

Disable marshal methods generation by default #9336

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

grendello
Copy link
Contributor

We've learned that enabling marshal methods appears to break not only
Blazor apps, but also select other apps. Since we currently have no
insight into what is exactly going on (only suspicions), it is prudent
that we disable marshal method generation by default for NET9 release.

We've learned that enabling marshal methods appears to break not only
Blazor apps, but also select other apps.  Since we currently have no
insight into what is exactly going on (only suspicions), it is prudent
that we disable marshal method generation by default for NET9 release.
@jonpryor
Copy link
Member

jonpryor commented Sep 26, 2024

Draft commit message:

Context: https://github.com/dotnet/android/issues/8253

We've had a number of reports about *hangs* when LLVM Marshal Methods
are enabled (see also dotnet/android#8253), but we had been willing
to accept having LLVM Marshal Methods enabled by default as it helps
improve app startup time.

However, [a customer reported a native crash][0] when LLVM Marshal
Methods are enabled:

> ![Thread Stack Trace][1]

	syscall at line 28 within libc
	__futex_wait_ex(void volatile*, bool, int, bool, timespec const*) at line 144 within libc
	sem_wait at line 108 within libc
	within libmonosgen-2.0.so (Build Id: …)
	within <anonymous:…>

This native crash suggests "something going wrong" within MonoVM.
We find this "scary" enough that we feel it is once again prudent
to disable LLVM Marshal Methods by default in .NET 9.

[0]: https://discord.com/channels/732297728826277939/732297837953679412/1288758900627345463
[1]: https://cdn.discordapp.com/attachments/732297837953679412/1288758900522483712/05F6ED48-2BBA-4FC0-A54F-81BB57606500.png?ex=66f659c1&is=66f50841&hm=8f6f78f283f8c03e9fb37a452ad5d0babaaad3dd4bcc1b4887f4dcf60f651c12&

@jonpryor jonpryor merged commit afb5f7e into main Sep 30, 2024
56 of 58 checks passed
@jonpryor jonpryor deleted the dev/grendel/disable-marshal-methods branch September 30, 2024 11:30
jonathanpeppers pushed a commit to jonathanpeppers/xamarin-android that referenced this pull request Sep 30, 2024
Context: dotnet#8253
Context: dotnet#9343

We've had a number of reports about *hangs* when LLVM Marshal Methods
are enabled (see also dotnet#8253), but we had been willing
to accept having LLVM Marshal Methods enabled by default as it helps
improve app startup time.

However, [a customer reported a native crash][0] when LLVM Marshal
Methods are enabled:

> ![Thread Stack Trace][1]

	syscall at line 28 within libc
	__futex_wait_ex(void volatile*, bool, int, bool, timespec const*) at line 144 within libc
	sem_wait at line 108 within libc
	within libmonosgen-2.0.so (Build Id: …)
	within <anonymous:…>

This native crash suggests "something going wrong" within MonoVM.
We find this "scary" enough that we feel it is once again prudent
to disable LLVM Marshal Methods by default in .NET 9, by setting the
default value of `$(AndroidEnableMarshalMethods)`=False.

Even though dotnet#9343 has a potential fix for the hang, we feel that
we're too close to .NET 9 GA to validate in the wild.  This feature
will need to wait for .NET 10.  🙁

To explicitly enable LLVM Marshal Methods, set
`$(AndroidEnableMarshalMethods)`=True in your App `.csproj`.

[0]: https://discord.com/channels/732297728826277939/732297837953679412/1288758900627345463
[1]: https://cdn.discordapp.com/attachments/732297837953679412/1288758900522483712/05F6ED48-2BBA-4FC0-A54F-81BB57606500.png?ex=66f659c1&is=66f50841&hm=8f6f78f283f8c03e9fb37a452ad5d0babaaad3dd4bcc1b4887f4dcf60f651c12&
jonathanpeppers added a commit that referenced this pull request Sep 30, 2024
…9346)

Context: #8253
Context: #9343

We've had a number of reports about *hangs* when LLVM Marshal Methods
are enabled (see also #8253), but we had been willing
to accept having LLVM Marshal Methods enabled by default as it helps
improve app startup time.

However, [a customer reported a native crash][0] when LLVM Marshal
Methods are enabled:

> ![Thread Stack Trace][1]

	syscall at line 28 within libc
	__futex_wait_ex(void volatile*, bool, int, bool, timespec const*) at line 144 within libc
	sem_wait at line 108 within libc
	within libmonosgen-2.0.so (Build Id: …)
	within <anonymous:…>

This native crash suggests "something going wrong" within MonoVM.
We find this "scary" enough that we feel it is once again prudent
to disable LLVM Marshal Methods by default in .NET 9, by setting the
default value of `$(AndroidEnableMarshalMethods)`=False.

Even though #9343 has a potential fix for the hang, we feel that
we're too close to .NET 9 GA to validate in the wild.  This feature
will need to wait for .NET 10.  🙁

To explicitly enable LLVM Marshal Methods, set
`$(AndroidEnableMarshalMethods)`=True in your App `.csproj`.

[0]: https://discord.com/channels/732297728826277939/732297837953679412/1288758900627345463
[1]: https://cdn.discordapp.com/attachments/732297837953679412/1288758900522483712/05F6ED48-2BBA-4FC0-A54F-81BB57606500.png?ex=66f659c1&is=66f50841&hm=8f6f78f283f8c03e9fb37a452ad5d0babaaad3dd4bcc1b4887f4dcf60f651c12&

Co-authored-by: Marek Habersack <grendel@twistedcode.net>
jonathanpeppers added a commit to dotnet/docs-maui that referenced this pull request Oct 14, 2024
It appears this section might have been AI generated, as it appeared
to contain "hallucinated" MSBuild property names. It also didn't make
sense that the example set things to `false`. I updated the section as
appropriate.

We also have made the feature non-default in:

* dotnet/android#9336

I also linked to GitHub commits, as we have more detailed text inside,
generally, than pull requests.
davidbritch pushed a commit to dotnet/docs-maui that referenced this pull request Oct 15, 2024
It appears this section might have been AI generated, as it appeared
to contain "hallucinated" MSBuild property names. It also didn't make
sense that the example set things to `false`. I updated the section as
appropriate.

We also have made the feature non-default in:

* dotnet/android#9336

I also linked to GitHub commits, as we have more detailed text inside,
generally, than pull requests.
@github-actions github-actions bot locked and limited conversation to collaborators Oct 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants