Skip to content

Confirmed INVITE dialogs are never removed from DialogLayer → mem leak #134

Description

@jhiver

Hello there, first of all, thank you for this library.

I noticed a memleak while running benchmarks. Claude report below. I will open a PR later on.

Thank you!
-- JM

Crate: rsipstack Version: 0.5.18 (also present in the current source layout for do_invite / do_invite_async)

Summary

When a UAC establishes an outbound INVITE dialog, the confirmed dialog is inserted
into the global DialogLayerInner.dialogs map and is never removed by the
library
, not even when the dialog reaches the Terminated state (BYE
sent/received, cancel, error). The only removal path is the caller explicitly
calling DialogLayer::remove_dialog. A caller that establishes calls but does not
(or forgets to) call remove_dialog on the confirmed dialog id leaks one Dialog
per answered call, forever.

Each retained Dialog holds the full dialog state (request, 200 OK, negotiated
SDP, route set, tags), on the order of ~20 KB in our measurements. Under call
churn this is an unbounded, monotonic memory leak that eventually OOMs a
long-running process.

Evidence / root cause (file:line, 0.5.18)

The confirmed dialog is inserted on 2xx and never removed:

  • src/dialog/invitation.rs:588do_invite_async: on a 2xx response the client
    dialog is re-inserted under its confirmed id (new_id, with the remote
    to-tag):
    if is_2xx {
        inner.dialogs.insert(new_id.to_string(), Dialog::ClientInvite(dialog_clone.clone()));
    }
  • src/dialog/invitation.rs:530do_invite (sync) has the identical pattern.

The only removal of a confirmed dialog is the public method, called by the
application, never by the library on termination:

  • src/dialog/dialog_layer.rs:510pub fn remove_dialog(&self, id: &DialogId).

The dialog state machine transitions to Terminated
(src/dialog/dialog.rs, DialogState::Terminated(..)) but the transition does
not remove the entry from DialogLayerInner.dialogs. So a terminated,
dead dialog stays in the map indefinitely.

Note the asymmetry that makes this easy to hit: the early (pre-2xx) key is
cleaned up automatically (invitation.rs:575, and on non-2xx the entry is
removed), so failed/rejected calls do not leak — only answered calls do.
The library's own example (src/bin/bench_ua.rs:115,285) has to call
remove_dialog by hand, which confirms the contract is "caller must remove" — but
nothing enforces it and nothing logs when it is missed.

Reproduction

Minimal UAC loop against any UAS that answers and then terminates (or where the
UAC hangs up):

  1. DialogLayer::do_invite (or do_invite_async) to an endpoint that returns
    200 OK.
  2. Let the call terminate normally (BYE either direction).
  3. Do not call remove_dialog.
  4. Observe dialog_layer_inner.dialogs.len() (or expose a counter) after each
    call: it grows by exactly one per answered call and never decreases, even
    after all calls are long finished and the process is idle.

We reproduced this deterministically with a global allocator tracking live
bytes: with the removal missing, live heap grows ~20 KB per answered call and
stays pinned after churn stops (idle, zero active calls) — i.e. a true retention,
not allocator arena reuse. Rejected/unanswered calls do not leak.

Impact

  • Unbounded RSS growth proportional to the number of answered outbound calls
    (independent of concurrency). In our field measurement: ~+600 MB/h at ~13
    answered calls/s, projecting to OOM in roughly a day for a permanently running
    UAC. File descriptors and active-call counts stay flat, which makes it look
    like a "memory-only" leak and easy to misdiagnose as allocator behavior.
  • Because the retained entries are keyed by confirmed dialog id, the map itself
    also grows, slowly degrading dialog lookup for in-dialog requests over time.

Suggested fix (upstream)

The robust fix is to make dialog cleanup automatic rather than caller-driven,
so the API is leak-proof by construction. Options, in order of preference:

  1. Self-removal on terminal state. When a dialog transitions to
    DialogState::Terminated, have it remove itself from
    DialogLayerInner.dialogs. The ClientInviteDialog already holds a reference
    to the layer inner (it uses self.dialog_layer_inner.dialogs.remove(..) for
    the early key at invitation.rs:161/182/518), so the confirmed entry can be
    removed at the same place the Terminated transition is emitted. Mirror the
    same for the server side.
  2. If self-removal is undesirable (callers may still want to inspect a terminated
    dialog), provide a reaper / TTL on terminated dialogs, or hold entries as
    Weak so a dropped dialog disappears from the map.
  3. At minimum, document loudly on do_invite/do_invite_async that the caller
    MUST call remove_dialog on the confirmed id after termination, and consider a
    debug-level warning when the layer is dropped with non-empty dialogs.

Option 1 is what we'd prefer as downstream users: it removes an entire class of
"who owns cleanup" bugs.

Workaround (downstream, what we did)

We wrap the outbound driver in an RAII guard whose Drop calls
remove_dialog(&dialog.id()) (the confirmed id) on every exit path of the call
driver, mirroring what our inbound path already did on termination. This works but
is exactly the kind of caller-side bookkeeping a leak-proof library API should not
require.


Filed by a downstream user of rsipstack (a Rust SIP voice daemon). Happy to
provide a minimal reproducing example or test if useful.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions