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

Cache key based on "self" in @cached decorator might not always be unique #734

Closed
joakimnordling opened this issue Jun 26, 2023 · 1 comment

Comments

@joakimnordling
Copy link

I found out the hard way that the default behaviour of get_cache_key (or well actually _key_from_args) in the cached decorator is a bit dangerous when using it on a method of a class. By default it converts args to a string, thus rendering self to something like <foo.bar.Baz object at 0x120e9a070> as part of the cache key.

In most cases this is likely not going to cause any issues, but we were actually in our unit tests creating a lot of instances of the class and calling the method with no arguments. Our tests got random failures and in the end I was able to track it down to being caused by the fact that the same memory address was once in a while reused for a new instance and thus old, cached data from the previous <foo.bar.Baz object at 0x120e9a070> was returned by the decorator instead of the cache being empty for the new instance that happened to be in the same memory address.

For now I was able to get around the issue by adding a dummy uuid to each instance and creating a custom key_builder function that uses that uuid to ensure the cache is indeed unique to each instance (current or past).

If you can figure out some way of ensuring the cache would generate the key in a safer way, it'd be highly appreciated. Also adding some sort of disclaimer of the current behaviour to the README or to the documentation of the noself parameter would be nice in lack of a proper fix.

joakimnordling added a commit to ioxiocom/pyjwt-key-fetcher that referenced this issue Jun 26, 2023
### Fixed

- In rare conditions the cache used by a `Provider` could return old data from another no longer existing `Provider`. This was seen in some unit tests that created a lot of `AsyncKeyFetcher` instances which created a lot of `Provider` instances. The main reason to the problem was that the `aiocache` library would create the cache key using a string like `<pyjwt_key_fetcher.provider.Provider object at 0x120e9a070>` that then got reused by a new instance occupying the same memory address later. This is now fixed by ensuring each provider instance gets a UUID and it's used in the cache key. An [issue was opened in aiocache](aio-libs/aiocache#734) regarding this. This issue would likely not have affected any real world use cases.

### Changed

- Updated `PyJWT`, `cachetools` and `aiocache`.
@Dreamsorcerer
Copy link
Member

Duplicate #641.

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

2 participants