Remove `Sync` bounds #2

malobre · 2021-07-10T18:31:52Z

Currently all route handlers needs to be Send, Sync and 'static, the default hyper Executor only requires Send and 'static bounds (see this line).

The Sync bound is currently needed because RouterService holds an Arc<Router> which only implements Send if the underlying type is both Send AND Sync.

A workaround would be to wrap the router in a Mutex: Arc<Mutex<Router>>. Since Mutex implements Send and Sync for all types that are Send this should removes the need for all Sync bounds.

However this would prevent the user from wrapping a Router in an Arc themself:

// `router` won't be `Send` because of `Arc` `Sync` requirements
let router = Arc::new(Router::default());

let make_svc = make_service_fn(move |_| {
     let router = router.clone();
     async move {
         Ok::<_, Infallible>(service_fn(move |req: Request<Body>| {
             let router = router.clone();
             async move { router.serve(req).await }
         }))
     }
 });

 let server = Server::bind(&([127, 0, 0, 1], 3000).into())
     .serve(make_svc)
     .await;

An alternative would be to wrap all the inner fields of Router in a struct, wrapped in a Mutex:

struct Router {
    inner: Mutex<RouterInner>
}

struct RouterInner {
    /* -- snip --*/
}

There may be other solutions, but I'm not aware of them ATM.

The text was updated successfully, but these errors were encountered:

ibraheemdev · 2021-07-10T18:44:45Z

Wrapping the router in a mutex would mean requiring mutual exclusion during the creation of a response future, which is not something I want to do. Is there a reason the Sync bound is a problem?

malobre · 2021-07-11T10:31:22Z

In my experience, future are often !Sync.
I use futures::future::BoxFuture quite a lot and it isn't Sync, the same goes for https://github.com/dtolnay/async-trait which expands to !Sync futures.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `Sync` bounds #2

Remove `Sync` bounds #2

malobre commented Jul 10, 2021 •

edited

Loading

ibraheemdev commented Jul 10, 2021

malobre commented Jul 11, 2021

Remove Sync bounds #2

Remove Sync bounds #2

Comments

malobre commented Jul 10, 2021 • edited Loading

ibraheemdev commented Jul 10, 2021

malobre commented Jul 11, 2021

Remove `Sync` bounds #2

Remove `Sync` bounds #2

malobre commented Jul 10, 2021 •

edited

Loading