fix(abi): the panic boundary was documented as universal and wasn't

`abi.rs`'s header states "panics never cross the boundary: every entry point is wrapped in
`catch_unwind`". Of its 78 `extern "C"` entry points, 16 were not. Ten of those are fine and were
always fine — `punktfunk_abi_version` returns a constant, and the nine `punktfunk_connect*` shims
forward every argument unchanged into a guarded implementation — but five ran real code bare:
`session_free`, `connection_close`, `connection_disconnect_quit`, `reanchor_gate_free`,
`reanchor_gate_arm`.

The three `*_free`/`close` ones are the point. They run `Drop` for an entire `Session` or
`Connection` — transports, threads, mutexes — and a `Drop` impl that unwraps a poisoned lock panics.
Since Rust 1.81 that unwind is a hard abort rather than undefined behaviour, so this is not a
soundness hole; it is worse-behaved than it looks. Aborting the CALLER'S process because one of our
teardown paths hit a poisoned mutex is not an acceptable failure mode for a library, and it would
present as "the app died in punktfunk_session_free" with no Rust backtrace to explain it.

Adds `guard_void`, the sibling of `guard` for entry points with no status to report through: it
catches, logs, and returns — right for teardown, where the object is going away regardless. The five
are wrapped in it.

The header now says what is actually true, including WHICH entry points are deliberately bare and
why, so the next reader can check the claim instead of trusting it. That is the same failure this
sweep found in `MappedView`'s `Sync` proof: a stated invariant that a reviewer would rely on and that
had quietly stopped holding.

Verified: Linux .21 fmt + both CI clippy steps rc=0, and the C ABI harness — which exercises the
`*_free`/`close` paths it touches — still passes, 4 frames byte-exact through lossy loopback.
This commit is contained in:
2026-07-29 08:48:42 +02:00
parent aa070f2d55
commit bcfb833ff7
+53 -28
View File
@@ -8,7 +8,11 @@
//! [`punktfunk_session_free`]. A [`PunktfunkFrame`]'s `data` is borrowed until the next //! [`punktfunk_session_free`]. A [`PunktfunkFrame`]'s `data` is borrowed until the next
//! `poll`/`free` on that session — copy it out before then. //! `poll`/`free` on that session — copy it out before then.
//! - Versioned: [`punktfunk_abi_version`] + `PunktfunkConfig::struct_size` for forward-compat. //! - Versioned: [`punktfunk_abi_version`] + `PunktfunkConfig::struct_size` for forward-compat.
//! - Panics never cross the boundary: every entry point is wrapped in `catch_unwind`. //! - Panics never cross the boundary. Status-returning entry points use [`guard`], which reports a
//! panic as `PunktfunkStatus::Panic`; the teardown/mutator ones that return nothing use
//! [`guard_void`], which swallows and logs. The remaining bare ones are bare ON PURPOSE and only
//! because they cannot panic: [`punktfunk_abi_version`] returns a constant, and the
//! `punktfunk_connect*` shims forward every argument unchanged into a guarded implementation.
// THE ABI CONTRACT, stated once - most `// SAFETY:` proofs below are an instance of it. // THE ABI CONTRACT, stated once - most `// SAFETY:` proofs below are an instance of it.
// //
@@ -211,6 +215,17 @@ fn guard<F: FnOnce() -> PunktfunkStatus>(f: F) -> PunktfunkStatus {
std::panic::catch_unwind(AssertUnwindSafe(f)).unwrap_or(PunktfunkStatus::Panic) std::panic::catch_unwind(AssertUnwindSafe(f)).unwrap_or(PunktfunkStatus::Panic)
} }
/// `guard` for the entry points that return nothing — the teardown/mutator calls, which have no
/// status to report a panic through. They still must not let one unwind into C: since Rust 1.81
/// that is a hard abort rather than undefined behaviour, but aborting the CALLER'S process because
/// one of our `Drop` impls hit a poisoned mutex is not an acceptable failure mode for a library.
/// Swallowing is right here — the object is being torn down either way.
fn guard_void<F: FnOnce()>(f: F) {
if std::panic::catch_unwind(AssertUnwindSafe(f)).is_err() {
tracing::error!("panic escaped a punktfunk_* teardown entry point; swallowed at the C ABI");
}
}
fn new_handle(session: Session) -> *mut PunktfunkSession { fn new_handle(session: Session) -> *mut PunktfunkSession {
Box::into_raw(Box::new(PunktfunkSession { Box::into_raw(Box::new(PunktfunkSession {
inner: session, inner: session,
@@ -382,11 +397,13 @@ pub unsafe extern "C" fn punktfunk_test_loopback_pair(
/// `s` must be a handle from `punktfunk_session_new`/`punktfunk_test_loopback_pair`, freed once. /// `s` must be a handle from `punktfunk_session_new`/`punktfunk_test_loopback_pair`, freed once.
#[no_mangle] #[no_mangle]
pub unsafe extern "C" fn punktfunk_session_free(s: *mut PunktfunkSession) { pub unsafe extern "C" fn punktfunk_session_free(s: *mut PunktfunkSession) {
if !s.is_null() { guard_void(|| {
// SAFETY: per the ABI contract - the pointer operands in this block are caller-supplied if !s.is_null() {
// and are null-checked or handle-validated on this path before they are read. // SAFETY: per the ABI contract - the pointer operands in this block are caller-supplied
drop(unsafe { Box::from_raw(s) }); // and are null-checked or handle-validated on this path before they are read.
} drop(unsafe { Box::from_raw(s) });
}
});
} }
/// Host: FEC-protect, packetize, seal and send one encoded access unit. /// Host: FEC-protect, packetize, seal and send one encoded access unit.
@@ -4145,12 +4162,14 @@ pub unsafe extern "C" fn punktfunk_connection_probe_result(
#[cfg(feature = "quic")] #[cfg(feature = "quic")]
#[no_mangle] #[no_mangle]
pub unsafe extern "C" fn punktfunk_connection_disconnect_quit(c: *mut PunktfunkConnection) { pub unsafe extern "C" fn punktfunk_connection_disconnect_quit(c: *mut PunktfunkConnection) {
// SAFETY: per the ABI contract - an opaque handle from a `*_new`/`*_pair` that the caller has guard_void(|| {
// not yet freed, or null, which `as_mut`/`as_ref` reports as `None` and the `match` here // SAFETY: per the ABI contract - an opaque handle from a `*_new`/`*_pair` that the caller has
// handles. // not yet freed, or null, which `as_mut`/`as_ref` reports as `None` and the `match` here
if let Some(c) = unsafe { c.as_ref() } { // handles.
c.inner.disconnect_quit(); if let Some(c) = unsafe { c.as_ref() } {
} c.inner.disconnect_quit();
}
});
} }
/// Close the connection and free the handle (joins the internal threads). NULL is a no-op. /// Close the connection and free the handle (joins the internal threads). NULL is a no-op.
@@ -4160,11 +4179,13 @@ pub unsafe extern "C" fn punktfunk_connection_disconnect_quit(c: *mut PunktfunkC
#[cfg(feature = "quic")] #[cfg(feature = "quic")]
#[no_mangle] #[no_mangle]
pub unsafe extern "C" fn punktfunk_connection_close(c: *mut PunktfunkConnection) { pub unsafe extern "C" fn punktfunk_connection_close(c: *mut PunktfunkConnection) {
if !c.is_null() { guard_void(|| {
// SAFETY: per the ABI contract - the pointer operands in this block are caller-supplied if !c.is_null() {
// and are null-checked or handle-validated on this path before they are read. // SAFETY: per the ABI contract - the pointer operands in this block are caller-supplied
drop(unsafe { Box::from_raw(c) }); // and are null-checked or handle-validated on this path before they are read.
} drop(unsafe { Box::from_raw(c) });
}
});
} }
// ---- Post-loss re-anchor freeze gate ---- // ---- Post-loss re-anchor freeze gate ----
@@ -4193,11 +4214,13 @@ pub extern "C" fn punktfunk_reanchor_gate_new(frames_dropped: u64) -> *mut Reanc
/// `g` was returned by [`punktfunk_reanchor_gate_new`] and is not used after this call. /// `g` was returned by [`punktfunk_reanchor_gate_new`] and is not used after this call.
#[no_mangle] #[no_mangle]
pub unsafe extern "C" fn punktfunk_reanchor_gate_free(g: *mut ReanchorGate) { pub unsafe extern "C" fn punktfunk_reanchor_gate_free(g: *mut ReanchorGate) {
if !g.is_null() { guard_void(|| {
// SAFETY: per the ABI contract - the pointer operands in this block are caller-supplied if !g.is_null() {
// and are null-checked or handle-validated on this path before they are read. // SAFETY: per the ABI contract - the pointer operands in this block are caller-supplied
drop(unsafe { Box::from_raw(g) }); // and are null-checked or handle-validated on this path before they are read.
} drop(unsafe { Box::from_raw(g) });
}
});
} }
/// Arm the freeze: a loss was detected (a frame-index gap, or a decoder wedge/demotion). Zeroes the /// Arm the freeze: a loss was detected (a frame-index gap, or a decoder wedge/demotion). Zeroes the
@@ -4207,12 +4230,14 @@ pub unsafe extern "C" fn punktfunk_reanchor_gate_free(g: *mut ReanchorGate) {
/// `g` is a valid gate handle. /// `g` is a valid gate handle.
#[no_mangle] #[no_mangle]
pub unsafe extern "C" fn punktfunk_reanchor_gate_arm(g: *mut ReanchorGate) { pub unsafe extern "C" fn punktfunk_reanchor_gate_arm(g: *mut ReanchorGate) {
// SAFETY: per the ABI contract - an opaque handle from a `*_new`/`*_pair` that the caller has guard_void(|| {
// not yet freed, or null, which `as_mut`/`as_ref` reports as `None` and the `match` here // SAFETY: per the ABI contract - an opaque handle from a `*_new`/`*_pair` that the caller has
// handles. // not yet freed, or null, which `as_mut`/`as_ref` reports as `None` and the `match` here
if let Some(g) = unsafe { g.as_mut() } { // handles.
g.arm(std::time::Instant::now()); if let Some(g) = unsafe { g.as_mut() } {
} g.arm(std::time::Instant::now());
}
});
} }
/// Fold one decoded frame and write to `out_present` whether to display it (`true`) or withhold it as /// Fold one decoded frame and write to `out_present` whether to display it (`true`) or withhold it as