webrtc-rs / webrtc

A pure Rust implementation of WebRTC
https://webrtc.rs
Apache License 2.0
4.06k stars 362 forks source link

[All] change callback fn from Box<dyn ...> to Arc<dyn ..> #121

Open rainliu opened 2 years ago

rainliu commented 2 years ago

example:

pub type FnTimeGen = Arc<dyn (Fn() -> Pin<Box<dyn Future + Send + 'static>>) + Send + Sync>;

The purpose is to avoid potential deadlock by allowing callback fn cloneable and using tokio::spawn to call it without locking

k0nserv commented 2 years ago

It would be nicer for the external interfaces if the signature of functions that specify callbacks didn't require specifying the smart pointer type e.g.

instead of

pub type FnTimeGen = Arc<dyn (Fn() -> Pin<Box<dyn Future<Output = SystemTime> + Send + 'static>>) + Send + Sync>;

fn set_callback(&self, fn: FnTimeGen) 

use

fn set_callback<F>(&self, fn: F)
where F: (Fn() -> Pin<Box<dyn Future<Output = SystemTime> + Send + 'static>>) + Send + Sync

This way it's simpler for the caller and a change like this doesn't change the public interface.

MarinPostma commented 2 years ago

I think one further step from what @k0nserv suggests would be to have an interface like so:

trait TimeGen {
type Future: Future<Output = SystemTime> + Unpin; // other bounds may be necessary

fn call() -> Self::Future;

I think that such a trait is object safe, and can be stored in the same way it is done now.

There are 3 major advantages to this:

Pascualex commented 2 years ago

I would also appreciate a more ergonomic API.

jeremy-prater commented 2 years ago

Yes, I'm having some major trouble calling async functions from inside the Boxed closure... I either get 'static lifetime issues, or cycle detected issues.

k0nserv commented 2 years ago

@llacqie is working on an attempt to improve this in several PRs:

See this branch for the full code so far