Glimesh / waveguide

A polyglot WebRTC media server.
GNU Affero General Public License v3.0
37 stars 8 forks source link

Figure out the right way to cancel in progress work #16

Open clone1018 opened 1 year ago

clone1018 commented 1 year ago

A live stream can stop (for whatever reason) either from the Protocol / Input layer, or from the Control layer. This bi-directional cancellation is currently a mess, and there's some hacky code in place to make it work.

Reasons for Cancellation

Options

It seems like context may be appropriate for this, however I don't have a ton of experience using it.

Alternatively, depending on the Control architecture, we could use callbacks -- though that's not very "go" like.

nassah221 commented 1 year ago

I have been reviewing the code around the current implementation. While it works, I think doing bi-directional cancellation is better done through the use of channels as well as context.

Generally, context is meant to be passed from a parent to derived tasks or subtasks. The parent is then responsible for cancelling any subtasks it may have created. Storing context in a struct is also frowned upon.

For clarity, here's what I have in mind.

  1. When the cancellation is directed from the control to the session stream, there should be a parent context for OS signals which is propagated from main.go to the code responsible for handling new streams. This context is cancelled when server is killed/restarted.

  2. When the cancellation is directed from the session stream to control due to bad io etc. the use of channels would be more appropriate. AFAIK, the stream type is responsible for cleanup/removal itself. Instead of propagating context from a child/spawned task to a parent task, we could use a oneshot channel to signal cleanup/cancellation to control.

WDYT @clone1018

clone1018 commented 1 year ago

I like the idea of using a simpler setup with channels and contexts. I implemented an experimental version of this in https://github.com/Glimesh/waveguide/commit/3004b21aff00b28a27ba5d24a451bba25527e0cb however they are stored in a struct. It didn't seem clear to me how to keep access to them over time and handle cancellation inside the inputs. Looking forward to seeing your improved implementation!