search

mercurius-js / mercurius

Implement GraphQL servers and gateways with Fastify
https://mercurius.dev/
MIT License
2.36k stars 238 forks source link

Memory leak in withFilter #1120

Open Sitronik opened 1 month ago

Sitronik commented 1 month ago

Hello. Recently I was looking for a memory leak in my project and noticed that there is a problem in subscriptions namely AsyncGenerator Further analysis of the code led me to the subscriber.js file of your library and here I saw the withFilter function, which is actually the reason why the memory leak occurs. I believe this issue is exposed if you are using async generators syntax sugar, e.g In this case the return call from graphql into the asyncIterable returned by this function isn't properly propagated to the pubsub-returned asyncIterator, so it never unsubscribes.

In my project I use nestjs and pubsub redis, so I easily changed the mercuris driver to yoga then I checked or there is a leak there, it is not there since they do not use async generators syntax there

To reproduce the leak you need to take one snapshot, then it will be enough to refresh the page up to five times on which there is a subscription

Similar problems related to async generators: https://github.com/davidyaha/graphql-redis-subscriptions/issues/124 https://github.com/tc39/proposal-async-iteration/issues/126

Thanks, for your work.

memory-leak
mcollina commented 1 month ago

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

Sitronik commented 1 month ago

I would love to add it, but right now I really don’t have the time, essentially you just need to rewrite the withFilter function without asyncGenerator, maybe in the future I’ll have some free time and I’ll deal with this problem if it’s still relevant