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

@vue/apollo-option - memory leak on SSR render #1550

Open
amenella opened this issue Apr 16, 2024 · 5 comments · May be fixed by #1553
Open

@vue/apollo-option - memory leak on SSR render #1550

amenella opened this issue Apr 16, 2024 · 5 comments · May be fixed by #1553

Comments

@amenella
Copy link

Description

Hi,

We've encountered some memory leaks using @vue/apollo-option (through the related Nuxt 3 plugin : https://github.com/nuxt-modules/apollo).

After some debug we found that the context of following method is constantly increasing and seems never released:

My best call would be to first destroy the context then re-render, but this is only a feeling and I did not tried (on those lines).

This behaviour has only been seen on production (maybe due production build + traffic).

Here are some screenshot of Node.js debug mode on production, on which we can see that the memory allocation is constantly increasing:
image

The line referred in previous screenshot ☝️ led us to those particular lines:
image

Reproduction

Unfortunately I have no easy example to reproduce this issue (we are not even able to reproduce it with the same stack on a non-production environment...)

Expected behavior

No memory leak

Versions

vue - 3.3.11
@nuxtjs/apollo - 5.0.0-alpha.11
@apollo/client: 3.8.8
@vue/apollo-option: 4.0.0-beta.12
@vue/apollo-composable - 4.0.0-beta.12

Additional context

This issue seems related to some old ones (which should have fixed it):

  1. @vue/apollo-option fails during ssr #1424
  2. Memory leak with Nuxt SSR #806

We also have a specific usage of apollo client within the Nuxt app using a specific mixin to allow update of endpoint at runtime using env var, which is highly inspired from nuxt-modules/apollo#442 (comment)

Any help or hint would be appreciated, thanks a lot

deleteme added a commit to deleteme/vuejs-apollo that referenced this issue May 9, 2024
Fixes vuejs#1550

Two leaks were fixed:

1) Prevents repeatedly wrapping `ssrRender` by checking if it's already
   been wrapped. Added a `__IS_VUE_APOLLO_WRAPPED` boolean to track this.
   I verified that this was actually happening by throwing an error if
   it was already wrapped, and I observed the error.

2) `this.$options.ssrRender` doesn't always exist, but this.$apollo
   does. When the new wrapped `ssrRender` was called, it would throw,
   which prevented the `destroy.call(this)` line from running. The fix
   here was to not create a wrapped `ssrRender` if there isn't an original
   one.
deleteme added a commit to deleteme/vuejs-apollo that referenced this issue May 9, 2024
Fixes vuejs#1550

Two leaks were fixed:

1) Prevents repeatedly wrapping `ssrRender` by checking if it's already
   been wrapped. Added a `__IS_VUE_APOLLO_WRAPPED` boolean to track this.
   I verified that this was actually happening by throwing an error if
   it was already wrapped, and I observed the error.

2) `this.$options.ssrRender` doesn't always exist, but this.$apollo
   does. When the new wrapped `ssrRender` was called, it would throw,
   which prevented the `destroy.call(this)` line from running. The fix
   here was to not create a wrapped `ssrRender` if there isn't an original
   one.
@deleteme
Copy link

deleteme commented May 9, 2024

@amenella #1553 this might help. Is it possible for you to try this branch out?

@amenella
Copy link
Author

Hi @deleteme, thanks a lot for the suggestion! 🙏

I'll try to test this asap on a test environment, however like said in my original message, we have only been able to reproduce this on production for the moment

@amenella
Copy link
Author

Hi again, I just tried with your suggestion, however I've got an error during the build on dev mode:

ERROR  Pre-transform error: Failed to resolve entry for package "@vue/apollo-option". The package may have incorrect main/module/exports specified in its package.json.

which lead to following Nuxt error:

image

I tried the suggestion using a yarn override in my package.json:

  "resolutions": {
    "@vue/apollo-option": "git+https://github.com/deleteme/vuejs-apollo.git#fix-memory-leak-in-vue-apollo-option-beforeCreate"
  }

the relaunching the dev mode (after reinstalling my deps): nuxt dev

I've also tried to manually build the deps (directly in my node_modules) by following the contribution guide but it leads to the same error.

am I missing something here?

@deleteme
Copy link

@amenella Sorry, I am not familiar with the build process in this repository. You can use yarn patch to replicate the changes from https://github.com/vuejs/apollo/pull/1553/files.

@amenella
Copy link
Author

hi again @deleteme , we've just tested the fix (not in production though) and the memory usage seems stable with the fix! 🙌

plus, looking at the code, changes seem good to me, however, I'm not maintainer in this project so this is not my word to say

thanks again for your time 🙏

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

Successfully merging a pull request may close this issue.

2 participants