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

Exception System.InvalidOperationException in HttpListener.EndGetContext #107025

Open
pjannesen opened this issue Aug 27, 2024 · 15 comments
Open
Assignees
Milestone

Comments

@pjannesen
Copy link
Contributor

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 dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Aug 27, 2024
Copy link
Contributor

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

@huoyaoyuan
Copy link
Member

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
Copy link
Contributor Author

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.

@wfurt wfurt removed the untriaged New issue has not been triaged by the area owner label Aug 27, 2024
@wfurt wfurt added this to the Future milestone Aug 27, 2024
@huoyaoyuan
Copy link
Member

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
Copy link
Contributor Author

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.

@pjannesen
Copy link
Contributor Author

I don't see the fix v8.0.11 or V9.0.0. What do I need to do t get this fix also in v8 and v9?

@huoyaoyuan
Copy link
Member

The fix was merged after 9.0 snapping, and was not backported yet.

You need to request a backport here. Usually you would be asked for reasons like usage, difficulty of workaround etc. @dotnet/ncl

@pjannesen
Copy link
Contributor Author

The only workaround is build dotnet rumtime from source with the patch :-(

Backport is extremely easy just a sherry pick of the commit.

@pjannesen
Copy link
Contributor Author

I don't see any response?
What needs to happen to get this fix in 8 and 9?

@huoyaoyuan
Copy link
Member

@dotnet/ncl for backport request.

@pjannesen
Copy link
Contributor Author

@dotnet/ncl

How? Because of the '/' I can not tag '@dotnet/ncl'

@huoyaoyuan
Copy link
Member

Internal teams are only visible to organization members. It's end-of-year holiday season now so the response may be not timely.

@karelz
Copy link
Member

karelz commented Dec 13, 2024

Thanks @huoyaoyuan for help here!
I definitely STRONGLY agree that usage of HttpListener should be discouraged. It is not the best production server, Kestrel/ASP.NET is. The problem is that HttpListener has lots of shortcomings when it comes to deep security options (e.g. rate limiting). Whenever someone uses it in serious deployment I am EXTRA NERVOUS (it should made you nervous as well).
The same holds on .NET Framework BTW.
And yes, it is easy to use - but therefore also it does not give all the flexibility one might need from serious production server.

Given it is a regression in migration without reasonable workaround, I don't mind backporting it.
Why would you need 8.0? Wouldn't 9.0 backport be sufficient?
Given you are the only impacted customer in years, it would be likely better for us.

@rokonec
Copy link
Member

rokonec commented Dec 13, 2024

Backporting to 9 has been initiated in #110695

@pjannesen
Copy link
Contributor Author

Thanks @huoyaoyuan for help here! I definitely STRONGLY agree that usage of HttpListener should be discouraged. It is not the best production server, Kestrel/ASP.NET is. The problem is that HttpListener has lots of shortcomings when it comes to deep security options (e.g. rate limiting). Whenever someone uses it in serious deployment I am EXTRA NERVOUS (it should made you nervous as well). The same holds on .NET Framework BTW. And yes, it is easy to use - but therefore also it does not give all the flexibility one might need from serious production server.

Given it is a regression in migration without reasonable workaround, I don't mind backporting it. Why would you need 8.0? Wouldn't 9.0 backport be sufficient? Given you are the only impacted customer in years, it would be likely better for us.

We have 2 NT services that we recently update from Framework to .NET 8. We use HttpListener for admin access to the services. Mostly for status overviews.
Kestrel/ASP.NET is not an good option because:

  • No port sharing
  • No integrated security.
  • Don't need to include ASP.NET.

ASP.NET is more a framework than a simple (API) webserver. I have read that Kestrel can be used without ASP.NET in .NET 8. But it is harder in .NET 9 because of a reference from Kestrel to ASP.NET. I don’t know if Microsoft.AspNetCore.Server.HttpSys can be used without ASP.NET.
HttpListener is perfect for this a type of a webservice in a NT service.

If you do not handle and log every unexpected exception, chances are you will miss these errors. As a result, application crashes or instabilities. I fear that the majority ignores these types of problems and only a small minority is willing and able to search for the cause and report the problem.

.NET 9 is release 1 month a go. That is to new for use. When 9.0.1/2 is released we will probable update to .NET 9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants