[srperens]

Refs #963

Problem

#986 waits for the source (pad, ERROR or EOS) before each control-call state change. Even so, a_storm_of_jumps_and_pauses_never_hangs_the_player failed on macOS CI run 37315207644 ("did not return after 1367 of 3000 calls").

I reproduced it on an M-series Mac (GStreamer 1.28.6). It happened in 5 of 90 runs with three storm loops in parallel and 8 yes CPU hogs, plus 1 of 20 in an earlier loaded run. I saw no hangs unloaded (0 of 20). Six sample captures of the hung process all show the same pair:

• control thread: goto → load_current_file_inner → start → set_state(PAUSED) → gst_uri_decode_bin3_change_state → gst_uri_source_bin_change_state → gst_bin_change_state_func → gst_parse_bin_change_state → gst_type_find_element_change_state → … activate_sink (pull) → gst_type_find_element_activate_src_mode → gst_pad_pause_task (blocked on the urisourcebin typefind's stream lock)
• typefindelement:sink task: gst_type_find_element_loop → emit_have_type → type_found → setup_parsebin_for_slot → gst_element_sync_state_with_parent (blocked on parsebin's state lock)

This is the #963 lock pair, but in a window that settle() cannot cover. Both sides belong to one READY→PAUSED set_state. In setup_source(), urisourcebin creates the typefind and starts it with sync_state_with_parent during that same state change. If typefind finds the type before the change has finished walking urisourcebin's children, it adds a parsebin on its own thread. The bin iterator resyncs and picks that parsebin up. Both threads then try to activate it. (setup_parsebin_for_slot holds the parsebin in locked state while it adds it. It unlocks it just before sync_state_with_parent, and that unlock opens the window.)

Change

MediaPlayerState::hold_typefind_while_starting() connects to deep-element-added on the source (uridecodebin3, or urisourcebin in passthrough). It hooks have-type on each typefind as that typefind is added. That happens inside gst_bin_add, before urisourcebin connects its own type_found, so our handler runs first. The handler waits until the state change that takes the pipeline up from READY has returned. start() and pause_locked() now make that change through up_from_ready(), which marks it as under way on its thread.

The wait has two safeguards:

• It never waits on the thread that runs the state change.
• It gives up after SETTLE_TIMEOUT (5 s) with a warn!. Without the timeout, a state change that tears a failing source down on the spot (and joins its task) would deadlock the other way.

Rejected: a BLOCK pad probe on the new source's src pad, installed from source-setup. I tried it first and it did not stop the hang (3 of 90). In pull mode, typefind calls the source's getrange function directly, so it emits have-type before any probe sees a pull. A Python check confirmed the order (have-type fires before the first probe call).

Evidence

• Typefind is held while a start from READY is under way: bridge::tests::the_source_waits_for_a_state_change_from_ready_to_return. It marks a change as under way on another thread, sets the decode and passthrough pipelines to PAUSED, and checks that no parsebin appears until the mark is cleared. With the two hold_typefind_while_starting calls commented out it fails: passthrough=false: typefind set up a parsebin while a state change from READY was still under way. With them in, it passes.
• The storm hang is gone: under the same parallel and loaded setup as above, 0 of 180 runs hung (two batches of 90), against 5 of 90 before. The storm test only fails probabilistically: 0 of 20 unloaded runs failed before the fix. It is a soak check, not a deterministic guard. The unit test above is the deterministic guard of the hold itself. It does not guard that start()/pause_locked() go through up_from_ready(); only the storm test covers that.

Tests

Ran on macOS (arm64), GStreamer 1.28.6:

• cargo fmt, cargo clippy --all-targets: clean
• cargo test --lib mediaplayer: 33 passed
• cargo test --test media_player_control_test: 5 runs, all passed; plus the 180-run soak above (local STROM_STORM_HOLD instrumentation, not committed)
• STROM_REQUIRE_GST_PLUGINS=1 cargo test --test failed_start_teardown_test: 3 passed
• STROM_REQUIRE_GST_PLUGINS=1 cargo test --test pipeline_lifecycle_test: 4 passed

Not run: Linux and Windows (left to CI).

For the reviewer

• The have-type handler blocks a typefind streaming thread for as long as one READY→PAUSED set_state takes (milliseconds). That thread holds only its own stream lock, and the state change waits on nothing from it unless a source fails mid-change, which is what the 5 s bound is for.
• It is connected to every typefind in the source, including parsebin's own. Those are created only after the hold is released, so the hold does not touch them in practice.
• bridge.rs is now about 1720 lines, over the 1500-line guideline. Its tests module is most of that and would be the natural split.

Out of scope

• An FD leak seen while soaking. With STROM_STORM_CALLS=10000, the test process grows by about 170 pipe FDs a second (GstPoll/GWakeup pipes, so bins are not being finalized on each file switch). It aborts with GLib-ERROR: Creating pipes for GWakeup: Too many open files after about 40 s. That is a separate leak on file switch. The 3000-call default stays under the limit. The leak is not fixed here and should get its own issue.

🤖 Generated with Claude Code

[srperens]

Verdict: Approve. The hold sits at the right layer: it acts before urisourcebin's own type_found, so no parsebin exists for the state change to pick up. It covers both paths that take the internal pipeline up from READY. Its one deadlock risk is bounded at 5 s and logged.

Claims

Claim	Verdict	Evidence
The hold is installed on both pipeline shapes	CONFIRMED	backend/src/blocks/builtin/mediaplayer/bridge.rs:92 — MediaPlayerState::hold_typefind_while_starting(&source, state); covers decode, and backend/src/blocks/builtin/mediaplayer/bridge.rs:155 — MediaPlayerState::hold_typefind_while_starting(&source, state); covers passthrough.
Every change up from READY goes through up_from_ready	CONFIRMED	Outside tests, the internal pipeline is raised only in start() and pause_locked(). Both take the stopped branch through backend/src/blocks/builtin/mediaplayer/state.rs:529 — self.up_from_ready(pipeline, gst::State::Paused) and backend/src/blocks/builtin/mediaplayer/state.rs:601 — self.up_from_ready(pipeline, gst::State::Paused). The remaining set_state(Paused/Playing) calls run on a pipeline that is already PAUSED, which creates no typefind.
No strong reference in the signal closures	CONFIRMED	backend/src/blocks/builtin/mediaplayer/state.rs:276 — let state = Arc::downgrade(state);. Both the deep-element-added and have-type closures capture only that Weak, and never an element.
The new test guards the hold, and ran	CONFIRMED	In Check (Linux) job 111856804722, bridge::tests::the_source_waits_for_a_state_change_from_ready_to_return ... ok. It asserts that no parsebin appears while starting is set, so removing the hold fails it. a_storm_of_jumps_and_pauses_never_hangs_the_player ... ok also ran.
The blocked have-type cannot stall the state change before parsebin exists	EXTERNAL	This assumes urisourcebin's READY→PAUSED walk needs nothing that the typefind streaming thread holds until a parsebin has been added. That is the trace in the body. Where the assumption fails (a source torn down mid-change), wait_until_started gives up after SETTLE_TIMEOUT and logs a warn!.

Diagnosis. The two-thread trace in the body explains why settle() (#986) could not cover this case: the race is with the very set_state that started typefind. Coverage is BOUNDED. The fix does not cover a state change that runs over 5 s, and the soak result (0/180) is probabilistic. The unit test guards the hold but not the up_from_ready routing. The body says so. On the healthy path, the cost is one uncontended mutex in each have-type (once per typefind), and nothing per buffer.

Radius. LOCAL. Only Media Player internal pipelines are affected, in decode and passthrough. The hook also fires for parsebin's own typefind, but that only exists after the hold is released.

Overlaps. #963 is closed. #755 also edits mediaplayer/state.rs, and a trial merge conflicts with this PR there. #755 already conflicts with main, though, so it has to rebase whichever lands first. No open issue owns this design.

Tests & CI. At 4788826: Check (Linux), both Linux builds, WASM, API Contract Check and Static JS Tests pass. macOS and Windows were skipped. The original failure was on macOS CI, but nothing here is platform-specific, so merging on Linux green and letting main's macOS run confirm it is reasonable. The branch merges cleanly with main. Nothing was built or run here.

Notes, not blocking

• bridge.rs is now 1720 lines. The body already suggests splitting off its tests module.
• The FD leak on file switch (about 170 pipes/s under STROM_STORM_CALLS=10000) has no issue yet. It is worth filing.

Confidence: MED