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

Backport #61798 to .NET 6 #73007

Closed
madelson opened this issue Jul 28, 2022 · 9 comments
Closed

Backport #61798 to .NET 6 #73007

madelson opened this issue Jul 28, 2022 · 9 comments

Comments

@madelson
Copy link
Contributor

#61798 is breaking concurrent use of the .NET Azure SDKs for us starting in .NET 6 (see Azure/azure-sdk-for-net#27018, for example). Can this fix be backported to .NET 6?

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 28, 2022
@ghost
Copy link

ghost commented Jul 28, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

#61798 is breaking concurrent use of the .NET Azure SDKs for us starting in .NET 6 (see Azure/azure-sdk-for-net#27018, for example). Can this fix be backported to .NET 6?

Author: madelson
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@MihaZupan
Copy link
Member

Judging by the stack trace you posted in Azure/azure-sdk-for-net#27018 (comment), you appear to be hitting #65379.

Can you please try the workaround mentioned in the issue?

@MihaZupan
Copy link
Member

MihaZupan commented Jul 28, 2022

Duplicate of #65379.

(Of course, feel free to continue discussion around this issue there)

@MihaZupan MihaZupan closed this as not planned Won't fix, can't repro, duplicate, stale Jul 28, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jul 28, 2022
@madelson
Copy link
Contributor Author

@MihaZupan thanks for the info on the other error.

However, I'm also seeing the index-out-of-bounds error (see Azure/azure-sdk-for-net#27018 (comment)) so I'd still appreciate that fix being backported.

@MihaZupan
Copy link
Member

#65379 is the cause for that IndexOutOfRangeException (see EstablishProxyTunnelAsync in the stack trace).

@madelson
Copy link
Contributor Author

madelson commented Jul 28, 2022

@MihaZupan ah ok I misunderstood. So #61799 covers both stacktraces and #68115 is completely unrelated?:

System.IndexOutOfRangeException: Index was outside the bounds of the array. 
at System.Net.Http.Headers.HttpHeaders.ReadStoreValues[T](Span`1 values, Object storeValue, HttpHeaderParser parser, Int32& currentIndex) 
at System.Net.Http.Headers.HttpHeaders.GetStoreValuesAsStringOrStringArray(HeaderDescriptor descriptor, Object sourceValues, String& singleValue, String[]& multiValue) 
at System.Net.Http.Headers.HttpHeaders.GetStoreValuesAsStringArray(HeaderDescriptor descriptor, HeaderStoreItemInfo info) 
at System.Net.Http.Headers.HttpHeaders.TryGetValues(HeaderDescriptor descriptor, IEnumerable`1& values) 
at System.Net.Http.HttpConnectionPool.EstablishProxyTunnelAsync(Boolean async, HttpRequestHeaders headers, CancellationToken cancellationToken) 
at System.Net.Http.HttpConnectionPool.ConnectAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken) 
at System.Net.Http.HttpConnectionPool.CreateHttp11ConnectionAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken) 
at System.Net.Http.HttpConnectionPool.AddHttp11ConnectionAsync(HttpRequestMessage request) 
at System.Threading.Tasks.TaskCompletionSourceWithCancellation`1.WaitWithCancellationAsync(CancellationToken cancellationToken) 
at System.Net.Http.HttpConnectionPool.GetHttp11ConnectionAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken) 
at System.Net.Http.HttpConnectionPool.SendWithVersionDetectionAndRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken) 
at System.Net.Http.DiagnosticsHandler.SendAsyncCore(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken) 
at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken) 
at System.Net.Http.HttpClient.<SendAsync>g_Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken) 

AND

System.InvalidOperationException: Collection was modified; enumeration operation may not execute. 
at System.Collections.Generic.List`1.Enumerator.MoveNextRare() 
at System.Net.Http.Headers.HttpHeaders.ReadStoreValues[T](Span`1 values, Object storeValue, HttpHeaderParser parser, Int32& currentIndex)
at System.Net.Http.Headers.HttpHeaders.GetStoreValuesAsStringOrStringArray(HeaderDescriptor descriptor, Object sourceValues, String& singleValue, String[]& multiValue) 
at System.Net.Http.Headers.HttpHeaders.GetStoreValuesAsStringArray(HeaderDescriptor descriptor, HeaderStoreItemInfo info) 
at System.Net.Http.HttpConnectionPool.EstablishProxyTunnelAsync(Boolean async, HttpRequestHeaders headers, CancellationToken cancellationToken) 
at System.Net.Http.HttpConnectionPool.ConnectAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken) at System.Net.Http.HttpConnectionPool.CreateHttp11ConnectionAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken) 
at System.Net.Http.HttpConnectionPool.AddHttp11ConnectionAsync(HttpRequestMessage request) 
at System.Threading.Tasks.TaskCompletionSourceWithCancellation`1.WaitWithCancellationAsync(CancellationToken cancellationToken) 
at System.Net.Http.HttpConnectionPool.GetHttp11ConnectionAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken) 
at System.Net.Http.HttpConnectionPool.SendWithVersionDetectionAndRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken) 
at System.Net.Http.DiagnosticsHandler.SendAsyncCore(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken) 
at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken) 
at System.Net.Http.HttpClient.<SendAsync>g_Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken) 

Is that correct?

@MihaZupan
Copy link
Member

MihaZupan commented Jul 28, 2022

#61798 tracks the issue that before 7.0, reading from a header collection concurrently was not thread-safe, blowing up in many different ways (like the two exceptions you observed).

We treat concurrent use like that as improper usage. But there are cases where HttpClient itself accesses the collection when it shouldn't, leading to errors in otherwise well-behaved client code.
#65379 tracks one such bug that occurs if the user uses a proxy. In your case, both exceptions are just different manifestations of the same race condition.

TL;DR; #65379 covers both of the exceptions you observed.
#68115 is not completely unrelated - it mitigates the issue, but doesn't eliminate it completely.

@madelson
Copy link
Contributor Author

@MihaZupan thanks. It is still my understanding that these problems are unique to (or at least much more prevalent in) .NET 6; we're hitting them through the Azure SDK and we did not see them prior to the .NET 6 upgrade. Do you think any of the relevant changes will be backported to 6?

@karelz karelz added this to the 7.0.0 milestone Aug 7, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants