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] fix potential AsyncTask.Log calls #4205

Merged
merged 1 commit into from
Jan 31, 2020

Conversation

jonathanpeppers
Copy link
Member

Backport of: #4187

We have had a continuing concern of accidentally calling methods on
AsyncTask.Log from a background thread. If it happens, this can
cause a hang inside Visual Studio on Windows.

I finally had the idea of creating a build error if we inadvertently
do this:

[Obsolete("You should not use the 'Log' property directly for AsyncTask. Use the 'Log*' methods instead.", error: true)]
public new TaskLoggingHelper Log { get; }

This uncovered 26 build errors! Some were actually on the main
thread and completely OK, but several were not OK.

In most of the places, I could simply change Log.LogDebugMessage()
to LogDebugMessage().

However a few of the more troublesome places:

  • ManifestDocument required a TaskLoggingHelper for its ctor.
    I moved this to only the methods that need it.

  • NdkUtils and NdkUtilsOld I used Action<string> and
    Action<string, string> callbacks instead.

  • The <Aot/> task had an empty ValidateAotConfiguration()
    method I removed.

Going forward, it should be a lot harder for us to introduce a hang
by using AsyncTask.Log.

…et#4187)

We have had a continuing concern of accidentally calling methods on
`AsyncTask.Log` from a background thread.  If it happens, this can
cause a hang inside Visual Studio on Windows.

I finally had the idea of creating a build error if we inadvertently
do this:

	[Obsolete("You should not use the 'Log' property directly for AsyncTask. Use the 'Log*' methods instead.", error: true)]
	public new TaskLoggingHelper Log { get; }

This uncovered 26 build errors!  Some were actually on the main
thread and completely OK, but several were not OK.

In most of the places, I could simply change `Log.LogDebugMessage()`
to `LogDebugMessage()`.

However a few of the more troublesome places:

  * `ManifestDocument` required a `TaskLoggingHelper` for its ctor.
    I moved this to only the methods that need it.

  * `NdkUtils` and `NdkUtilsOld` I used `Action<string>` and
    `Action<string, string>` callbacks instead.

  * The `<Aot/>` task had an empty `ValidateAotConfiguration()`
    method I removed.

Going forward, it should be a lot harder for us to introduce a hang
by using `AsyncTask.Log`.
@jonpryor jonpryor merged commit 6274165 into dotnet:d16-5 Jan 31, 2020
@jonathanpeppers jonathanpeppers deleted the asynctask.log-d16-5 branch January 31, 2020 01:12
@github-actions github-actions bot locked and limited conversation to collaborators Jan 28, 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.

None yet

3 participants