urql icon indicating copy to clipboard operation
urql copied to clipboard

Possible memory leak when using multiple queries on the same page

Open juusopiikkila opened this issue 1 year ago • 13 comments

Describe the bug

When using two queries on the same page and the other one is awaited and the other one is not and that one is using data from the awaited query, there is a memory leak.

Using the reproduction memory usage climbs from ~40 Mb up to ~800 Mb (autocannon with 60 sec duration and 10 connections) and it never comes down.

Reproduction

https://github.com/juusopiikkila/nuxt-urql-memory-leak

Urql version

@urql/vue v1.1.2

Validations

  • [X] I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
  • [X] Read the docs.
  • [X] Follow our Code of Conduct

juusopiikkila avatar Feb 17 '24 21:02 juusopiikkila

Just to clarify before looking into this because that's an important distinction: Does it never come down and stay stuck at 800MB (I assume MB not Mb?) or does it crash?

If your server doesn't actually rise in usage and/or crash it's not a leak, so just clarifying here.

Also just to point this out, there isn't anything special going on here. If you have just two operations (ie two document + variables combos) then it's more of a question how many concurrent queries are actually run as no memory is retained in the core client, bindings, or exchanges 🤔

kitten avatar Feb 18 '24 01:02 kitten

Yeah sorry I meant MB. I tested it again and when the test was completed memory usage was at ~800MB and then it slightly decreased to ~700MB. No crashes, still works fine at that point when manually checking with browser. Memory usage just stays at ~700MB while I waited for 10 minutes for it to come down.

juusopiikkila avatar Feb 18 '24 10:02 juusopiikkila

Hm, ok gotcha 👍 That doesn't necessarily sound like a memory leak, but a Chrome debugger memory snapshot could show you what's being retained and what's using that memory.

Generally, it's possible that a combination of GraphQL result data and Vue app tree data is being retained concurrently. But it's hard to tell how much memory that adds up to. Generally, 800MB could be plausible given that the GC may choose to also not activate an aggressive sweep phase when it still has enough heap space left

kitten avatar Feb 18 '24 23:02 kitten

Ok I've now updated the reproduction repo with clearer tests. It wasn't even about using the data from the first query in the second, it leaks with just two awaited queries. With just one query there isn't any leak. Also I added a mock API endpoint under the api routes so that the external service isn't affecting the results.

I also set max_old_space_size to 512MB to see if it changed anything.

Below are the results from the autocannon tests. I'm not that good at decyphering the snapshots yet so that doesn't help me much. Those memory readings are taken from the Chrome debuggers memory view after each test.

# /no-leak

11k requests in 60.04s, 17 MB read
Memory usage: 23.8 MB

# /leak

9k requests in 60.04s, 13.2 MB read
Memory usage: 1201 MB

juusopiikkila avatar Feb 19 '24 07:02 juusopiikkila

Gotcha! That's awesome. I'll have a look at the memory debugger myself this evening. Typically, I'd assume we've created some kind of promise chain or cycle here 🤔

kitten avatar Feb 20 '24 05:02 kitten

Have you had time to check this one out yet?

juusopiikkila avatar Mar 04 '24 07:03 juusopiikkila

I can confirm we have the same issue on our project. Using @urql/vue v1.1.2 with nuxt 3.

Any page/component which has two queries will retain all the setup context of the component in memory and it is never released. image image image

For a while I thought it was not an issue with urql as it is not just urql objects like graphcache, queries and data that are retained but also other objects like router object, unhead object etc... Basically anything in the setup function.

But if I remove the second query, it always fixes the problem, on all of the pages/components.

@kitten Have you had a chance to look at this issue yet ?

Bcavez avatar Mar 26 '24 16:03 Bcavez

It seems that I'm in the same boat. My SSR is failing and getting killed by OOM. I've been profiling for a long time, but the only clues I have point to the Store object from graphcache. I'm not entirely sure how to verify this, as I don't see any obvious references where it could be stored. image

I had to increase the amount of memory to reduce the crash rate. image

negezor avatar Jun 17 '24 12:06 negezor

@negezor are you by any chance initializing your graphCache globally? If so you might want to move that to your boostrapping code so that it can be garbage collected when your SSR completes.

JoviDeCroock avatar Jun 17 '24 12:06 JoviDeCroock

@JoviDeCroock no. I have a new instance of the application initialized for each request. Including graphcache, otherwise the same data would be sent for each user and it would be a data access violation.

negezor avatar Jun 17 '24 12:06 negezor

After investigating further, I seem to have some ideas about why the memory leak is happening. For some reason, the request object, which is not being mutated on my end, has a large number of ReactiveEffect. Screenshot 2024-06-18 040625

Apparently, this is happening here. https://github.com/urql-graphql/urql/blob/118d74b238007264cacfb91fc12de74370d5766e/packages/vue-urql/src/useQuery.ts#L248

UPD: By applying markRaw(QueryDocument) from vue the memory leak disappeared. But I don't think this should be the solution, we should get rid of reactive() for args.

negezor avatar Jun 17 '24 18:06 negezor

@negezor, https://github.com/urql-graphql/urql/pull/3612 completely breaks reactivity support for variables, like { variables: { somevar: ref('value') }}, as reactive() unwraps all nested reactive props

yurks avatar Jun 18 '24 12:06 yurks

@JoviDeCroock @negezor this issue should be resolved in https://github.com/urql-graphql/urql/pull/3619 without side-effects

yurks avatar Jun 18 '24 17:06 yurks

I have consistent memory leak, but I don't know where from. Memory raises from 300MB to 4.5GB (which is maximum of the server itself) and then crashes with OOM error. Waiting for release to test if leak was coming from here. :)

Warxcell avatar Jul 26 '24 15:07 Warxcell

Confirmed. After upgrading to 1.4.0-canary-068df71f - Now memory stays at ~200MB.

Edit: But when deployed on production - memory again grows. Probably I have another memory leak. lol

Warxcell avatar Jul 26 '24 15:07 Warxcell