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

Upload Progress with ProgressMessageHandler Completes Unexpectedly #21537

Open
tylersouthard opened this issue Oct 28, 2024 · 5 comments · May be fixed by #21660
Open

Upload Progress with ProgressMessageHandler Completes Unexpectedly #21537

tylersouthard opened this issue Oct 28, 2024 · 5 comments · May be fixed by #21660
Assignees
Labels
networking If an issue or pull request is related to networking
Milestone

Comments

@tylersouthard
Copy link

tylersouthard commented Oct 28, 2024

Apple platform

iOS

Framework version

net8.0-*

Affected platform version

.NET 8.0.302, VS Code 1.94.2, .NET MAUI extension v1.4.36

Description

When attempting to upload a file, it appears that the entirety of the file is first copied into memory, then uploaded. This causes two problems.

  1. Large file uploads cause devices with limited memory to run out of memory.
  2. Attempting to track progress for the actual upload doesn't work, since it jumps to 100% quickly (the copy to memory), then reports no progress for the actual upload, which takes much longer than the copy to memory.

The issue here seems to point to similar problems, but the discussion centered around whether the content was coming in as a MemoryStream or FileStream. This is a FileStream, which I would not expect to enter memory all at once, since it should be streamed from disk to be uploaded in chunks.

Steps to Reproduce

  1. Follow the steps here to create a new MAUI application (MAUI was used for simplicity, but the code in question appears to be generic to xamarin-macios).
  2. Write the code below. In summary, it's simply taking a local file (large enough to observe a delay when uploading) and uploading it to an endpoint that will accept it. (Note that the specifics of our endpoint are removed for privacy.)
  3. Observe that the ProgressMessageHandler reports progress in chunks, as expected, but the progress is much too fast to be the actual upload. They all happen within the same second or two.
  4. Then observe that the upload doesn't actually finish until several seconds later. See Relevant Log Output section below.
  5. We would expect to see the ProgressMessageHandler track the actual upload, instead of presumably tracking the copy to memory.
using System.Net.Http.Handlers;
using System.Net.Http.Headers;

namespace MyMauiApp
{
	public partial class MainPage : ContentPage
	{
		public MainPage()
		{
			InitializeComponent();
		}

		private async void OnTestBtnClicked(object sender, EventArgs e)
		{
			await PerformUploadAsync();
		}

		private static async Task PerformUploadAsync()
		{
			ProgressMessageHandler progressMsgHander = new ProgressMessageHandler();
			progressMsgHander.HttpSendProgress += (object? sender, HttpProgressEventArgs e) =>
			{
				Console.WriteLine($"{DateTime.Now}: Sent {e.BytesTransferred}, Percent Complete: {e.ProgressPercentage}%");
			};
			progressMsgHander.InnerHandler = new HttpClientHandler();

			HttpClient httpClient = new HttpClient(progressMsgHander);
			using MultipartFormDataContent formContent = new MultipartFormDataContent("mycoolboundary");
			if (formContent.Headers.ContentType != null)
				formContent.Headers.ContentType.MediaType = "multipart/form-data";

			using FileStream fs = new FileStream("test.zip", FileMode.Open);
			using StreamContent streamContent = new StreamContent(fs);

			string token = "<redacted>";
			httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token);
			httpClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("multipart/form-data"));

			streamContent.Headers.ContentLength = fs.Length;
			formContent.Add(streamContent, "test", fs.Name);

			HttpResponseMessage response = await httpClient.PostAsync("<redacted>", formContent);
			response.EnsureSuccessStatusCode();

			Console.WriteLine($"{DateTime.Now}: Done Uploading");
		}
	}
}

Did you find any workaround?

Adding a breakpoint to the HttpSendProgress event handler allows us to see NSUrlSessionHandler.cs in the call stack. That file appears to check MaxInputInMemory (line 503 currently). If we create a platform-specific shim to set this value to something much smaller than long.MaxValue, the behavior is as expected.

It doesn't seem possible that this workaround would be expected/required for what seems to be a common use case. Is that expectation correct?

Relevant log output

2024-10-28 10:19:10.982692-0400 MyMauiApp[99543:3985957] 10/28/2024 10:19:10 AM: Sent 18, Percent Complete: 0%
2024-10-28 10:19:10.983369-0400 MyMauiApp[99543:3985957] 10/28/2024 10:19:10 AM: Sent 515, Percent Complete: 0%
2024-10-28 10:19:11.002468-0400 MyMauiApp[99543:3986541] 10/28/2024 10:19:11 AM: Sent 131587, Percent Complete: 0%
2024-10-28 10:19:11.004298-0400 MyMauiApp[99543:3986543] 10/28/2024 10:19:11 AM: Sent 262659, Percent Complete: 1%
2024-10-28 10:19:11.005538-0400 MyMauiApp[99543:3986544] 10/28/2024 10:19:11 AM: Sent 393731, Percent Complete: 2%
2024-10-28 10:19:11.005892-0400 MyMauiApp[99543:3986543] 10/28/2024 10:19:11 AM: Sent 524803, Percent Complete: 3%
...
2024-10-28 10:19:11.044001-0400 MyMauiApp[99543:3986541] 10/28/2024 10:19:11 AM: Sent 17302019, Percent Complete: 99%
2024-10-28 10:19:11.044295-0400 MyMauiApp[99543:3986541] 10/28/2024 10:19:11 AM: Sent 17363459, Percent Complete: 99%
2024-10-28 10:19:11.045475-0400 MyMauiApp[99543:3986541] 10/28/2024 10:19:11 AM: Sent 17363481, Percent Complete: 100%
2024-10-28 10:19:27.667022-0400 MyMauiApp[99543:3985957] 10/28/2024 10:19:27 AM: Done Uploading
@rolfbjarne rolfbjarne added the networking If an issue or pull request is related to networking label Oct 31, 2024
@rolfbjarne
Copy link
Member

@mandel-macaque can you have a look at this?

@rolfbjarne rolfbjarne added this to the Future milestone Nov 1, 2024
@mandel-macaque
Copy link
Member

Does look like the default size was wrongly set in mono long time ago. For what I can see, the code was moved with that max size value from the mono repo around 2019. I'll make sure we map this size with the one in the managed handler to make it more reasonable.

@tylersouthard
Copy link
Author

@mandel-macaque Thanks for your response. I'm a little confused as to why it doesn't appear to be a commonly-seen problem? It would seem to apply to anyone performing uploads of files with significant size, and the workaround isn't incredibly obvious or straightforward.

Also, we'd like to remove the workaround (platform-specific) code as soon as possible. Do you have any thoughts on when this change might be available?

@mandel-macaque
Copy link
Member

@tylersouthard we have had not many reports about this issue, I suspect it is because most of our users are uploading small files and they do not use the progress handler.

The ideal solution would be to find a balance between using the NSData and a NSInputStream that makes sense. Unfortunately the Apple documentation has no pointers to the ideal memory limit in this scenario.

One of my ideas is to use the same value used by the dotnet BCL. Both the HttpContent class and the MemoryStream class use a max value of int.MaxValue. You will also notice that HttpClient uses the HttpContent max value to ensure that MaxResponseContentBufferSize is not too big.

Regarding the platform specific code, it is expected for an app developer to configure the native handler according to the platform, we can set some defaults that might make sense, but the app developer knows best.

mandel-macaque added a commit that referenced this issue Nov 19, 2024
…CL classes. Fixes #21537

The MaxInputInMemory property is set by default to be long.MaxValue.
This value is much larger than the one we can find in other BCL classes
such as the HttpContent [max value](https://github.com/microsoft/referencesource/blob/master/System/net/System/Net/Http/HttpContent.cs#L20)
and the MemoryStream [max value](https://github.com/dotnet/runtime/blob/1d1bf92fcf43aa6981804dc53c5174445069c9e4/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs#L37).

It is important to notice that the HttpContent.MaxValue is used to
limit the value of the MaxResponseContentBufferSize property in the
HttpClient. This means that by making this move we are following the
rest of the BCL.

Fixes #21537
@mandel-macaque
Copy link
Member

@tylersouthard I have created an issue in the dotnet SDK, ideally we would want to expose an API for this kind of settings, that way we limit the amount of platform specific code you need, and maybe just change the values of the max size based on the platform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
networking If an issue or pull request is related to networking
Projects
None yet
3 participants