dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.95k stars 4.65k forks source link

Exception System.InvalidOperationException in HttpListener.EndGetContext #107025

Open pjannesen opened 2 weeks ago

pjannesen commented 2 weeks ago

Description

After updating the application from Framework to .Net 8 we occasionally get the following exceptions:

System.IndexOutOfRangeException:
   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.Net.HttpListener.RegisterForDisconnectNotification(HttpListenerSession session, UInt64 connectionId, DisconnectAsyncResult& disconnectResult)
   at System.Net.HttpListener.HandleAuthentication(HttpListenerSession session, RequestContextBase memoryBlob, Boolean& stoleBlob)
   at System.Net.ListenerAsyncResult.IOCompleted(ListenerAsyncResult asyncResult, UInt32 errorCode, UInt32 numBytes)
   at System.Net.HttpListener.EndGetContext(IAsyncResult asyncResult)

System.InvalidOperationException:
   at System.Collections.Generic.Dictionary`2.FindValue(TKey key)
   at System.Net.HttpListener.HandleAuthentication(HttpListenerSession session, RequestContextBase memoryBlob, Boolean& stoleBlob)
   at System.Net.ListenerAsyncResult.IOCompleted(ListenerAsyncResult asyncResult, UInt32 errorCode, UInt32 numBytes)
   at System.Net.HttpListener.EndGetContext(IAsyncResult asyncResult)

After further investigation in the System.Net.HttpListener sourecode I found that the Dictionary DisconnectResults is not always used in a thread safe manner. It is missing lock around the access of DisconnectResults

Reproduction Steps

It is hard to reproduce. It happens randomly

Expected behavior

No exception

Actual behavior

Exceptions and possible internal corruption of HttpListener

Regression?

No response

Known Workarounds

I have fixed version of HttpListener (see patch). If this fix is stable for 1 week I will submit a pull request.

HttpListener-fix-Operations-that-change-non-concurre.patch

Configuration

No response

Other information

No response

dotnet-policy-service[bot] commented 2 weeks ago

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

huoyaoyuan commented 2 weeks ago

Neither HttpListener nor the Begin/End pattern are under active development. It has many other bugs unfixed.

Using a ConcurrentDictionary instead of ordinal dictionary under lock would be a better fix, but I'm not sure whether it's open for pull requests.

pjannesen commented 2 weeks ago

DisconnectResults is used in 5 places. 2 of them had already the 'lock ((DisconnectResults as ICollection).SyncRoot)' pattern so therefor I opted to add locking to the other 3 places.

Without fixing this bug HttpListener is unstable and cannot be used in a production environment.

huoyaoyuan commented 2 weeks ago

Without fixing this bug HttpListener is unstable and cannot be used in a production environment.

Even with the fix, it's still hard to be considered stable. I would rather use ASP.NET components as HTTP listener.

pjannesen commented 2 weeks ago

We have 2 application that are using HttpListener one is 10+ old and the other 5+. Changing them to ASP.NET is a lot of work.

HttpListener is originally a C# wrapper around http.sys (windows kernel drive, iis core). We are using it >10 years with Framework without a glitch. Because it is uses http.sys it has a lot of advantages like Integrated Windows Authentication, Port sharing and easy to use in a windows service. So, wat you are suggesting is that with the merger of Framework and Core something has gone wrong.