Duplicate GraphQL subscriptions sent

[rjwills28]

This issue relates to the memory leak found in the Coniql application: DiamondLightSource/coniql#35

While debugging the memory leak in the Coniql application, I came across an issue in the GraphQL subscription mechanism in the cs-web-lib code. I was able to consistently reproduce a case that caused the memory leak in Coniql and one that did not.

Memory leak: This occurs when the a page is loaded which is subscribing to PVs through Coniql, but Coniql has not yet been started (or has been taken down). When the page loads, the subscribe commands are sent from the web app. As Coniql has not yet been started, the application performs an unsubscribe after a disconnect. When Coniql is started, the web app sends another subscribe on reconnection. The previous subscribe on start up has not been cancelled (for some reason) and so 2 subscribes are picked up by Coniql. One of these is old and appears to not be attached to a client - this might be what is causing the memory leak.

No memory leak: If Coniql is already running before a web page subscribing to PVs is loaded then there is no memory leak. The subscribe is sent once and only once. Note that if you then bring Coniql down and restart it, we will end up in the first situation and get a memory leak.

Possibly something wrong with the unsubscribe that is not clearing the initial subscription on disconnect.

[rjwills28]

I have got to the bottom of why duplicate subscriptions are sometimes sent from the web application and I have developed a fix. There were two issues to address:

1. The web app would send a GraphQL subscribe even if the websocket was not connected. Internally it would then try to disconnect but as the GraphQL server (i.e. Coniql) is not running, it cannot handle the unsubscribe() so it tries to handle it when it does start up. The fix for this is to only subscribe if the websocket client has connected, otherwise add this to the list of disconnected PVs so that when the connection is established, they will be 'reconnected'. Internally we can pick up when the websocket client connects and so set a variable to represent this.
2. The unsubscribe() (on disconnect) was not fully removing the subscription from the websocket and so was never removed from the GraphQL server (Coniql). On the next reconnection we add yet another subscribe leading to many duplicate subscriptions which leak memory. After reading advice online I added an unsubscribeAll() and close() to the websocket on disconnect.

I have run several tests monitoring the number of connections in Coniql as well as the memory usage. This fixes the issue under any of the following circumstances:

• Coniql running and then displaying a webpage
• Coniql not running, displaying a webpage and then starting Coniql
• Coniql running + webpage, stop Coniql (disconnect), start up Coniql again (reconnect)
• Coniql running + webpage, close webpage, reopen webpage.

I have also updated the tests and added a new one to check that the subscribe() does not occur if the websocket client is not connected.
Changes are in the branch: https://github.com/dls-controls/cs-web-lib/tree/issue11_duplicate_subscriptions
Draft PR: #12

[aawdls]

@AlexanderWells-diamond We should check whether the same issue exists in the pts app, as likely you followed some similar patterns to cs-web-proto.

[AlexanderWells-diamond]

I have certainly seen similar a similar issue - on my development setup I see a GraphQL connection to Coniql established and then closed every minute. Strangely this does not happen in the deployed Production build. I also note that the PTS app does not use the SubscriptionClient, instead using ApolloClient and its useSubscription hook. So I suspect its unlikely to be the same issue, but I'll raise one anyway for the weird behaviour in the Development environment.

[rjwills28]

Adding link to another related issue in Coniql resulting in duplicate subscriptions: DiamondLightSource/coniql#34

[aawdls]

Fixed with #12