[Memory-Leak]: Unsubscribe and return from iterator is missed, when using subscription´s onDisconnect

[TimSusa]

Notes

• Memory-Leak is appearing
• We like to make this project more resilient to memory leaks
• Maybe a code Smell at pullValue could be assumed
• In some eslint rules, for example, it is strongly recommended to avoid creating promises via "new"
• The code was taken into Steffi-Graph for debugging analysis
• Implemented subscriptions onDisconnect to close the connection, which is triggered from the client (closing his browsertab for example)

Analysis of Log Data (We found a moment, where unsubscribe would be missed):

steffi-graph: subscribe  { topicName: 'article',
steffi-graph:   onMessage: [AsyncFunction: bound pushValue],
steffi-graph:   options: undefined }
steffi-graph: getSubscriptions  
steffi-graph: exists?   true article
steffi-graph: subscribe all [ 6 ]
steffi-graph: next  Promise { [ 6 ] }  listening?:  true
steffi-graph: (not resolved yet) pullQueue: pushed resolve reference to queue with length:  1
//
// Unsubscribe and return seems to be missing here!
// 
steffi-graph: onDissconnect initial context  { auth:
steffi-graph:    { user:
steffi-graph:       { accountname: 'admin',
steffi-graph:        
steffi-graph:      isAuthenticated: true,
steffi-graph:      scope:
steffi-graph:   ...
steffi-graph:   token: ...

//
// Websocket connection to specific client should be closed here after onDisconnect
//

Question

Would an alternative approach make sense here?

New Approach to discuss

...

Feedback from Nacho (Google) to this Implementation

It seems that you have 800k instances of Promises, which is taking 80% of your memory. This most frequently happens when a Promise is neither resolved or rejected, which looking at your code could be happening at pullValue:

pullValue() {
    return new Promise(resolve => {
      if (this.pushQueue.length !== 0) {
        const res = this.pushQueue.shift()
        resolve({ value: res, done: false })
      } else {
        this.pullQueue.push(resolve)
      }
    })
  }

You are delaying the resolution of the Promise by adding it to pullQueue, could it be that nobody is pulling that data later? My guess is that there is no cleanup of these delayed Promises by either resolving or rejecting.

A test case for this would be useful. I am forwarding this to a colleague in case they can offer an additional insight.

[frandiox]

@TimSusa @jonas-arkulpa Hi! Is there any news about this? I've been reading related issues and I'm not sure anymore if this is a problem in this package, in user apps, or in graphql-subscriptions package itself.

I'd like to horizontally scale my GraphQL server soon and I think subscriptions are the only concern for now. Any workaround or hint to minimize these problems?

Thanks!

[jonas-app]

Hey @frandiox ,
currently I have unfortunately no time to investigate and fix this issue.
As far as I know it's kind of the same situation for @TimSusa .

If I would currently plan to scale a GraphQL server I would separate the subscription part (stateful) from the rest (stateless) into different services.
Thats the best advice I can offer you atm.

More input or fixing PRs would be very welcome.

P.S.: Still active in the onsen-ui community? 🙂

[frandiox]

@jonas-arkulpa I see, thanks for the answer!

If I would currently plan to scale a GraphQL server I would separate the subscription part (stateful) from the rest (stateless) into different services.

Yeah I was thinking in deploying the server twice under two domains and use one of them only for subscriptions. If it crashes at least it won't take down the whole app.

P.S.: Still active in the onsen-ui community?

Hah, not really, I left almost 2 years ago, but it's nice to see people who still know about that project 😄

[umran]

Hey guys, any updates on this issue? I would really like to use this in my project and am willing to contribute my time to help fix this bug. Please let me know if anyone has more information regarding the memory leak. I have only just discovered this repo and still familiarizing myself with the source.

p.s.
thanks for an awesome library. I really appreciate the work that's been done already.

[jonas-app]

Hey Umran, thx for your interest! Any available information is documented in the open issues. I missed documenting our findings and open ends at the time we invested more time investigating. Feel free to open a PR if you find something helpful, I am happy to take a look and review the changes. But otherwise I unfortunately do not have any spare time for this project as stated above.