[Bug] Subscription result handling can miss result

[b0l0k]

Describe the bug Subscribe with streaming rpc client can miss subscription result stucking the subscription state in WaitingResult

To Reproduce There is no perfect scenario given this a race condition, but if I've the feeling that if you subscribe twice on the same account, given the second subscription is faster then, the race condition happens more often.

When I see

        private async Task<SubscriptionState> Subscribe(SubscriptionState sub, JsonRpcRequest msg)
        {
....
                await ClientSocket.SendAsync(mem, WebSocketMessageType.Text, true, CancellationToken.None).ConfigureAwait(false);
                AddSubscription(sub, msg.Id);
....
        }

I suspect, as there is concurrency between send and receive, it is possible that we receive the subscription result before adding the subscription in unconfirmedRequests

I would suggest to change the order of this two lines (or add a lock) and also, to add logs in ConfirmSubscription if we receive a confirmation that we don't know. What do you think ?

Expected behavior Never loose a subscription result.

[b0l0k]

My suspicion was not good, here the proofs of the bug and that the ws client received the result (subscriptionId filled)

Logs

[Sending]{"method":"accountSubscribe","params":["83astBRguLMdt2h5U1Tpdq5tjFoJ6noeGwaY3mDLVcri",{"encoding":"base64","commitment":"confirmed"}],"jsonrpc":"2.0","id":0}
[Received]{"jsonrpc":"2.0","result":528003,"id":0}
[Sending]{"method":"slotSubscribe","jsonrpc":"2.0","id":1}
[Received]{"jsonrpc":"2.0","result":486504,"id":1}
[Received]{"jsonrpc":"2.0","method":"slotNotification","params":{"result":{"slot":154294774,"parent":154294773,"root":154294742},"subscription":486504}}
[Received]{"jsonrpc":"2.0","method":"slotNotification","params":{"result":{"slot":154294775,"parent":154294774,"root":154294743},"subscription":486504}}
[Sending]{"method":"accountSubscribe","params":["83astBRguLMdt2h5U1Tpdq5tjFoJ6noeGwaY3mDLVcri",{"encoding":"base64","commitment":"confirmed"}],"jsonrpc":"2.0","id":2}
[Received]{"jsonrpc":"2.0","method":"slotNotification","params":{"result":{"slot":154294776,"parent":154294775,"root":154294744},"subscription":486504}}
[Received]{"jsonrpc":"2.0","result":528003,"id":2}

[b0l0k]

After analysis, I think that the client doesn't manage correctly if the subscriptionId is already known.

I suspect that confirmedSubscriptions.Add(resultId, sub); is crashing and prevent sub?.ChangeState(SubscriptionStatus.Subscribed);

[tiago18c]

The problem here is that the library wasn't made with multiple subscriptions of the same object in mind.

Ideally, you want to create an abstraction layer that manages account subscriptions and that only calls the underlying StreamingRpcClient.SubscribeAccountInfo once per address, store the resulting subscription object and a list of handlers to be notified on your app.

[b0l0k]

@tiago18c It's what we did. Do I have to understand that you won't fix that? It can be disturbing to have a SubscriptionState which is stucked.