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

Reactive variables prevent cache from being garbage collected #7622

Closed
kkgrigas opened this issue Jan 27, 2021 · 3 comments
Closed

Reactive variables prevent cache from being garbage collected #7622

kkgrigas opened this issue Jan 27, 2021 · 3 comments

Comments

@kkgrigas
Copy link

kkgrigas commented Jan 27, 2021

Intended outcome:
InMemoryCache is garbage collected from memory after client stops.

This is similar to #7274, but I believe this is a separate issue. Similarly to the linked issue, InMemoryCache never gets evicted after apollo client has finished running during SSR. The client is stopped with apolloClient.stop() at the end. The new code introduced as a fix in #7279 is executed, but it doesn't fix the issue.

image

Actual outcome:
InMemoryCache should be evicted from memory after running.

How to reproduce the issue:
I've made a minimal repo in https://github.com/kkgrigas/apollo-client-sample
Run it, and then make many requests to started server (i.e. using siege)

Versions

System:
OS: macOS 11.1
Binaries:
Node: 14.12.0 - /usr/local/bin/node
Yarn: 1.22.10 - ~/projects/web/node_modules/.bin/yarn
npm: 6.14.8 - /usr/local/bin/npm
Browsers:
Chrome: 88.0.4324.96
Edge: 88.0.705.53
Firefox: 85.0
Safari: 14.0.2
npmPackages:
@apollo/client: 3.3.7 => 3.3.7
apollo: 2.31.1 => 2.31.1
apollo-link-logger: 2.0.0 => 2.0.0
apollo-link-persisted-queries: 0.2.2 => 0.2.2
apollo-upload-client: 14.1.3 => 14.1.3

@kkgrigas kkgrigas changed the title Reactive variables prevent cache from being evicted Reactive variables prevent cache from being garbage collected Jan 27, 2021
@benjamn benjamn self-assigned this Jan 27, 2021
@benjamn
Copy link
Member

benjamn commented Feb 5, 2021

@kkgrigas Do these InMemoryCache objects still have anything in their cache.watches sets? I ask because #7279 will only help if all watches are cancelled before the cache is discarded.

@kkgrigas
Copy link
Author

kkgrigas commented Feb 5, 2021

It doesn't look like it is relevant. I tried SSR'ing different pages which contain different queries, with new InMemoryCache created on each request. Most of them had watches empty or emptied after rendering, one of them didn't.

image

Let me know if I can help somehow else.

@benjamn
Copy link
Member

benjamn commented Feb 9, 2021

@kkgrigas This should be fixed in @apollo/[email protected] (published earlier today), and we now have a regression test that should prevent this kind of leak from happening in the future. Thanks again for the excellent reproduction!

@benjamn benjamn closed this as completed Feb 9, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants