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

[linker] Make sure we mark *Invoker types #3364

Merged
merged 3 commits into from
Jul 19, 2019

Conversation

radekdoulik
Copy link
Member

Fixes #3263

Looks like
5b945ab
introduced a new regression, where in some cases we were not marking
the *Invoker type anymore, as this line
is gone.

This is fixed by marking the *Invoker types in MarkJavaObjects step.

Fixes dotnet#3263

Looks like
dotnet@5b945ab
introduced a new regression, where in some cases we were not marking
the *Invoker type anymore, as [this
line](dotnet@5b945ab#diff-144727b152107ec306fbe284bd5902e3L60)
is gone.

This is fixed by marking the *Invoker types in MarkJavaObjects step.
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

Is there a way we can add a test for this? Maybe we can put the repro in Mono.Android-Tests where it would crash in the same way?

@radekdoulik
Copy link
Member Author

The Mono.Android test fails differently than the original repro of the issue, because it uses jnimarshalmethod-gen. It fails though, so it will be a good test for it anyway.

@radekdoulik radekdoulik merged commit 4d8c28f into dotnet:master Jul 19, 2019
jonpryor pushed a commit that referenced this pull request Jul 19, 2019
[linker] Make sure we mark *Invoker types

Fixes #3263

Looks like
5b945ab
introduced a new regression, where in some cases we were not marking
the *Invoker type anymore, as [this
line](5b945ab#diff-144727b152107ec306fbe284bd5902e3L60)
is gone.

This is fixed by marking the *Invoker types in MarkJavaObjects step.

Added test to Mono.Android runtime test, which should catch the issue
if it happens again.
jonpryor pushed a commit that referenced this pull request Jul 31, 2019
[linker] Make sure we mark *Invoker types

Fixes #3263

Looks like
5b945ab
introduced a new regression, where in some cases we were not marking
the *Invoker type anymore, as [this
line](5b945ab#diff-144727b152107ec306fbe284bd5902e3L60)
is gone.

This is fixed by marking the *Invoker types in MarkJavaObjects step.

Added test to Mono.Android runtime test, which should catch the issue
if it happens again.
@tipa
Copy link

tipa commented Jun 12, 2022

The problem reported in #3263 happens again with VS 17.2.3 using a .NET 6 Android application.
Can the issue be reopened or should I file a new issue?

@jonathanpeppers
Copy link
Member

@tipa we still use this same linker step in .NET 6:

https://github.com/xamarin/xamarin-android/blob/aa460895d24e3406fc07f7643da80104b9706a6c/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.ILLink.targets#L63

But if you have an example that breaks, file an issue, and we can take a look. Thanks!

I recall you might be using AndroidLinkMode=Full, so maybe there is an issue when using that.

@tipa
Copy link

tipa commented Jun 15, 2022

It also crashes when not using AndroidLinkMode, but just the default csproj created by File -> New

I opened a new issue with a demo project here: #7097

@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 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.

Link assemblies causes app crashes if you have an EditText in VS2019 Preview 2
3 participants