Azure / azure-c-shared-utility

Azure C SDKs common code
Other
109 stars 203 forks source link

uws_client: No indication that WebSocket has been closed, if close is initiated by server. #616

Open minichma opened 1 year ago

minichma commented 1 year ago

This affects the uws_client WebSocket client. In cases where the server (rather than the client) initiates the closing handshake by sending a close frame, uws_client will respond to the server with a corresponding response frame and invoke the on_ws_peer_closed callback which was provided to uws_client_open_async(). When sending the close frame is completed, the underlying connection will eventually be closed. However at this point there is no callback indicating that the closing procedure has been completed to the user. As a consequence the user has no chance to know, for how much longer after receiving the on_ws_peer_closed callback it must continue to call uws_client_dowork. There's also no function that could be used to query the client's state.

There is a on_ws_close_complete callback that can be passed to uws_client_close_async(), but this function only comes into play if the closed handshake is initiated by the client, not in case it's initiated by the server.

Suggestion: Allow passing an on_ws_closed callback to uws_client_open_async() which would be called as soon as uws_client reaches a final state (i.e. the underlying connection is closed).

ewertons commented 1 year ago

HI @minichma,

After uws_client sends the CLOSE frame response https://github.com/Azure/azure-c-shared-utility/blob/master/src/uws_client.c#L1554 It does close the underlying I/O right after: https://github.com/Azure/azure-c-shared-utility/blob/master/src/uws_client.c#L1561 Before invoking on_ws_peer_closed https://github.com/Azure/azure-c-shared-utility/blob/master/src/uws_client.c#L1573

So if you receive a on_ws_peer_closed, you can stop calling dowork right away and destroy the client.

minichma commented 1 year ago

Hi @ewertons,

Thanks for the clarification and sorry for the late reply. Maybe I'm understanding this wrong, but to my understanding the underlying xio could be implemented in an asynchronous manner. Therefore after calling the close function, the underlying layers might still be busy. After uws_client reports it's ready, I would expect that I can just tear down the underlying stack, but this doesn't seem to be the case. Instead I have to ask the underlying layer whether it's still busy and tear it down only when its finished. So I would expect uws_client to signal that closing is complete only after closing of the underlying TLS/TCP connection has completed as well. To my understanding this is how uws_client_close_async handles the closing procedure.

Does this make sense?