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

Cached instances of objects can be altered outside of Stash #377

Open
Ruud-Zuiderlicht opened this issue Jan 30, 2018 · 3 comments
Open

Comments

@Ruud-Zuiderlicht
Copy link

I'm getting "Serialization of Closure is not allowed" in concrete5 which uses Stash. This happens because objects stored in or returned by Empheral are a reference and any changes to the original/returned item also alters the stored instance in cache.
Therefore requesting the same cached item twice might result in different returned values even though the key has not been altered through a regular Stash set function.

This might be intended behaviour, although I don't feel it is really intuitive. It was suggested in the issue (concretecms/concretecms#6113) that this could be something that can be changed in Stash as well.

footnote: concrete5 clones objects before caching in Stash, but retrieves the cached instance without cloning. So the problem is less apparent there.

@Ruud-Zuiderlicht Ruud-Zuiderlicht changed the title Serialization of Closure is not allowed Cached instances of objects can be altered outside of Stash Jan 30, 2018
@orcinus
Copy link

orcinus commented Apr 16, 2018

I believe the common pattern is to always return a clone of the cached object, whether it's from an in-memory array cache, or an "external" cache. Decoupling this from the adapter would, however, require using proxies for the get methods (that call the adapter implementation + do the cloning).

@tedivm
Copy link
Member

tedivm commented Apr 16, 2018

Since this is specific to the ephemeral driver I'd recommend making a PR that clones the data on read from that driver.

@orcinus
Copy link

orcinus commented Apr 16, 2018

I'm planning on using it to test some long-running process stuff this weekend. If no one makes a PR for it sooner, i'll do it on weekend.

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

3 participants