Skip to content
This repository has been archived by the owner on Dec 19, 2023. It is now read-only.

fix: Memory leak when using reactive subscriptions with KeepAlive #642

Conversation

KriKroff
Copy link

Hi !

This PR should fix a memory leak when using webflux with subscriptions and KeepAlive.

Explanation

ApolloSubscriptionKeepAliveRunner is responsible for sending keep alive messages through the websocket as long as the websocketSubscriptionSession is "opened". This runner, uses a scheduled Executor for this purpose.
When a subscription is closed, of an exception occurs while sending message, KeepAlive message is not more scheduled and everything works fine.

Bug :
ReactiveWebSocketSubscriptionSession is the object representing a Reactive Subscription, this object always return trueon the isOpenmethod, the keepAliveRunner will never stop sending messages, and stack references to dead subscriptions (Memory leak).
No exception occurs while sending messages on a closed subscription, because we are just publishing to a Flux without subscribers. See ReactiveWebSocketSubscriptionsHandler.

A memory leak is also present on SingleSubscriberPublisher in graphql-java project as we keep adding data to the ConcurrentLinkedDeque even if there is no subscribers.

Fix :
ReactiveWebSocketSubscriptionSession is now catching the call to close method to update the internal state of the websocket.

@oliemansm
Copy link
Member

Thanks a lot for your contribution @KriKroff !

@oliemansm oliemansm added this to the 12.0.0 milestone Jun 4, 2021
@KriKroff KriKroff force-pushed the fix/reactive-subscriptions-memory-leak branch from 088b32c to b98a883 Compare July 9, 2021 16:39
@setchy setchy merged commit 91ea773 into graphql-java-kickstart:master Jul 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants