Skip to content

webrtc: signaller: fix closing handshake

Fixing closing handshake (first commit)

In the signaller clients and servers, the following sequence is used to close the websocket (in the send task):

    ws_sink.send(WsMessage::Close(None)).await?;
    ws_sink.close().await?;

tungstenite's WebSocket::close() doc states:

Calling this function is the same as calling `write(Message::Close(..))``

So we might think they are redundant and either could be used for this purpose (send() calls write(), then flush()).

The result is actually is bit different as write() starts by checking the state of the connection and returns SendAfterClosing if the socket is no longer active, which is the case when a closing request has been received from the peer via a call to do_close()). Note that do_close() also enqueues a Close frame.

This behaviour is visible from the server's logs:

1. tungstenite::protocol: Received close frame: None
2. tungstenite::protocol: Replying to close with Frame { header: FrameHeader { .., opcode: Control(Close), .. }, payload: [] }
3. gst_plugin_webrtc_signalling::server: Received message Ok(Close(None))
4. gst_plugin_webrtc_signalling::server: connection closed: None this_id=cb13892f-b4d5-4d59-95e2-b3873a7bd319
5. remove_peer{peer_id="cb13892f-b4d5-4d59-95e2-b3873a7bd319"}: gst_plugin_webrtc_signalling::server: close time.busy=285µs time.idle=55.5µs
6. async_tungstenite: websocket start_send error: WebSocket protocol error: Sending after closing is not allowed

1: The server's websocket receives the peer's Close(None). 2: do_close() enqueues a Close frame. 3: The incoming Close(None) is handled by the server. 4 & 5: perform session closing. 6: ws_sink.send(WsMessage::Close(None)) attempts to write() while the ws is no longer active. The error causes an early return, which means that the enqueued Close frame is not flushed.

Depending on the peer's shutdown sequence, this can result in the following error, which can bubble up as a Message on the application's bus:

ERROR: from element /GstPipeline:pipeline0/GstWebRTCSrc:webrtcsrc0: GStreamer encountered a general stream error.
Additional debug info:
net/webrtc/src/webrtcsrc/imp.rs(625): gstrswebrtc::webrtcsrc::imp::BaseWebRTCSrc::connect_signaller::{{closure}}::{{closure}} (): /GstPipeline:pipeline0/GstWebRTCSrc:webrtcsrc0:
Signalling error: Error receiving: WebSocket protocol error: Connection reset without closing handshake

On the other hand, close() ensures the ws is active before attempting to write a Close frame. If it's not, it only flushes the stream.

Thus, when we want to be able to close the websocket and/or to honor the closing handshake in response to the peer Close message, the ws_sink.close() variant is preferable.

This can be verified in the resulting server's logs:

tungstenite::protocol: Received close frame: None
tungstenite::protocol: Replying to close with Frame { header: FrameHeader { is_final: true, rsv1: false, rsv2: false, rsv3: false, opcode: Control(Close), mask: None}, payload: [] }
gst_plugin_webrtc_signalling::server: Received message Ok(Close(None))
gst_plugin_webrtc_signalling::server: connection closed: None this_id=192ed7ff-3b9d-45c5-be66-872cbe67d190
remove_peer{peer_id="192ed7ff-3b9d-45c5-be66-872cbe67d190"}: gst_plugin_webrtc_signalling::server: close time.busy=22.7µs time.idle=37.4µs
tungstenite::protocol: Sending pong/close

We now get the notification Sending pong/close (the closing handshake) instead of websocket start_send error from step 6 with previous variant.

The Connection reset without closing handshake was not observed after this change.

Logging and closing after a send task error (second commit)

This commit discards the early error returns in the send tasks to log the error and attempt to close the websocket.

Merge request reports

Loading