[Mono.Android] Fix race condition in AndroidMessageHandler

[simonrozsival]

Fixes #8740

There is a race condition in AndroidMessageHandler (see #8740 (comment) for more information). The solution is fairly simple: we should determine the decompression strategy only during the setup, when AutomaticDecompression is set. We don't need to do any more work during the request.

To prevent changing the decompression strategy while there are requests mid-flight, the setter will only accept values until the first request is sent. This is exactly what the SocketsHttpHandler does. I didn't want to touch any code unrelated to the race condition in this PR so if we want to prevent other properties from being mutated while HTTP requests are being sent, we should do it in a follow-up PR.

/cc @grendello

[grendello]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[tipa]

Thanks for the quick fix. One small comment/suggestion: Is the _acceptEncoding variable even needed now or wouldn't it be easier to check for _acceptEncoding == null instead?

xamarin-android/src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs

Line 770 in c2e0759

if (!decompress_here || String.IsNullOrEmpty (httpConnection.ContentEncoding)) {

[simonrozsival]

@tipa Yes, we could replace decompress_here with _acceptEncoding is not null && _acceptEncoding != IDENTITY_ENCODING 👍

[grendello]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jonpryor]

@grendello, @simonrozsival: is there any reasonable way to write a unit test for this?

[simonrozsival]

@jonpryor I haven't come up with a good way to test this. The repro project needs to send several hundreds of requests before it hits the race condition. We could probably use HttpListener and hammer it with requests for several seconds with ~20 threads and make sure all the responses are automatically decompressed.

[grendello]

@jonpryor I don't think there is such a way. What @simonrozsival proposes would sometimes work and would sometimes fail - another test we learn to ignore. No point IMO.

[tipa]

Any chance this will be merged and released before .NET 9? :)