search

Glimesh / janus-ftl-plugin

A plugin for the Janus WebRTC gateway to enable relaying of audio/video streams utilizing Mixer's FTL (Faster-Than-Light) protocol.
https://hayden.fyi/posts/2020-08-03-Faster-Than-Light-protocol-engineering-notes.html
GNU Affero General Public License v3.0
46 stars 11 forks source link

🧵 Threadless network transports #119

Closed danstiner closed 3 years ago

danstiner commented 3 years ago

This makes the network transport layer very dumb, the media/control connection classes now have threads that monitor and drive the streams.

This has a few benefits:

  1. Easier to test and debug hopefully
  2. Stopping a stream is now non-blocking
    • Stop() methods now ask the jthread to stop, it asynchronously does and then calls the onClose callbacks. This should be more reliable than making the original thread which asked for the stop to do all that work (which had often lead to deadlocks)
  3. Destructors on the transport and connection classes gracefully teardown

I also did a bit of cleanup to use the more modern scoped_lock, etc.

Ready for initial reviews but there's still a couple TODOs and I'd like to do more testing before declaring it ready to merge.

danstiner commented 3 years ago

There's definitely a regression here where when I first connect it fails and if I connect again quickly it works, but if I wait a bit it fails again, unsure why

danstiner commented 3 years ago

The connect fail was a race condition because the jthread member was before the regex members so I think sometimes the CONNECT regex would not be initialized in time and the match would always fail.

There is another issue with the shutdown order, it SEGV's:

Apr 17 23:22:30 ftl-ingest-2 janus[669131]: [2021-04-17 23:22:30.922] [debug] Stopping control connection thread for Channel 1
Apr 17 23:22:30 ftl-ingest-2 janus[669131]: [2021-04-17 23:22:30.922] [debug] FtlServer::onStreamClosed queueing StreamClosedEvent event
Apr 17 23:22:30 ftl-ingest-2 janus[669131]: [2021-04-17 23:22:30.922] [debug] FtlServer::eventStreamClosed processing StreamClosed event...
Apr 17 23:22:31 ftl-ingest-2 janus[669131]: [2021-04-17 23:22:31.107] [debug] Stopping media connection thread for Channel 1 / Stream 610
Apr 17 23:22:31 ftl-ingest-2 janus[669131]: [2021-04-17 23:22:31.107] [error] Media connection closed unexpectedly for channel 1 / stream 610
Apr 17 23:22:31 ftl-ingest-2 systemd[1]: janus.service: Main process exited, code=killed, status=11/SEGV
danstiner commented 3 years ago

Fixed the SEGV, think this might be ready to merge. It's a big enough change might be worth baking for awhile on the test ingest server first though?

danstiner commented 3 years ago

Thanks for working through this big change with me. It works great on my home lab, think it's time to merge!