search

apereo / dotnet-cas-client

Apereo .NET CAS Client
Apache License 2.0
234 stars 172 forks source link

CacheServiceTicketManager cleanup issues #41

Closed sunnymoon closed 7 years ago

sunnymoon commented 8 years ago

When the memory pressure is hig on the HttpContext.Current.Cache, it gets cleared by IIS. Then the user is actually "logged out" in the middle of an otherwise authenticated session.

Implementing the storage of local tickets on the HttpContext Cache seems flawed by design, since collection of items might not be totally controlled.

phantomtypist commented 7 years ago

@sunnymoon this could be solved by the RFC for a distributed cache provider in issue #61, correct?

I think any time you use an in-memory cache, such as System.Web.Caching or System.Runtime.Caching, you are going to run into the possibility of IIS recycling the app pool. The in-memory caching just isn't durable.... there is no serialization to disk.

I guess another possibility is making a cache provider for the proxy and service ticket managers that is in-memory, but also serializes to local disk for durability. Any chance you'd be up for writing that?

lanorkin commented 7 years ago

@phantomtypist I don't think backing up in-memory to file is good idea. It will require significant efforts to implement it in a good way (like support multithreading, possible issues with file read/write, issues with network drives etc), and it still won't work in distributed environments, like scale up in Azure etc.

There are many ways which will lead to in-memory cache cleanup, so I think it would be just good to emphasize that default (and the only) implementation of cache is in-memory. Actually it is even commented in source code here https://github.com/apereo/dotnet-cas-client/blob/master/DotNetCasClient/State/CacheProxyTicketManager.cs#L27 in CacheServiceTicketManager summary:

An IServiceTicketManager implementation that relies on the ASP.NET Caching model for ticket storage. Generally this implies that the ticket storage is maintained locally on the web server (either in memory or on disk). A limitation of this model is that it will not support clustered, load balanced, or round-robin style configurations.

I think highlighting it in the repository readme to warn users is really good idea.

For real-life production cases something like in-memory cache + database can be useful, but that is really part of #61 IMO.

phantomtypist commented 7 years ago

I'll be finalizing the 1.1.0 release today and at the same time cleaning up a bit of the documentation on the readme. I'll add a note for that. Some of the content from the readme will be migrated to the GitHub wiki in this repo too.

I'm going to close this issue as the in-memory caching is working by design (in .NET, that is.) If anybody wants a scenario where the tickets survive an IIS or AppPool recycle, then they'll have to use the option(s) eventually created by issue #61.

sunnymoon commented 7 years ago

We've implemented an alternative with a static global variable. The reallity is if the application pool gets recicled we'll loose all logged in session, true.

But actually the webcache is actually cleared by .net at any point in time, nevermind the pool recycling. It depends only on memory pressure and not memory exhaustion...

Please read https://msdn.microsoft.com/en-us/library/system.web.caching.cache.effectivepercentagephysicalmemorylimit.aspx

For additional details...