Commit graph

9 commits

Author SHA1 Message Date
François Laignel
91bfd0f7c3 webrtc: signallers: attempt to close the ws when an error occurs
This commit discards the early error returns in the send tasks to log the error
and attempt to close the websocket.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1435>
2024-02-01 18:08:41 +01:00
François Laignel
f54d714afd webrtc: only use close() to close websockets
In the signaller clients and servers, the following sequence is used to close
the websocket (in the [send task]):

```rust
    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:👿: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.

[send task]: 63b568f4a0/net/webrtc/signalling/src/server/mod.rs (L165)
[`WebSocket::close()` doc]: https://docs.rs/tungstenite/0.21.0/tungstenite/protocol/struct.WebSocket.html#method.close
[returns `SendAfterClosing`]: 85463b264e/src/protocol/mod.rs (L437)
[call to `do_close()`]: 85463b264e/src/protocol/mod.rs (L601)
[`close()` ensures the ws is active]: 85463b264e/src/protocol/mod.rs (L531)

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1435>
2024-02-01 18:08:41 +01:00
Nirbheek Chauhan
63b568f4a0 webrtc/signalling: Fix potential hang and FD leak
If a peer connects via TCP and never initiates TLS, then the server
will get stuck in the accept loop. Spawn a task when accepting a TLS
connection, and timeout if it doesn't complete in 5 seconds.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1412>
2023-12-29 13:28:48 +00:00
Mathieu Duponchelle
7447d95f1b webrtc/signalling: fix race condition in message ordering
Spawning one task per message to send out instead of sending them out
sequentially from the one task used to poll the handler sometimes
resulted in peers receiving ICE candidates before SDP offers, triggering
hard to understand errors in the browser.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1236>
2023-06-08 13:24:45 +02:00
Sebastian Dröge
3b4c48d9f5 Fix various new clippy warnings
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1062>
2023-01-25 10:31:19 +02:00
Zhao, Gang
9fa838e366 webrtc: Fix rustfmt errors
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1019>
2022-12-27 11:12:54 +02:00
Zhao, Gang
1ffeb4d44d webrtc: Move from async-std to tokio
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1019>
2022-12-26 23:10:40 +00:00
Thibault Saunier
5ab7be6124 webrtc: Add SDPX license header on every file 2022-10-20 11:51:58 +02:00
Thibault Saunier
5e7537953c webrtc: Move to net/webrtc 2022-10-18 15:18:53 +02:00