From a2d4dd3bf4e14d85d7364b7f29cb3bd4fa389df7 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Wed, 9 Sep 2026 08:52:19 -0400 Subject: [PATCH] test(dns): pin down why publish's callers must cross spawn_blocking reqwest::blocking::Client::new builds a shell tokio runtime to check it is safe to block, then drops it; in a debug build only, dropping that runtime panics when the calling thread already belongs to one that forbids blocking, which is exactly what an async task on the server's own runtime is. spawn_blocking runs on a thread the runtime marks safe to block, so the same construction there does not panic. A new test in route53.rs proves both halves, so a reqwest upgrade that changes this behavior does not go unnoticed. Change-Id: Icc65197b47856946da80d4f8af86d0792097a5fb --- Cargo.lock | 1 + crates/didbot-dns/Cargo.toml | 8 ++++++ crates/didbot-dns/src/route53.rs | 46 ++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+) 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" + ); + } } -- 2.51.2