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

InProc with ConcurrentRequest not thread-safe. #95

Closed
mellamokb opened this issue Jul 13, 2023 · 1 comment
Closed

InProc with ConcurrentRequest not thread-safe. #95

mellamokb opened this issue Jul 13, 2023 · 1 comment

Comments

@mellamokb
Copy link

This is a long-standing issue and the team is probably well aware. When using InProcess mode with the AllowConcurrentRequestsPerSession on there is a race condition on adding session items. At high volume within a single session it is possible to permanently corrupt the session object causing almost all future session lookups to fail with NRE for that user.

Due to a variety of constraints, we run a .Net Framework app using this library in Azure App Service on InProc mode (using sticky sessions/ARR Affinity). It is a remarkable improvement in performance for busy pages (with lots of AJAX calls) to have a concurrent session. Using this library took some of our busiest pages from 10 sec to <0.5 sec TTI. I've tried Sql session and the performance is abysmal, so would prefer to keep using InProc for as long as possible. However we were getting frequent complaints of corrupt session errors that break the site for some users under decent load (100+ users, ~10 req/sec).

In researching this issue I think the solution is relatively easy. The Item[] indexers in SessionStateItemCollection need the same lock applied as Remove/RemoveAt/Clear calls. Unfortunately this is in System.Web so cannot change, but I verified by adding a custom ThreadSafeSessionStateItemCollection with identical behavior, with simulated load testing the issue appears to be resolved.

I don't know if this is a desirable support scenario but wanted to offer the solution in case it could be incorporated, or some other dev stumbles into the same issue.

mellamokb added a commit to mellamokb/AspNetSessionState that referenced this issue Aug 23, 2023
Added new ThreadSafeSessionStateItemCollection to replace built-in Microsoft one.  This fixes the race condition in the indexers. Also commented out all serialization since this is intended to only be used with the in-memory session, so serialization is not necessary.
@mellamokb
Copy link
Author

I have added a pull request with the new version of SessionStateItemCollection that corrects the race condition. This class was reverse engineered and customized so unlikely to be brought into the main branch, but I offer it here in case any other development team would like to use it. We have been using it for at least a month in a high-volume production web server with no issues, so it does work and does provide an error-free in-memory session solution allowing concurrent requests for a web server farm with sticky sessions.

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

No branches or pull requests

1 participant