diff --git a/Cargo.lock b/Cargo.lock index c0b1484e..a1d66308 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1062,6 +1062,7 @@ dependencies = [ "sha2", "thiserror 2.0.20", "time", + "tokio", "tracing", ] diff --git a/crates/didbot-dns/Cargo.toml b/crates/didbot-dns/Cargo.toml index f8702f70..7d8f2689 100644 --- a/crates/didbot-dns/Cargo.toml +++ b/crates/didbot-dns/Cargo.toml @@ -35,5 +35,13 @@ sha2 = { workspace = true, optional = true } hex = { workspace = true, optional = true } time = { workspace = true, optional = true } +[dev-dependencies] +# Only to prove, in `route53.rs`'s own tests, that a caller of `publish`, +# `provision` or `delete` must reach this crate's blocking HTTP client +# through `spawn_blocking` rather than inline on an async task: the panic +# that skipping that seam causes in a debug build needs a runtime to panic +# inside of. +tokio.workspace = true + [lints] workspace = true diff --git a/crates/didbot-dns/src/route53.rs b/crates/didbot-dns/src/route53.rs index bf044817..f04ad3a3 100644 --- a/crates/didbot-dns/src/route53.rs +++ b/crates/didbot-dns/src/route53.rs @@ -3004,4 +3004,50 @@ mod tests { "one read at construction and one refresh shared by every waiter" ); } + + /// Why every caller of `publish`, `provision` or `delete` has to reach + /// this file's blocking client through `tokio::task::spawn_blocking` + /// rather than inline on an async task. + /// + /// `reqwest::blocking::Client::new` builds a shell `current_thread` + /// runtime purely to check it is safe to block, then drops it; dropping + /// a runtime blocks the thread until its worker shuts down, and — in a + /// debug build only, per reqwest's own `wait::enter` — that panics when + /// the thread already belongs to a runtime that forbids blocking, which + /// is exactly what an async task on `#[tokio::main]`'s two-worker + /// runtime is. `spawn_blocking` runs on a thread the runtime itself + /// marks safe to block, so the same call there does not panic. + /// + /// This is not a hazard specific to `didbot-dns`: it is why `routes.rs` + /// crosses `spawn_blocking` for every call into this file's provider, + /// and why `didbot-serve`'s `--demo` startup task had to be fixed the + /// same way rather than calling `Registry::provision`/`delete` inline. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn building_the_blocking_client_inline_on_an_async_task_is_the_hazard_spawn_blocking_avoids( + ) { + let inline = std::panic::catch_unwind(reqwest::blocking::Client::new); + #[cfg(debug_assertions)] + assert!( + inline.is_err(), + "reqwest::blocking::Client::new() no longer panics when built inline on an async \ + task; if a reqwest upgrade fixed this, every caller that crosses spawn_blocking \ + for it may not need to any more" + ); + #[cfg(not(debug_assertions))] + assert!( + inline.is_ok(), + "outside a debug build this should not panic, only silently risk blocking a worker" + ); + + let via_spawn_blocking = tokio::task::spawn_blocking(|| { + std::panic::catch_unwind(reqwest::blocking::Client::new) + }) + .await + .expect("the blocking task itself must not panic"); + assert!( + via_spawn_blocking.is_ok(), + "spawn_blocking should isolate the same construction from the panic the inline \ + case hits in a debug build" + ); + } }