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

[Xamarin.Android.Build.Tasks] Rework AsyncTask to use ConfigureAwait #998

Closed
wants to merge 1 commit into from

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Nov 1, 2017

We should be using ConfigureAwait(false) for our Task.Run
calls to ensure that the Continuations do NOT run on the
main thread. This is probably the reason why VS locks up
when we call these methods. The Continuation which calls
Complete is trying to run on the UI thread, which will
be locked waiting for the Task to complete.

We should also provide the Cancelation Token and the
TaskScheduler for the Parallel.ForEach calls.

@dellis1972 dellis1972 changed the title Rework AsyncTask stuff a bit [WIP Rework AsyncTask stuff a bit [WIP] Nov 1, 2017
@dellis1972 dellis1972 added do-not-merge PR should not be merged. full-mono-integration-build For PRs; run a full build (~6-10h for mono bumps), not the faster PR subset (~2h for mono bumps) labels Nov 1, 2017
@dellis1972 dellis1972 force-pushed the asyncstuff branch 2 times, most recently from b5759fd to 96c885c Compare November 9, 2017 15:16
@dellis1972 dellis1972 changed the title Rework AsyncTask stuff a bit [WIP] [Xamarin.Android.Build.Tasls] Rework AsyncTask stuff a bit Nov 9, 2017
@dellis1972 dellis1972 removed the do-not-merge PR should not be merged. label Nov 9, 2017
@dellis1972 dellis1972 changed the title [Xamarin.Android.Build.Tasls] Rework AsyncTask stuff a bit [Xamarin.Android.Build.Tasls] Rework AsyncTask to use ConfigureAwait Nov 9, 2017
@dellis1972 dellis1972 changed the title [Xamarin.Android.Build.Tasls] Rework AsyncTask to use ConfigureAwait [Xamarin.Android.Build.Tasks] Rework AsyncTask to use ConfigureAwait Nov 9, 2017
We should be using `ConfigureAwait(false)` for our `Task.Run`
calls to ensure that the Continuations do NOT run on the
main thread. This is probably the reason why VS locks up
when we call these methods. The Continuation which calls
`Complete` is trying to run on the UI thread, which will
be locked waiting for the Task to complete.

We should also provide the Cancelation Token and the
TaskScheduler for the `Parallel.ForEach` calls.

task.ContinueWith ( (t) => {
Complete ();
});
}).ConfigureAwait (false);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure ConfigureAwait(false) does anything unless you are awaiting it. I think it only controls the code after an await statement.

I think you need to passing the options to an overload of ContinueWith: https://msdn.microsoft.com/en-us/library/dd321561(v=vs.110).aspx

Probably not a way to write a unit test for this to know if it's for sure correct...

@dellis1972 dellis1972 closed this Nov 9, 2017
jonpryor pushed a commit that referenced this pull request Jul 18, 2022
…7123)

Context: e1af958
Context: #7163

Changes: dotnet/java-interop@c942ab6...fadbb82

  * dotnet/java-interop@fadbb82c: [generator] Add support for @explicitInterface metadata (#1006)
  * dotnet/java-interop@3e4a3c4f: [Java.Interop.Tools.JavaCallableWrappers] JavaCallableMethodClassifier (#998)

dotnet/java-interop@3e4a3c4f reworked how
`Java.Interop.Tools.JavaCallableWrappers.JavaCallableWrapperGenerator`
can be used to find JNI marshal methods.

Update `Xamarin.Android.Build.Tasks.dll` to provide a
`JavaCallableMethodClassifier` to `JavaCallableWrapperGenerator`,
collecting Cecil `MethodDefinition` instances for JNI marshal methods
which can be created at build time via LLVM Marshal Methods.

Methods which cannot be created at build time include methods with
`[Export]`.

Once candidate LLVM marshal methods are found, the marshal method is
updated to have the [`UnmanagedCallersOnlyAttribute` attribute][0],
and related System.Reflection.Emit-based infrastructure such as the
`Get…Handler()` methods and `cb_…` fields are removed.

As with e1af958, this feature is *not* enabled by default, and
remains a xamarin-android Build time configuration option.

[0]:https://docs.microsoft.com/dotnet/api/system.runtime.interopservices.unmanagedcallersonlyattribute?view=net-6.0
@github-actions github-actions bot locked and limited conversation to collaborators Feb 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
full-mono-integration-build For PRs; run a full build (~6-10h for mono bumps), not the faster PR subset (~2h for mono bumps)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants