dylib routing issue caused by RouteId

[Curid]

• I have looked for existing issues (including closed) about this

Bug Report

Version

├── axum v0.6.9
│ ├── axum-core v0.3.2

Platform

Linux

Description

Adding routes to a router from a shared library causes routes to point to the wrong handlers. I'm guessing this is the issue.

Route /a should respond with A instead of C

minimal example repo

main.rs

#[tokio::main]
async fn main() {
    let router = Router::new()
        .route("/a", get(|| async { "A" }))
        .route("/b", get(|| async { "B" }));

    let dylib = unsafe { Library::new("./target/debug/libdylib.so").expect("load library") };
    let add_route: Symbol<fn(router: Router) -> Router> =
        unsafe { dylib.get(b"add_route") }.expect("load symbol");

    let router = add_route(router);

    axum::Server::bind(&"0.0.0.0:3000".parse().unwrap())
        .serve(router.into_make_service())
        .await
        .unwrap();
}

dylib.rs

#[no_mangle]
pub fn add_route(router: Router) -> Router {
    router.route("/c", get(|| async { "C" }))
}

[davidpdrsn]

I haven't really worked much with dynamic libraries but I suppose the solution would be to use a random number for the ids instead of incrementing an atomic counter. Wdyt?

[jplatte]

I would worry about introducing that kind of non-determinism. It could still lead to duplicate usage of an ID, even if very unlikely. Couldn't we just have a regular u64 inside Router that is used for the RouteId and incremented in the two places we currently use RouteId::next()?

[davidpdrsn]

I used to think that wouldn't work for cases like

let one = Router::new().route("/one", ...);
let two = Router::new().route("/two", ...);

one.merge(two)

but since merge just calls route which creates a new id I actually think it should work!

[Curid]

Thanks for the quick fix! I wasn't sure if this even was a supported use case, since core Tokio has a few similar issues that can't really be fixed without breaking changes.

tokio-rs/tokio#4940