AndroidMessageHandler disposes request MultipartFormDataContent inner JsonContent immediately after sending request

Android application type

Classic Xamarin.Android (MonoAndroid12.0, etc.)

Affected platform version

VS for Mac 8.10.23 (build 7)

Description

AndroidMessageHandler disposes HttpContents buffered streams after request content was written to HttpConnection.OutputStream

    var multipartFormDataContent = new MultipartFormDataContent();
    var jsonContent = JsonContent.Create(stringValue);
    multipartFormDataContent.Add(jsonContent,
				 "Name");
    var request = new HttpRequestMessage(HttpMethod.Post, $"{httpClient.BaseAddress}{SEND_REQUEST_URI}")
    {
        Content = multipartFormDataContent,
    };

    _ = await httpClient.SendAsync(request);
   
    // Throws exception
    var contentValue = await jsonContent.ReadAsStringAsync();

Steps to Reproduce

Select AndroidClientHandler in Project options -> Android build -> HttpClientImplementation;
Create MultipartFormDataContent and JsonContent;
Create HttpRequestMessage and add MultipartFormDataContent to its content;
Call HttpClient.SendAsync with created request;
Call JsonContent ReadAsStringAsync();
ObjectDisposed exception is raised;

Did you find any workaround?

Override AndroidClientHandler WriteRequestContentToOutput method. This allows us to prevent content buffered stream from disposing after writing to httpConnection.OutputStream:

	public class CustomAndroidHandler : AndroidClientHandler
	{
		protected override async Task WriteRequestContentToOutput(HttpRequestMessage request, HttpURLConnection httpConnection, CancellationToken cancellationToken)
		{
			var stream = await request.Content.ReadAsStreamAsync().ConfigureAwait(false);
			await stream.CopyToAsync(httpConnection.OutputStream!, 4096, cancellationToken).ConfigureAwait(false);

			if (stream.CanSeek)
			{
				_ = stream.Seek(0, SeekOrigin.Begin);
			}
		}
	}

AndroidClientHandler getting disposed HttpRequestMessage.Content immediately after sending

Relevant log output

  at System.IO.MemoryStream.EnsureNotClosed () [0x00008] in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/corefx/src/Common/src/CoreLib/System/IO/MemoryStream.cs:121 
  at System.IO.MemoryStream.get_Length () [0x00000] in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/corefx/src/Common/src/CoreLib/System/IO/MemoryStream.cs:333 
  at System.Net.Http.HttpContent.ReadBufferedContentAsString () [0x00000] in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/corefx/src/System.Net.Http/src/System/Net/Http/HttpContent.cs:175 
  at System.Net.Http.HttpContent+<>c.<ReadAsStringAsync>b__36_0 (System.Net.Http.HttpContent s) [0x00000] in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/corefx/src/System.Net.Http/src/System/Net/Http/HttpContent.cs:168 
  at System.Net.Http.HttpContent.WaitAndReturnAsync[TState,TResult] (System.Threading.Tasks.Task waitTask, TState state, System.Func`2[T,TResult] returnFunc) [0x0006b] in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/corefx/src/System.Net.Http/src/System/Net/Http/HttpContent.cs:727

@grendello

@grendello this bug still exists in .NET 8 & the new AndroidMessageHandler and makes my app's logic way more complicated that it should be. I have a retry-logic for failed http requests and on iOS/macOS/Windows I can re-use the HttpContent for multiple requests / HttpRequestMessages - except on Android.

The exception can be very easily reproduced:

var jsonContent = JsonContent.Create("{}");
var request = new HttpRequestMessage(HttpMethod.Post, "https://httpbin.org/post") { Content = jsonContent };
_ = await new HttpClient().SendAsync(request);
var contentValue = await jsonContent.ReadAsStringAsync();

Ar there any plans to change this or is it "working as expected"?

@grendello

@grendello @simonrozsival I believe the problem lies here:
https://github.com/xamarin/xamarin-android/blob/a82ac4e5608b9745916db320f4ff36921d4d6dee/src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs#L539-L562

I believe the stream shouldn't be disposed of with a using block, especially because the comment in it doesn't make much sense when doing so. Why is the stream rewound when it's disposed of right after?
Additionally, would it make sense to call the ReadAsStreamAsync overload with a CancellationToken that was added with .NET? https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpcontent.readasstreamasync?view=net-8.0#system-net-http-httpcontent-readasstreamasync(system-threading-cancellationtoken)

Some background on why this fix would help me simplifying my apps' logic. I currently need to do something like this:

async Task<HttpResponseMessage> SendRequest(Func<HttpContent> contentFunc, int attempt = 1)
{
    using var requestMessage = new HttpRequestMessage(HttpMethod.Post, "https://someurl.com") { Content = contentFunc.Invoke() };
    var response = await httpClient.SendAsync(requestMessage);

    if (!response.IsSuccessStatusCode && attempt < 3)
    {
        await Task.Delay(2000 * attempt * attempt);
        return await SendRequest(contentFunc, attempt + 1);
    }
    return response;
}

await SendRequest(() => new ByteArrayContent(data));

But I would prefer to use something like this (which works without issues on other platforms (iOS, macOS, Windows):

async Task<HttpResponseMessage> SendRequest(HttpContent content, int attempt = 1)
{
    using var requestMessage = new HttpRequestMessage(HttpMethod.Post, "https://someurl.com") { Content = content };
    var response = await httpClient.SendAsync(requestMessage);

    if (!response.IsSuccessStatusCode && attempt < 3)
    {
        await Task.Delay(2000 * attempt * attempt);
        return await SendRequest(content, attempt + 1);
    }
    return response;
}

await SendRequest(new ByteArrayContent(data));

@tipa

@tipa I can confirm that removing that using fixes this use-case. You're right that rewinding the stream just before disposing it seems weird. SocketsHttpHandler doesn't assume it owns the request message and it doesn't dispose the content. This code seems to be quite old (the rewinding was implemented in #608, the using was introduced even earlier in 0c35978).

@grendello it seems that disposing the content stream is a bug, especially since the sockets handler doesn't do it.

Related: dotnet/macios#16341

@simonrozsival

@simonrozsival would you mind opening a PR? I can't even build XA today, getting segfaults when building Mono.Android :(

I found that there are related bug reports to the same issue, that can be closed as well once the fix is merged:
#4476
#2901

As you can see, a long-standing bug ;)

[Mono.Android] Do not dispose request content stream in AndroidMessag…

Navigation Menu

Repository navigation

AndroidMessageHandler disposes request MultipartFormDataContent inner JsonContent immediately after sending request #7086

Android application type

Affected platform version

Description

Steps to Reproduce

Did you find any workaround?

Relevant log output

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

AndroidMessageHandler disposes request MultipartFormDataContent inner JsonContent immediately after sending request #7086

Description

Android application type

Affected platform version

Description

Steps to Reproduce

Did you find any workaround?

Relevant log output

Activity

tipa commented on Feb 22, 2024

tipa commented on Feb 25, 2024

simonrozsival commented on Feb 26, 2024

simonrozsival commented on Feb 26, 2024

grendello commented on Feb 26, 2024

tipa commented on Feb 27, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions