PureWeen
commented
1 year ago @jonathanpeppers thoughts?
Closed UkeHa closed 11 months ago
PureWeen
commented
1 year ago @jonathanpeppers thoughts?
ghost
commented
1 year ago Hi @UkeHa. We have added the "s/needs-repro" label to this issue, which indicates that we require steps and sample code to reproduce the issue before we can take further action. Please try to create a minimal sample project/solution or code samples which reproduce the issue, ideally as a GitHub repo that we can clone. See more details about creating repros here: https://github.com/dotnet/maui/blob/main/.github/repro.md
This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.
jonpryor
commented
1 year ago This is going to need a repro.
Two thoughts:
ObjectDisposedException implies that Something Bad Happened; how was AndroidMessageHandler disposed in such a way that it's being used after disposal?For (2), note the bottom half of the provided stack trace:
at Amazon.Runtime.HttpWebRequestMessage.GetResponseAsync(CancellationToken cancellationToken) in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\HttpHandler\_netstandard\HttpRequestMessageFactory.cs:line 515
at Amazon.Runtime.Internal.HttpHandler`1.<InvokeAsync>d__9`1[[System.Net.Http.HttpContent, System.Net.Http, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a],[Amazon.SQS.Model.SendMessageResponse, AWSSDK.SQS, Version=3.3.0.0, Culture=neutral, PublicKeyToken=885c28607f98e604]].MoveNext() in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\HttpHandler\HttpHandler.cs:line 185
at Amazon.Runtime.Internal.Unmarshaller.<InvokeAsync>d__3`1[[Amazon.SQS.Model.SendMessageResponse, AWSSDK.SQS, Version=3.3.0.0, Culture=neutral, PublicKeyToken=885c28607f98e604]].MoveNext() in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Core\Amazon.Runtime\Pipeline\Handlers\Unmarshaller.cs:line 85
at Amazon.SQS.Internal.ValidationResponseHandler.<InvokeAsync>d__1`1[[Amazon.SQS.Model.SendMessageResponse, AWSSDK.SQS, Version=3.3.0.0, Culture=neutral, PublicKeyToken=885c28607f98e604]].MoveNext() in D:\JenkinsWorkspaces\trebuchet-stage-release\AWSDotNetPublic\sdk\src\Services\SQS\Custom\Internal\ValidationResponseHandler.cs:line 43These are all in some Amazon.* namespace. Therefore, some additional NuGet package is being used. What is that package? Does it use AndroidMessageHandler? (It certainly appears to!). How does it use AndroidMessageHandler?
The repro steps don't even mention an Amazon-related NuGet package, and that's a crucial missing piece to figuring out what's going on.
jonpryor
commented
1 year ago Some cursory searching suggests that the aws/aws-sdk-net package is being used, and it's throwing around here:
var responseMessage = await _httpClient.SendAsync(_request, HttpCompletionOption.ResponseHeadersRead, cancellationToken)
.ConfigureAwait(continueOnCapturedContext: false);The ObjectDisposedException implies that _httpClient has been disposed. Is that possible? Likely:
If ClientConfig.DisposeHttpClients(_clientConfig) is true, then disposing of the HttpClientResponseData.ResponseBody instance will call HttpResponseMessageBody.Dispose(), which will dispose of the underlying HttpClient instance, and if/when that happens, I don't see anything obvious within HttpRequestMessageFactory that would "reset or clear" the _httpClient instance.
At a glance, and not knowing anything about AWS, that sounds not good.
So I check the aws/aws-sdk-net issues for anything that "rhymes with" GetResponseAsync or HttpClientResponseData, and find https://github.com/aws/aws-sdk-net/issues/1789 , which has this interesting comment:
[the] SDK pipeline would dispose of the HttpClient instances when it is finished with processing the request, whether it is successful or not.
Which helps reiterate that I have no idea what's going on, but "SDK pipeline dispose of the HttpClient instances when finished" could certainly explain the observed ObjectDisposedException…
jonpryor
commented
1 year ago This increasingly looks like "not a MAUI bug".
UkeHa
commented
1 year ago @jonpryor, it's certainly possible. I've only ever run into this problem with dotnet8 preview. I'll try to create a repro solution for you to follow along
UkeHa
commented
1 year ago Trying to reproduce it with httpclient directly works as expected. I'm uncertain as to why it only fails with .net8 preview though, as it's rock stable with .net7.
arahmancsd
commented
12 months ago @UkeHa I am facing the same issue at the latest .net8 rc. Switching back to .net 7 resolves the issue. Calling any endpoint in a parallel loop results in this error.
ghost
commented
12 months ago Hello lovely human, thank you for your comment on this issue. Because this issue has been closed for a period of time, please strongly consider opening a new issue linking to this issue instead to ensure better visibility of your comment. Thank you!
UkeHa
commented
12 months ago @arahmancsd, you got a sample to reproduce this?
arahmancsd
commented
12 months ago @UkeHa Here is the repo. AndroidMessageHandler
Also,
On a side note, If you reference .net7, you won't get any error.
UkeHa
commented
12 months ago Thanks Abdul, @jonpryor could you try to reproduce it on your side?
Redth
commented
11 months ago Possibly related to dotnet/runtime#95658
Redth
commented
11 months ago I'm seeing this too, however I'm able to work around by instantiating HttpClient each time in the method that's using it instead of reusing the same instance. There seems to be some issue down the stack with something being disposed too early.
I tried instantiating it with the ctor overload specifying to not dispose, which did not help either. This also worked fine for me in .NET 7
steveisok
commented
11 months ago @wfurt most of the action is happening in AndroidMessageHandler, but have you seen this type of problem before?
jonpryor
commented
11 months ago @arahmancsd: thank you for the repro.
For "fun", I applied this patch to xamarin/xamarin-android, and built your repro app against the updated Mono.Android.dll:
diff --git a/external/xamarin-android-tools b/external/xamarin-android-tools
--- a/external/xamarin-android-tools
+++ b/external/xamarin-android-tools
@@ -1 +1 @@
-Subproject commit 2b8ac126a4cd31abe76ccae53631672636db0c76
+Subproject commit 2b8ac126a4cd31abe76ccae53631672636db0c76-dirty
diff --git a/src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs b/src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs
index 6804db9ce..605d5268e 100644
--- a/src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs
+++ b/src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs
@@ -337,6 +337,7 @@ namespace Xamarin.Android.Net
protected override void Dispose (bool disposing)
{
+ global::System.Console.WriteLine ($"# jonp: AndroidMessageHandler.Dispose ({disposing}): {new System.Diagnostics.StackTrace (true)}");
disposed = true;
base.Dispose (disposing);After the app fails, adb logcat contains:
# jonp: AndroidMessageHandler.Dispose (True): at Xamarin.Android.Net.AndroidMessageHandler.Dispose(Boolean disposing)
at System.Net.Http.HttpMessageHandler.Dispose()
at System.Net.Http.HttpClientHandler.get_Handler()
at System.Net.Http.HttpClientHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
at System.Net.Http.HttpMessageInvoker.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
at System.Net.Http.HttpClient.<>n__0(HttpRequestMessage request, CancellationToken cancellationToken)
at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[<<SendAsync>g__Core|83_0>d](<<SendAsync>g__Core|83_0>d& stateMachine)
at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
at System.Net.Http.HttpClient.SendAsync(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationToken cancellationToken)
at System.Net.Http.HttpClient.GetAsync(Uri requestUri, HttpCompletionOption completionOption, CancellationToken cancellationToken)Which is fascinating; it suggests we have this callstack:
What's most interesting to me is:
in which we:
handler aliases _nativeHandler on line 44.DiagnosticsHandler on line 48, which contains handler.handler on line 56, which could very well still be _nativeHandler._nativeHandler is valid after line 56.steveisok
commented
11 months ago
- Nothing ensures that
_nativeHandleris valid after line 56.
I think I've confirmed that is the issue. On a local branch, I removed the Dispose and replaced _nativeHandler with Handler at https://github.com/dotnet/runtime/blob/a9cc3c80fe43d19a38cacda4c1aecc51fb6eabb1/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs#L90. That made things work properly.
ghost
commented
11 months ago Tagging subscribers to this area: @dotnet/ncl See info in area-owners.md if you want to be subscribed.
| Author: | UkeHa |
|---|---|
| Assignees: | - |
| Labels: | `area-System.Net.Http`, `untriaged`, `needs-area-label` |
| Milestone: | - |
ghost
commented
11 months ago Tagging subscribers to 'arch-android': @steveisok, @akoeplinger See info in area-owners.md if you want to be subscribed.
| Author: | UkeHa |
|---|---|
| Assignees: | - |
| Labels: | `area-System.Net.Http`, `blocking-release`, `os-android`, `needs-area-label` |
| Milestone: | 8.0.0 |
ghost
commented
11 months ago Tagging subscribers to 'os-ios': @steveisok, @akoeplinger, @kotlarmilos See info in area-owners.md if you want to be subscribed.
| Author: | UkeHa |
|---|---|
| Assignees: | - |
| Labels: | `area-System.Net.Http`, `blocking-release`, `os-android`, `os-ios`, `needs-area-label` |
| Milestone: | 8.0.0 |
steveisok
commented
11 months ago @MihaZupan replacing references to _nativeHandler with Handler works except for https://github.com/dotnet/runtime/blob/a9cc3c80fe43d19a38cacda4c1aecc51fb6eabb1/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.InvokeNativeHandler.cs#L179 because it's calling methods directly. We also can't dispose of it in the way that was being attempted.
MihaZupan
commented
11 months ago I agree that the dispose at https://github.com/dotnet/runtime/blob/94f212275b2f51ca67025d677d7d5c5bc75f670f/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs#L56 should likely just be removed.
What do you mean by "replacing references to _nativeHandler with Handler"?
steveisok
commented
11 months ago What do you mean by "replacing references to
_nativeHandlerwithHandler"?
That seems to be the only spot since the intent is to make MetricsHandler be the top level one.
MihaZupan
commented
11 months ago Ah, I see. We could replace that with just Handler.Dispose, but it should be functionally the same given that the Metrics/Diagnostics handlers don't have any extra dispose logic.
jonpryor
commented
11 months ago @Redth: I don't think this is related to dotnet/runtime#95658, as the callstacks are significantly different. This issue (#93252) is due to an ObjectDisposedException from AndroidMessageHandler.AssertSelf(), while dotnet/runtime#95658 is an ObjectDisposedException from System.IO.MemoryStream.EnsureNotClosed().
karelz
commented
11 months ago Reopening to track backport into 8.0 in PR #93291
karelz
commented
11 months ago Fixed in 9.0 (main) in PR #93262 and in 8.0 (GA) in PR #93291
heron1
commented
10 months ago Hi I'm getting this error as well in 8.0.0-rc.2.23479.6. Is that to be expected?
pauldendulk
commented
10 months ago Hi I'm getting this error as well in 8.0.0-rc.2.23479.6. Is that to be expected?
Same here.
steveisok
commented
10 months ago The fix is lined up to be in GA.
Hi I'm getting this error as well in 8.0.0-rc.2.23479.6. Is that to be expected?
Yes, this is expected. The fix applied to GA and will be available soon.
Description
When running a http client in a second project inside a solution the connection to the web is broken sporadically in the latest .net8 preview. Each request will run into the same error. Restarting the debug session in the emulator can either solve the issue for the entire time the app runs or will still be borked. After several restarts i'll work again. In .net7 this worked exactly as intended.
Steps to Reproduce
Link to public reproduction project repository
No response
Version with bug
8.0.0-preview.7.8842
Is this a regression from previous behavior?
Yes, this used to work in .NET MAUI
Last version that worked well
7.0.92
Affected platforms
Android
Affected platform versions
Android 13
Did you find any workaround?
Restarting the app until it isn't broken.
Relevant log output