changed lifetimes

[pythoneer]

First of all nice work and thanks for the quick release of version 0.5!

I am currently porting an actix-web(v0.7) application that uses a2(v0.4) to the new versions with std::futures(async/await) "support". Currently i am having some lifetime issues but i am not really sure why. Maybe its just me not seeing the obvious but i have the feeling that some lifetimes changed significantly from 0.4 -> 0.5

I have made some minimal examples.

old one (a2 0.4, futures 0.1, actix-web 0.7) this one compiles Cargo.toml

#...
[dependencies]
actix-web = "0.7.19"
a2 = "0.4.0"
futures = "0.1.25"

main.rs

use actix_web::{client, http, server, App, Json, HttpRequest};
use futures::future::Future;

fn push_apns(_req: HttpRequest) -> impl actix_web::Responder {
    use a2::{Client, Endpoint, NotificationOptions, LocalizedNotificationBuilder, NotificationBuilder};

    let title = String::from("title1");
    let message = String::from("message1");
    let token = String::from("token1");
    let bundle_identifier = String::from("bundle1");

    let client = Client::token(
        String::from("certificate").as_bytes(),
        String::from("keyid"),
        String::from("teamid"),
        Endpoint::Sandbox,
    ).unwrap();

    let topic: Option<String> = Some(bundle_identifier);
    let options = NotificationOptions {
        apns_topic: topic.as_ref().map(|s| &**s),
        ..Default::default()
    };

    let mut builder = LocalizedNotificationBuilder::new(&title, &message);
    builder.set_sound("default");
    builder.set_badge(1u32);

    let payload = builder.build(&token, options.clone());
    let sending = client.send(payload);
    let sending = sending.map_err(|_| ()).map(|_| ());

    actix_web::actix::Arbiter::spawn(sending);

    "thanks for using our service"
}

fn main() {
    let server = server::new(|| {
        App::new()
            .resource("/push_apns", |r| {
                r.method(http::Method::POST).with(push_apns)
            })
    });
    server.bind("0.0.0.0:8080").unwrap().run();
}

new one (a2 0.5, futures 0.3, actix-web 2.0-alpha) this one does not compile Cargo.toml

#...
[dependencies]
actix-web = "2.0.0-alpha.4"
actix-rt = "1.0.0"
futures = "0.3.1"
a2 = "0.5.0"

main.rs

use actix_web::{web::scope, middleware, web, App, HttpRequest, HttpServer, HttpResponse};
use futures::future::FutureExt;

pub async fn push_apns(_req: HttpRequest) -> HttpResponse {
    use a2::{NotificationOptions, LocalizedNotificationBuilder, Client, Endpoint, NotificationBuilder};

    let title = String::from("title1");
    let message = String::from("message1");
    let token = String::from("token1");
    let bundle_identifier = String::from("bundle1");

    let client = Client::token(
        String::from("certificate").as_bytes(),
        String::from("keyid"),
        String::from("teamid"),
        Endpoint::Sandbox,
    ).unwrap();

    let topic: Option<String> = Some(bundle_identifier);
    let options = NotificationOptions {
        apns_topic: topic.as_ref().map(|s| &**s),
        ..Default::default()
    };

    let mut builder = LocalizedNotificationBuilder::new(&title, &message);
    builder.set_sound("default");
    builder.set_badge(1u32);

    let payload = builder.build(&token, options.clone());
    let sending = client.send(payload);
    let sending = sending.map(|_| ());

    actix_rt::Arbiter::spawn(sending);

    HttpResponse::from("thanks for using our service")
}

#[actix_rt::main]
async fn main() -> std::io::Result<()> {
    HttpServer::new(|| {
        App::new()
            .service(scope("/push")
                .route("apns", web::post().to(push_apns))
            )
    })
        .bind("127.0.0.1:8080")?
        .start()
        .await
}

having this compile errors:

error[E0597]: `topic` does not live long enough
  --> src/main.rs:21:21
   |
21 |         apns_topic: topic.as_ref().map(|s| &**s),
   |                     ^^^^^                  ---- returning this value requires that `topic` is borrowed for `'static`
   |                     |
   |                     borrowed value does not live long enough
...
36 | }
   | - `topic` dropped here while still borrowed

error[E0597]: `title` does not live long enough
  --> src/main.rs:25:57
   |
25 |     let mut builder = LocalizedNotificationBuilder::new(&title, &message);
   |                       ----------------------------------^^^^^^-----------
   |                       |                                 |
   |                       |                                 borrowed value does not live long enough
   |                       argument requires that `title` is borrowed for `'static`
...
36 | }
   | - `title` dropped here while still borrowed

error[E0597]: `message` does not live long enough
  --> src/main.rs:25:65
   |
25 |     let mut builder = LocalizedNotificationBuilder::new(&title, &message);
   |                       ------------------------------------------^^^^^^^^-
   |                       |                                         |
   |                       |                                         borrowed value does not live long enough
   |                       argument requires that `message` is borrowed for `'static`
...
36 | }
   | - `message` dropped here while still borrowed

error[E0597]: `token` does not live long enough
  --> src/main.rs:29:33
   |
29 |     let payload = builder.build(&token, options.clone());
   |                   --------------^^^^^^------------------
   |                   |             |
   |                   |             borrowed value does not live long enough
   |                   argument requires that `token` is borrowed for `'static`
...
36 | }
   | - `token` dropped here while still borrowed

error[E0597]: `client` does not live long enough
  --> src/main.rs:30:19
   |
30 |     let sending = client.send(payload);
   |                   ^^^^^^--------------
   |                   |
   |                   borrowed value does not live long enough
   |                   argument requires that `client` is borrowed for `'static`
...
36 | }
   | - `client` dropped here while still borrowed

Unfortunately i am not very familiar with the new futures 0.3 or maybe missing something obvious here. But from what i understand is that in version 0.4 the public send function and e.g. the private build_request have a lifetime 'a from the Payload<'a> and thus to the &title, &message and &token stored in that Payload<'a> that is NOT bound to the return value FutureResponse because it looks like you are copying the values before "moving" them into futures like

        let path = format!(
            "https://{}/3/device/{}",
            self.endpoint, payload.device_token
        );

and thus "guarding" FutureResponse from being bound to e.g. payload.device_token

I think this is not the case in version 0.5 Because send is anotated with async the return type Result<Response, Error> is auto wrapped with (i think) impl Future<Output = Result<Response, Error>> + 'a binding the future onto the lifetime of its input parameters that result in my compilation errors? Because the hole function is now a Future and the copying

        let path = format!(
            "https://{}/3/device/{}",
            self.endpoint, payload.device_token
        );

is just to late because we are already inside the future before copying?

EDIT I think this is roughly what happened

use std::future::Future;

// this is the "old" way writing this
fn lib_fn_old(data: &str) -> impl Future<Output = String> {
    let new_data = format!("new {}", data);
    futures::future::ready(new_data)
}

// this is the new way with async
async fn lib_fn_new(data: &str) -> String {
    format!("new {}", data)
}

// but it "silently" changes the lifetime bounds of the return value, binding
// it to the lifetime of the input!
fn lib_fn_new_desugar<'a>(data: &'a str) -> impl Future<Output = String> + 'a {
    async move {
        let new_data = format!("new {}", data);
        new_data
    }
}

// this would be the desugared function that have the same lifetime bounds 
fn lib_fn_new_same_lifetime(data: &str) -> impl Future<Output = String> {
    let new_data = format!("new {}", data);
    async move {
        new_data
    }
}

fn create_future() -> impl Future<Output = String> {
    let input_data = String::from("data");

    lib_fn_old(&input_data) // could do this with the old one

    // cannot do this with the new one
    // lib_fn_new(&input_data)
    // lib_fn_new_desugar(&input_data)
}

fn main() {
    let _future = create_future();
}

EDIT2 This is explicitly mentioned in the RFC 2394. My lib_fn_new_same_lifetime is mentioned as "Initialization pattern"

[pythoneer]

I tried to make a PR but i failed in my first attempt to entangle the lifetimes with the mentioned "Initialization pattern". Its a little bit more involved than i though originally because the signer.with_signature is also async and i don't really know how to pass that into the future without having a reference to self. I think &self on send is even a bigger problem because an immediate solution would be to move title etc. right into Payload – it think that's the way web-push is doing it. But i think the same also applies to web-push and fcm regarding &self because .awaiting on &self fields make it Pin? idk.

[pimeys]

Hey. I just saw this but already in bed. I'll check your code when I have my coffee tomorrow!

[pimeys]

Hey @pythoneer please try the branch in that pull request. You can read the code how I just go with a blocking RwLock for now, so I don't need to await it and can move the request building outside of the sending future. Now whatever gets captured in the sending future has a static lifetime and can be used together with spawn.

I don't think that blocking the thread with the signer lock will cause any trouble, but also I'm not working in a company that needs to send millions of notifications every minute anymore, so I can't really test...

It should be fine though, please give it a try.

[pimeys]

(btw. the coffee is exceptionally good this morning)

[dbrgn]

(btw. the coffee is exceptionally good this morning)

Good coffee, happy life! :coffee:

[pythoneer]

Thanks @pimeys looks like it is working. Have tried it also on an actual device. I expect the same thing to be the case on fcm (maybe also web-push), should i open a separate issue? I am just not there in the process of porting these too, currently to say for sure.

[pimeys]

It should be quite easy to fix them too. Why these problems happened originally is just me not being able to remember people might want to spawn these futures, due to me just running a consumer and spawning kafka messages instead of push notifications.

I'll fix fcm/web-push today.

[pimeys]

0.5.1 is out.