search

russelltg / srt-rs

SRT implementation in Rust
Apache License 2.0
236 stars 36 forks source link

Gathering statistics on SrtListener is blocked unless all clients are dropped #172

Closed Lighty0410 closed 2 years ago

Lighty0410 commented 2 years ago

I'm trying to gather statistics from SrtListener. However, there's a problem. As soon as a client connects to the multiplex server, statistics gathering is blocked. And it's being locked until all clients are dropped. How to reproduce: 1) Run the multiplexer with RUST_LOG=info - https://gist.github.com/Lighty0410/424dd2af7da8d45543ad208644b3b5ac 2) Run the client (receiver) - https://github.com/russelltg/srt-rs/blob/main/srt-tokio/examples/receiver.rs 3) Look at the multiplex server terminal. You can notice as soon as the client connects to the server there are no updates on the listener statistics. Statistics is gathering internally, however, the method call binding.statistics().next() is blocked.
4) Shut down all clients. 5) Look at the multiplex server terminal again. Now you can see the statistics get updates again.

I tried selecting futures instead of spawning another tokio-thread, but the outcome is all the same.

robertream commented 2 years ago

I didn't have time to debug this, but I took a look at this and I suspect the problem is that the "timeout" in the SrtListener loop is getting starved. I pushed a draft PR with a proposed fix that uses an interval instead of the sleep_until. I probably copied the sleep_until approach of SrtSocket, because SrtSocket buffers packets and sends based on a timer (i.e. SND timer), but this is inappropriate for the SrtListener, which doesn't buffer packets but rather handles/routes them immediately, and since the sleep_until is recalculated on each pass through the loop, the timer is only triggered when no packets are being sent or received. This PR ensures that an interval timer will be triggered every 100ms, regardless of I/O.

If you could build locally and test it out, I'll merge it later today.

Lighty0410 commented 2 years ago

I didn't have time to debug this, but I took a look at this and I suspect the problem is that the "timeout" in the SrtListener loop is getting starved. I pushed a draft PR with a proposed fix that uses an interval instead of the sleep_until. I probably copied the sleep_until approach of SrtSocket, because SrtSocket buffers packets and sends based on a timer (i.e. SND timer), but this is inappropriate for the SrtListener, which doesn't buffer packets but rather handles/routes them immediately, and since the sleep_until is recalculated on each pass through the loop, the timer is only triggered when no packets are being sent or received. This PR ensures that an interval timer will be triggered every 100ms, regardless of I/O.

If you could build locally and test it out, I'll merge it later today.

Sure ! I'm gonna test it right now. Thanks a lot !