Skip to content

[Xamarin.Android.Build.Tasks] make sure Disposable () is called for assembly #231

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

Closed
wants to merge 1 commit into from

Conversation

lewurm
Copy link
Contributor

@lewurm lewurm commented Sep 22, 2016

see jbevain/cecil#291

This fixes the Sharing violation issue we've seen in the RemoveRegisterAttribute task. I also attempted to fix #44529.

Depends on dotnet/java-interop#87, then this PR needs an update.

@lewurm
Copy link
Contributor Author

lewurm commented Sep 22, 2016

This supersedes #230. I couldn't repro #44529 specifically though, @radekdoulik could you test it please?

@@ -1 +1 @@
Subproject commit a1d3ecc8ba6e67b96ae1c633194b0e78c2ac5c23
Subproject commit d2b5bc101c6643dac5d377451a7a84d232adf93e
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure we need a java.interop change, you might need to update your submodules'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there're changes in Java.Interop required, see dotnet/java-interop#87.

@lewurm lewurm force-pushed the fix-sharing-violation branch from 7357b06 to 8a36f0e Compare September 22, 2016 09:44
… assembly

see jbevain/cecil#291

this fixes one of the `Sharing violation` issues, probably also #44529
@lewurm lewurm force-pushed the fix-sharing-violation branch from 8a36f0e to d0e77f6 Compare September 22, 2016 21:13
@lewurm
Copy link
Contributor Author

lewurm commented Sep 23, 2016

updated PR, now works with the repro provided by https://bugzilla.xamarin.com/show_bug.cgi?id=44529#c2

However, it's a bit tricky: DirectoryAssemblyResolver caches the results when it loads assemblies, so we need to make sure that we already pass the right ReaderParameters to res.Load (). The ReaderParameters are actually not part of the key in the cache, so they get ignored in res.GetAssembly (). Also, using using for res.GetAssembly () would be wrong (we would dispose it, but there's still a reference to it in the cache).

I think Dispose () of DirectoryAssemblyResolver should dispose all assemblies in its cache or not cache at all (Mono.Cecil.BaseAssemblyResolver doesn't cache either). I'll let someone else decide that.

@jonpryor
Copy link
Contributor

I think Dispose () of DirectoryAssemblyResolver should dispose all assemblies in its cache or not cache at all (Mono.Cecil.BaseAssemblyResolver doesn't cache either).

It should dispose; that is part of dotnet/java-interop#88

@jonpryor
Copy link
Contributor

This PR is superseded by PR #233.

@jonpryor jonpryor closed this Sep 24, 2016
radical pushed a commit that referenced this pull request May 8, 2018
Commit 7d51163 removed use of the `__RegisterNativeMembers()`
method, so there is no longer a need to ignore it.

Instead, we need to ignore `JavaProxyObject.RegisterNativeMembers()`,
which *is* invoked via Reflection.
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 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