-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
Sessions should be allowed to store objects #18159
Comments
We explicitly chose not to support arbitrary objects in We absolutely don't want to make assumptions about the deployment environment in If you want to store live objects, you can use something like |
There were lots of issues with serialisation, as it lead to the same issues that you have described above and more (which are in my first post) I have already implemented the IMemoryCache, as a quick interim while transition is taking place. Microsofts official page shouldn't recommend serialisation for storing objects - it is incorrect solution and isn't same as storing objects in session, unless user can guarantee that the serialised object is a readonly object. |
This is my full list of concerns B. If you were looking for a read-write object in the session, everytime you change the object that is read from the session after deserialisation - it needs to be written back to the session again by calling serialisation - and this alone can lead to multiple complexities as you will need to either keep track of the changes - or keep writing back to session after each change in any property. In one request to the server, you will have scenarios where the object is written back multiple times till the response is sent back. C. For a read-write object in the session, even on a single server it will fail, as the actions of the user can trigger multiple rapid requests to the server and not more than often system will find itself in a situation where the object is being serialised or deserialised by one thread and being edited and then written back by another, the result is you will end up with overwriting the object state by threads - and even locks won't help you much since the object is not a real object but a temporary object created by deserialisation. D. There are issues with serialising complex objects - it is not just a performance hit, it may even fail in certain scenario - especially if you have deeply nested objects that sometimes refer back to itself. |
These are all great arguments for why it's dangerous to storing live objects in session without extreme care. That's why we don't support it and don't plan to do so. If you need to do this, it's very important that you be aware of all the underlying issues.
I don't know if I'd suggest removing the documentation entirely but it would certainly be appropriate to add a warning to that documentation describing (very briefly) the concerns raised by serialization. Can you file an issue on https://github.com/aspnet/AspNetCore.Docs for that? Or even better, if you'd like to submit a PR to update https://github.com/aspnet/AspNetCore.Docs/blob/master/aspnetcore/fundamentals/app-state.md with a warning that would be cool. |
Yes, we have discussed great arguments for why to avoid objects in session - serialised or not. There are problems with both approach.
To recommend all together an incorrect workaround, like using serialisation must be avoided by all and not just the documents. However these are architecture level discussions for the targeted solutions, as a framework - it needs to support both the approaches and that is why i still think your decision to not allow the choice to the developers is incorrect - unless there is another reason which is Azure specific that we are not bringing on the table - because anyways developers will find a workaround for it - like using IMemoryCache.
Yes thanks for the suggestion, i will submit a change. I wonder though, If Serialisation solution can walk away with a warning - why can't storing live objects in Session walk away with an equally brief warning too ? Even this document from MS didn't highlight such a big change regarding Session. I think i will rest my case now ! |
I am migrating from asp.net framework 4.7 to core 3.0 - and i understand asp.net core team wants us to go away from sessions - however it is not practical for Change Management for such large projects. There are budget constraints, time constraints and it is important we move away from certain old ways of doing things in a phased manner - while reusing maximum that we can and keep release time shorter.
Session is one of the important aspect of any large scale applications containing complex forms.
I am right now left with following choices in asp.net core:
Either of them will be a huge change which we didn't want to pick up at this stage.
Either ways - an application cannot do without sessions - it is a matter of choice where to keep it - but that should be a call of the development team.
1. Combo of Session and Serialisation leads to many bugs in a threaded environment
Microsofts documentation recommend to serialise the object and store in session - but serialisation is not same as object - going through the process of serialisation and deserialisation we are essentially creating a new object each time and allocating a different memory space to it - which creates problem when user is rapidly making entry and series of parallel requests go to the server that need to perform some operations on the same object
Most of my application is stateless except for this 2 aspects for which we need session
My session had a timeout of 5 mins : and i have implemented heartbeat strategy to keep it active till the user browser is active on any WorkingObject.
My humble suggestion is to provide feature to store object in Session so that legacy code can be migrated easily
I understand eventually we need to move away from session for scalability and server farming - but that is again the choice of the development team - decision of which is based upon time, budget, technology, business model, product model, application needs and user behaviour.
The text was updated successfully, but these errors were encountered: