fix(whip): give a POST's session back on every error path

[srperens]

No issue filed; found while reviewing the WHIP POST error paths.

Problem

Some ways out of whip_post left a session behind, half torn down:

• The internal answer did not name the session. This covers three cases: no Location header, a Location in an unexpected format, and a Location that is not valid UTF-8. In each, the POST dropped the session pipeline while it was PLAYING, which leaks whipserversrc's sockets and threads. It also kept the slot held under the POST's temporary id, which nothing ever releases. The non-UTF-8 case did not even stop the session's watchdog.
• create_whipserversrc_for_session failed to reach PLAYING. Both of its error returns after set_state(Playing) dropped a pipeline that had already left NULL.
• The client went away mid-POST (found in review). axum then drops the handler future, so none of the error branches run. The proxy's retries alone can take close to a minute. The PLAYING pipeline was dropped, the watchdog kept running, and the slot stayed held under the temporary id for good.

Change

• The session is held as a PendingSession until it is registered. Dropping one gives the session back:

  • release the slot under the temporary id;
  • set cleanup_sent, which stops the watchdog;
  • take the pipeline to NULL with teardown_session_pipeline on a blocking thread, keeping the element alive until then;
  • prune the stats store's webrtcbin entries, as DELETE and the cleanup loop do.

  Every error branch, and a cancelled handler, ends there. Only registration takes the session out (into_parts).

• register_from_location parses the Location header. It either registers the session or gives it back.

• The session is built on a blocking task and handed over through a oneshot. A POST cancelled while the session is being built has nobody to receive it, so the task gives the session back itself, or releases the slot if the build failed.

• create_whipserversrc_for_session tears down before returning. It takes its pipeline down to NULL before it returns either PLAYING error.

• Behaviour change: when the internal server's answer does not name the session, the client gets 502 ("Internal WHIP server did not name the session"). Before, it got the SDP answer for a session that had just been dropped, so it negotiated against nothing. Error answers (4xx/5xx) from the internal server are still forwarded unchanged.

Evidence

• An answer without a Location gives the session back: an_answer_without_a_location_gives_the_session_up. The test checks three things: the slot is released, cleanup_sent is set, and the pipeline reaches NULL.
• An answer with an unexpected Location gives the session back: an_answer_with_an_unexpected_location_gives_the_session_up.
• An answer with a non-UTF-8 Location gives the session back: an_answer_with_a_non_utf8_location_gives_the_session_up.
• A dropped (cancelled) PendingSession gives the session back: a_dropped_pending_session_gives_the_session_up.
• The same holds when it is dropped outside a tokio runtime: a_pending_session_dropped_outside_a_runtime_gives_the_session_up.
• Revert check: with the give-back in Drop disabled, all five of the tests above fail.
• The counterpart: an_answer_with_a_location_registers_the_session checks that the session is registered, the slot is moved to its resource_id, the watchdog is still running and the pipeline is still PLAYING.
• Not guarded: the teardown in create_whipserversrc_for_session on a PLAYING failure has no test. Making whipserversrc fail its state change on demand needs ICE elements and a forced failure. Reverting those two lines leaves every test green.
• Not guarded: the oneshot hand-off for a POST cancelled during the build has no test. It would need the handler driven with a cancelled future while the blocking build is in flight.

Tests

Ran on macOS (arm64), GStreamer from Homebrew, after rebasing on main (#983 changed SessionActivity to per-medium stamps; the test fixture now builds a SlotOutput):

• cargo fmt --all -- --check, cargo clippy --workspace --all-targets --features efp,nvidia -- -D warnings: clean (pre-commit hook).
• cargo test --lib -- whip_ingest: 6 passed. Revert check as above.
• Linux CI failure fixed: an_answer_with_a_location_registers_the_session read current_state() before the fixture's live pipeline finished its async change to PLAYING (left: Paused, right: Playing). It now waits with pipeline.state(5 s). The api::whip_ingest::tests module passed 50 of 50 runs locally (no extra CPU load).
• cargo test --lib -- whip state::: 87 passed.
• STROM_REQUIRE_GST_PLUGINS=1 cargo test --test <t> for every backend/tests/whip_*.rs (idle_slot 3, session_branch 2, slot_caps_reuse 3, slot_decoder_reuse 2, video_damage 3, whip_whep_ice_transport_policy 9) and pipeline_lifecycle_test (4): all passed.

For the reviewer

• 502 instead of forwarding the answer. This is the only client-visible change.
• Independent of the other WHIP PRs. Rebased on main, which now has fix(whip): judge a seat's audio and video separately #983 (judge a seat's audio and video separately). It is independent of the slot flush PR, with test(whip): make the per-poll watchdog guard independent of scheduler latency #975 (per-poll watchdog guard) and with fix(whip): link a session pad only after its branch is PLAYING #976 (link a session pad only after its branch is PLAYING).

Out of scope

Raised in review and left alone:

• The slot is released before the session pipeline reaches NULL. Its teardown runs on a blocking thread afterwards, so a new POST can claim the slot while the old pipeline is still bridging into it. DELETE and the cleanup loop do the same. The slot flush PR makes the flush cover what is queued at release, but not a sample pushed after it.
• A panic in create_whipserversrc_for_session after PLAYING still drops the pipeline during unwinding.
• When rename_slot_holder fails (the slot changed hands mid-POST), the session is still registered, as before.
• Strong references in the whipserversrc connect_pad_added closure. It captures strong references to the main pipeline's slot appsrcs. That is not a cycle, but it is against CLAUDE.md's rule. It predates this PR.
• Four give-back sequences. The same sequence exists in PendingSession, register_session's refusal, the cleanup loop and DELETE. They could share one helper.

🤖 Generated with Claude Code

[srperens]

Verdict: Request changes — the mechanism is right and closes every exit from whip_post I could find, but the PR's own new counterpart test fails on Check (Linux), and that red stopped every integration binary (all whip_* tests and pipeline_lifecycle_test) from running.

Posted as a COMMENT review: this PR is from the account this stage runs under, and GitHub refuses a change request on it. The verdict above stands.

Requested changes

1. In pending_post() (backend/src/api/whip_ingest.rs), wait for the PLAYING transition to complete after set_state, e.g. let _ = pipeline.state(gst::ClockTime::from_seconds(5));. A live fakesrc pipeline can still be PAUSED when set_state returns. That is the failure CI shows, and it also means the five give-back tests may be abandoning a pipeline that never reached PLAYING.

Claims

Claim	Verdict	Evidence
Counterpart test passes	CONTRADICTED	Check (Linux) job 111741789809: an_answer_with_a_location_registers_the_session ... FAILED, left: Paused right: Playing at backend/src/api/whip_ingest.rs:1094 — assert_eq!(pipeline.current_state(), gst::State::Playing);. The state is set without a wait at backend/src/api/whip_ingest.rs:961 — pipeline.set_state(gst::State::Playing).unwrap();
Every exit from the handler gives the session back, including cancellation	CONFIRMED	the give-back lives in backend/src/api/whip_ingest.rs:446 — fn drop(&mut self) {; every former inline teardown is now pending.abandon(), and only into_parts skips it
A build cancelled mid-flight is given back by the blocking task	CONFIRMED	send failing returns the session to the task, which wraps and drops it; a failed build releases the slot at backend/src/api/whip_ingest.rs:186 — Err(Err(_)) => { (the build already tore its own pipeline down, see next row)
PLAYING failures in the builder go to NULL first	CONFIRMED	backend/src/blocks/builtin/whip.rs:1481 — WhipSessionManager::teardown_session_pipeline(&session_pipeline); and the same at line 1490. Unguarded, as the body says

Diagnosis — Right layer. A drop guard is the only construct that covers axum dropping the handler future, so the coverage is ABSOLUTE for non-panicking exits. The disclosed panic-unwind gap is the remaining bound. Teardown on drop goes to spawn_blocking, or to a fresh thread outside a runtime (backend/src/api/whip_ingest.rs:478 — std::thread::spawn(teardown);). That is a one-shot per abandoned POST, so there is no loop to cost.

Radius — SHARED: every WHIP Input block's POST path, plus the session builder in whip.rs. The one client-visible change is a 2xx answer with no usable Location, which now returns 502 instead of an SDP answer for a session that has already been dropped. I agree with it: the old answer negotiated against nothing.

Overlaps — no open issue owns this. Two open PRs touch the same files: #897 edits the same use block and removes a hunk in whip_post, so expect a small textual conflict whichever lands second. Draft #854 splits whip.rs and whip_session_manager.rs, so it will need a rebase. #885 is the WHEP player and is unrelated.

Tests & CI — Green at ca37327: Build (Linux x86_64/ARM64), Check & Build (WASM), API Contract Check. Red: Check (Linux), with lib 864 passed; 1 failed. Because cargo test stops at the first failing binary, no integration test ran. macOS/Windows were skipped, which is fine here: nothing in the diff is platform-specific. The author's macOS run is EXTERNAL, and the body itself says the integration runs were still in progress. The two unguarded paths are disclosed with reasons, which meets the Tests rule.

Confidence: HIGH