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.