Skip to content

fix(conc): scope channel hangup/answer callbacks to fix per-call leak (#121) - #122

Merged
ryanmurf merged 1 commit into
masterfrom
hardening/conc-callback-registry-leak
Jul 17, 2026
Merged

fix(conc): scope channel hangup/answer callbacks to fix per-call leak (#121)#122
ryanmurf merged 1 commit into
masterfrom
hardening/conc-callback-registry-leak

Conversation

@ryanmurf

Copy link
Copy Markdown
Owner

Fixes #121.

Problem

HANGUP_CALLBACKS / ANSWER_CALLBACKS in asterisk-core/src/channel/mod.rs
were append-only Vecs. Every inbound INVITE registered two per-call
closures (event_handler.rs:768/:775) keyed to one channel's unique_id
and never removed them; ConfBridge joins (confbridge.rs:1465) and Local
pairs (local.rs:301) added more.

  • Unbounded memory: ~2 leaked closures per call. Over the M5 500-call soak
    (and beyond) this grows without bound → OOM. The M4 exact-baseline harness
    never caught it — it counts the transaction/channel/call-state/NOTIFY maps,
    not these registries.
  • O(n²) fire cost: fire_*_callbacks scans the whole vector on every
    hangup/answer; the Nth event scans ~2N stale closures.

Fix

  • Key both registries by a monotonic id: HashMap<u64, Callback>.
  • Add a scoped RAII API — register_{hangup,answer}_callback_scoped returning a
    #[must_use] handle whose Drop unregisters the entry.
  • Migrate the four per-call sites to the scoped API, holding the handle for the
    call/leg lifetime (moved into the per-call task so it drops at task end).
  • Keep the permanent register_* API for process-lifetime subscribers (CDR).
  • Add registered_{hangup,answer}_callbacks() count accessors so the soak
    harness can assert the registries return to baseline.

Locking is unchanged (fire_* still invokes closures under the registry lock);
documented the pre-existing invariant that a closure must not register/drop a
handle synchronously from inside a callback.

Tests (red-capable, asterisk-core)

  • scoped_{hangup,answer}_callback_unregisters_on_drop — baseline+1 while held,
    back to baseline on drop.
  • scoped_hangup_callback_fires_while_held_and_never_after_drop — functional:
    fires once while held, never after drop (even for a reused unique_id).
  • soak_scoped_callbacks_return_to_baseline — 500 register/drop cycles return
    both registries to baseline (mirrors the M4 exact-baseline discipline).

RED control

Neutralizing both Drop impls (no-op removal — the pre-fix leak) fails all four:

scoped_hangup_callback_unregisters_on_drop:   left: 2  right: 1   "return the registry to baseline (leak)"
scoped_hangup_callback_fires_..._after_drop:  left: 2  right: 1   "dropped callback must not fire again"
soak_scoped_callbacks_return_to_baseline:     left: 502 right: 2  "hangup registry leaked across 500 simulated calls"

Gates

  • cargo test --workspace --exclude pjsip-shim4362 passed, 1 ignored.
  • cargo clippy --workspace --exclude pjsip-shim -- -D warningsclean.
  • Rust 1.97.0. No cargo fmt run.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XH6jhrG4ZCL8xfCktK4Zms

…#121)

The global HANGUP_CALLBACKS / ANSWER_CALLBACKS registries were append-only
Vecs: every inbound INVITE registered two per-call closures keyed to one
channel's unique_id and never removed them, and ConfBridge joins / Local
pairs added more. Over a soak this leaked ~2 closures per call (unbounded
memory) and made fire_*_callbacks scan an ever-growing vector on every
hangup/answer (O(n) per event -> O(n^2) over N calls). Invisible to the M4
exact-baseline harness, which does not count these registries.

Key both registries by a monotonic id (HashMap<u64, Callback>) and add a
scoped RAII API (register_{hangup,answer}_callback_scoped) whose handle's
Drop removes the entry. Migrate the four per-call sites (event_handler x2,
confbridge, local) to hold the handle for the call/leg lifetime. Retain the
permanent register_* API for process-lifetime subscribers (CDR). Add
registered_{hangup,answer}_callbacks() count accessors for soak assertions.

Regression tests (asterisk-core, red-capable): scoped registration returns
the registry to baseline on drop, the callback fires while held but never
after drop, and a 500-call soak returns both registries to baseline. Red
control: neutralizing the Drop impls climbs the registry to baseline+500.

cargo test --workspace --exclude pjsip-shim: 4362 passed.
cargo clippy --workspace --exclude pjsip-shim -- -D warnings: clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XH6jhrG4ZCL8xfCktK4Zms
@ryanmurf
ryanmurf force-pushed the hardening/conc-callback-registry-leak branch from 067d87e to 0ec9379 Compare July 17, 2026 00:49
@ryanmurf
ryanmurf merged commit 5509cc2 into master Jul 17, 2026
3 checks passed
@ryanmurf
ryanmurf deleted the hardening/conc-callback-registry-leak branch July 17, 2026 00:51
ryanmurf added a commit that referenced this pull request Jul 17, 2026
…137)

Adds the M5 tentpole: a 500-call establish+teardown soak that asserts EXACT
baseline restoration (zero drift) across every registry the plan enumerates —
not active_channel_count==0 (M-g).

New CoreStatus fields expose the registries that were not yet observable:
- SIPRtpSessions      — live RTP sessions (bound UDP ports); a process-global
                        gauge incremented on RtpSession bind and decremented on
                        Drop, so it catches a leaked socket that outlives its
                        driver-map entry.
- SIPRegistrarBindings — total contact bindings across all AoRs.
- SIPHangupCallbacks / SIPAnswerCallbacks — the #122 scoped-callback counts.

The harness (tests/freeswitch-pin-gate/run.sh) gains full_snapshot (all 13
registries: 5 core + 4 transaction maps + the 4 above) and a run_m5_soak_case
that runs N (default 500) bridged calls (FS -> rustisk 9500 -> Dial 9501 ->
fast hangup, exercising the inbound INVITE server txn, outbound INVITE client
txn, both BYE classes, both legs' driver/store/RTP/callback registrations),
then asserts full_snapshot returns to the exact pre-soak baseline. Runnable
standalone via FREESWITCH_PIN_GATE_CASE=m5-soak (M5_SOAK_CALLS to scale) and
appended to the full harness run.

#122 verified fully covered: inbound (event_handler), ConfBridge, and Local all
hold scoped handles for the leg lifetime; the soak asserts their counts return
to baseline.

RED control (M-g leak): neutralising release_outbound_media's store::deregister
strands one outbound register-store channel per call; a 10-call soak then drifts
to CoreCurrentCalls=10 and the exact-baseline assertion FAILS — proving both the
per-call outbound-leg exercise and that the soak detects a single-registry leak.

Co-authored-by: rustisk-m5-agent <agent@herodevs.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Channel hangup/answer callback registries leak one closure per call (unbounded memory + O(n²) fire)

1 participant