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:588 — do_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:530 — do_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:510 — pub 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):
DialogLayer::do_invite (or do_invite_async) to an endpoint that returns
200 OK.
- Let the call terminate normally (BYE either direction).
- Do not call
remove_dialog.
- 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:
- 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.
- 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.
- 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.
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:
rsipstackVersion: 0.5.18 (also present in the current source layout fordo_invite/do_invite_async)Summary
When a UAC establishes an outbound INVITE dialog, the confirmed dialog is inserted
into the global
DialogLayerInner.dialogsmap and is never removed by thelibrary, not even when the dialog reaches the
Terminatedstate (BYEsent/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_dialogon the confirmed dialog id leaks oneDialogper answered call, forever.
Each retained
Dialogholds the full dialog state (request, 200 OK, negotiatedSDP, 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:588—do_invite_async: on a 2xx response the clientdialog is re-inserted under its confirmed id (
new_id, with the remoteto-tag):
src/dialog/invitation.rs:530—do_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:510—pub fn remove_dialog(&self, id: &DialogId).The dialog state machine transitions to
Terminated(
src/dialog/dialog.rs,DialogState::Terminated(..)) but the transition doesnot 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 isremoved), 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 callremove_dialogby hand, which confirms the contract is "caller must remove" — butnothing 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):
DialogLayer::do_invite(ordo_invite_async) to an endpoint that returns200 OK.remove_dialog.dialog_layer_inner.dialogs.len()(or expose a counter) after eachcall: 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
(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.
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:
DialogState::Terminated, have it remove itself fromDialogLayerInner.dialogs. TheClientInviteDialogalready holds a referenceto the layer inner (it uses
self.dialog_layer_inner.dialogs.remove(..)forthe early key at
invitation.rs:161/182/518), so the confirmed entry can beremoved at the same place the
Terminatedtransition is emitted. Mirror thesame for the server side.
dialog), provide a reaper / TTL on terminated dialogs, or hold entries as
Weakso a dropped dialog disappears from the map.do_invite/do_invite_asyncthat the callerMUST call
remove_dialogon the confirmed id after termination, and consider adebug-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
Dropcallsremove_dialog(&dialog.id())(the confirmed id) on every exit path of the calldriver, 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.