Memory Leak - InMemoryCache and Optimism never deletes records

[kamilkisiela]

In a project I work on (used by milions of people) we use Electron and in every pop-out window, we pass a reference to Window object within an operation's context. Because the callback holds a reference to QueryInfo => ObservableQuery => ObservableQuery.options => options.context => context.window we have a memory leak.
Consumers open new windows every few minutes (it's a chat app) which adds more and more instances of Window.

How to reproduce the issue:

Repository | Deployed app

Here, we use dep from optimism package that creates a Map:

apollo-client/src/cache/inmemory/inMemoryCache.ts

Line 356 in 481b30d

private watchDep = dep<Cache.WatchOptions>();

When there's a new query, InMemoryCache runs broadcastWatch method in which it uses optimism.
The watchDep.dirty(c) removes a record from the Map

apollo-client/src/cache/inmemory/inMemoryCache.ts

Line 375 in 481b30d

this.watchDep.dirty(c);

Later on in broadcastWatch, the watchDep(c) is called and it sets a new record in our Map:

apollo-client/src/cache/inmemory/inMemoryCache.ts

Line 383 in 481b30d

this.watchDep(c);

The problem is that c which represents Cache.WatchOptions

apollo-client/src/cache/core/types/Cache.ts

Lines 25 to 28 in 481b30d

	export interface WatchOptions extends ReadOptions {
	immediate?: boolean;
	callback: WatchCallback;
	}

is stored forever in the Map created in Optimism's dep() and that callback is function that depends on QueryInfo class

apollo-client/src/core/QueryInfo.ts

Lines 246 to 251 in 481b30d

	this.cancel = this.cache.watch(this.lastWatch = {
	query: this.document!,
	variables,
	optimistic: true,
	callback: diff => this.setDiff(diff),
	});

which depends directly on ObservableQuery class in which we store query's options operation's context.

Versions

  System:
    OS: macOS 10.15.6
  Binaries:
    Node: 10.18.0 - ~/.nvm/versions/node/v10.18.0/bin/node
    Yarn: 1.22.4 - ~/.yarn/bin/yarn
    npm: 6.13.4 - ~/.nvm/versions/node/v10.18.0/bin/npm
  Browsers:
    Chrome: 85.0.4183.121
    Edge: 85.0.564.63
    Firefox: 79.0
    Safari: 13.1.2
  npmPackages:
    @apollo/client: 3.2.0 => 3.2.0

[kamilkisiela]

@benjamn I'm not entirely sure how all of that works but I can help to fix it, just need few clues :)

[kamilkisiela]

I believe we store callback in multiple places, not only in InMemoryCache.watchDep

[smykhailov]

#7013 Might be related to this

[benjamn]

The optimism wrapper system does use a bounded LRU cache for every wrapped function, so the amount of cached data cannot grow indefinitely, but the default size is pretty large.

[dotansimha]

@benjamn I believe this issue should get more attention. Since we migrated a huge project to v3, we see multiple memory leaks.
The one @kamilkisiela demonstrated above is only one, we are chasing those and it's not easy to find. Here's an example for a heap snapshot. It's using Electron and opens a new Window. Eventually, we have Detached Window(s) since Apollo keeps records of the query params in memory, and never deletes it.

Here's another heap snapshot of a memory leak we chase, and caused by retained context from observableQuery:

I think the 2 issues above might be related, and caused by the same issue, and @kamilkisiela 's issue is just one symptom of it. Both seems to be related to watchDep and the way it keeps it's records.

[benjamn]

Thanks to @dotansimha's reproduction and @kamilkisiela's proposed solution, I'm confident this leak is fixed in @apollo/client@3.2.4 and @apollo/client@3.3.0-beta.12. Thanks for digging into these implementation details and correctly identifying the underlying problem, @kamilkisiela!