Repository navigation
Deprecated ServerInviteDialog::handle moves the dialog from WaitAck back to Calling when accept() came first #196
Description
Found while integrating rsipstack in a SIP application; same reporter as #192.
Versions affected. 0.7.3 (latest) (5/5 runs), 0.6.12 and 0.6.10 (5/5
runs each). In 0.6.x ServerInviteDialog is not deprecated. In 0.7.x it is
deprecated in favour of InviteDialog, but still public.
Environment. rsipstack 0.7.3, the latest version on crates.io, default features. Rust 1.98.1 stable, tokio 1.53 (multi-thread runtime). Linux 6.12 x86_64, 4 CPUs. UDP over loopback only: everything on 127.0.0.1, every port assigned by the OS. Default EndpointOption (T1 = 500 ms, T2 = 4 s, T4 = 5 s).
Where. Line numbers are from the 0.7.3 sources.
-
src/dialog/server_dialog.rs:881-891(ServerInviteDialog::handle_invite):if !self.inner.is_confirmed() && matches!(tx.original.method, crate::sip::Method::Invite) && self.inner.transition(DialogState::Calling(self.id())).is_ok()
WaitAckcounts as "not confirmed". If the application calledaccept()
(server_dialog.rs:185-199, which setsWaitAck) beforehandle()ran,
handle_inviteoverwritesWaitAckwithCalling. -
DialogInner::transition(src/dialog/dialog.rs:1643-1677) only refuses
Terminated → *andConfirmed → WaitAck, soWaitAck → Callingis
accepted. -
InviteDialog::handle_invite(src/dialog/invite_dialog.rs:928-938)
checksmatches!(self.state(), Calling(_))first, which avoids the
deterministic case. The check and the transition still take the lock
twice. I could not make that window fire: 0 out of 5 000 attempts on 0.7.3, and the same on 0.6.12 and 0.6.10, withaccept()
racinghandle()on two tasks (part B of the program).
How to run. Part A receives the INVITE and builds the dialog with
get_or_create_server_invite. It converts it with
ServerInviteDialog::try_from(&Dialog::Invite(..)), calls
accept(None, None), then spawns handle(). The peer does not ACK yet.
Reproduction program (one self-contained file, 341 lines) and how to run it
It uses only rsipstack's public API plus a raw tokio::net::UdpSocket peer
on 127.0.0.1. It prints what it observes and exits with 1 when the problem
reproduces, 0 when it does not.
Cargo.toml (replace the [dependencies] section):
[dependencies]
rsipstack = "=0.7.3"
tokio = { version = "1", features = ["rt-multi-thread", "macros", "net", "time", "sync"] }
tokio-util = "0.7"src/main.rs:
//! Issue B: `ServerInviteDialog::handle()` moves the dialog from `WaitAck`
//! back to `Calling` when `accept()` ran first.
//!
//! Part A (deterministic, deprecated `ServerInviteDialog`): `accept()`, then
//! `handle()`. The peer does not ACK yet.
//! Part B (race, `InviteDialog`): `accept()` and `handle()` start on two
//! tasks; calls whose states go `WaitAck` -> `Calling` are counted.
//!
//! Usage: cargo run --release -- [calls for part B] (default 1000)
#![allow(deprecated, dead_code)] // dead_code: the harness is shared by several issues
use rsipstack::dialog::dialog::{Dialog, DialogState};
use rsipstack::dialog::server_dialog::ServerInviteDialog;
use std::net::SocketAddr;
use std::time::{Duration, Instant};
use tokio::sync::mpsc::{unbounded_channel, UnboundedReceiver};
fn went_back(states: &[String]) -> bool {
states
.windows(2)
.any(|w| w[0] == "WaitAck" && w[1] == "Calling")
}
#[tokio::main(flavor = "multi_thread")]
async fn main() -> Result<(), BoxError> {
println!(
"== issue B, WaitAck -> Calling | {} | 127.0.0.1 only",
rsipstack::VERSION
);
let n: usize = std::env::args()
.nth(1)
.and_then(|a| a.parse().ok())
.unwrap_or(1000);
let stack = Stack::start().await?;
let mut calls = stack.uas_loop()?;
let uas = stack.addr;
let peer = Peer::bind().await?;
// Part A.
peer.send(uas, &peer.invite(uas, "b-a")).await?;
let mut call = calls.recv().await.ok_or("no call")?;
let server = ServerInviteDialog::try_from(&Dialog::Invite(call.dialog.clone()))?;
server.accept(None, None)?;
let mut tx = call.tx.take().ok_or("no tx")?;
let mut s2 = server.clone();
tokio::spawn(async move { s2.handle(&mut tx).await });
let states_a = drain_states(&mut call.states, Duration::from_millis(300)).await;
let now_a = state_name(&server.state());
let got_200 = peer
.recv_until(Duration::from_millis(200), |r| r.is_response(200, "INVITE"))
.await
.is_some();
let bye = tokio::time::timeout(Duration::from_secs(1), server.bye()).await;
let bye_sent = peer
.recv_until(Duration::from_millis(300), |r| r.is_request("BYE"))
.await
.is_some();
println!(
"A (ServerInviteDialog, accept() then handle()): 200 sent: {got_200}; states: {states_a:?}"
);
println!(
"A: state() 300 ms later, no ACK yet: {now_a}; bye() -> {:?}; BYE on the wire: {bye_sent}",
bye.map(|r| r.map_err(|e| e.to_string()))
);
// Part B.
let mut backwards = 0;
for i in 0..n {
let call_id = format!("b-b-{i}");
peer.send(uas, &peer.invite(uas, &call_id)).await?;
let mut call = calls.recv().await.ok_or("no call")?;
let d = call.dialog.clone();
// Shift accept() by 0..63 scheduler yields to sweep the window.
let accept = tokio::spawn(async move {
for _ in 0..(i % 64) {
tokio::task::yield_now().await;
}
d.accept(None, None)
});
call.spawn_handle();
let _ = accept.await?;
if went_back(&drain_states(&mut call.states, Duration::from_millis(30)).await) {
backwards += 1;
}
let ok = peer
.recv_until(Duration::from_millis(500), |r| {
r.is_response(200, "INVITE")
&& header(&r.text, "Call-ID").as_deref() == Some(call_id.as_str())
})
.await;
if let Some(ok) = ok {
peer.send(uas, &peer.ack_for(uas, &ok)).await?;
}
}
println!(
"B (InviteDialog, accept() racing handle()): {backwards}/{n} calls went WaitAck -> Calling"
);
let a = went_back(&states_a);
verdict(
a || backwards > 0,
&format!("part A backwards: {a}; part B backwards: {backwards}/{n}"),
)
}
// ---- harness: the stack under test and a raw UDP peer, 127.0.0.1 only ----
type BoxError = Box<dyn std::error::Error + Send + Sync>;
fn verdict(reproduced: bool, what: &str) -> ! {
let tag = if reproduced {
"REPRODUCED"
} else {
"not reproduced"
};
println!("RESULT: {tag} - {what}");
std::process::exit(i32::from(reproduced))
}
/// The stack under test: one UDP transport on 127.0.0.1:0, default options.
struct Stack {
endpoint: rsipstack::transaction::Endpoint,
dialogs: std::sync::Arc<rsipstack::dialog::dialog_layer::DialogLayer>,
addr: SocketAddr,
cancel: tokio_util::sync::CancellationToken,
}
impl Stack {
async fn start() -> Result<Stack, BoxError> {
let cancel = tokio_util::sync::CancellationToken::new();
let tl = rsipstack::transport::TransportLayer::new(cancel.child_token());
let udp = rsipstack::transport::udp::UdpConnection::create_connection(
"127.0.0.1:0".parse()?,
None,
Some(cancel.child_token()),
)
.await?;
let addr = udp.get_addr().get_socketaddr()?;
tl.add_transport(udp.into());
let endpoint = rsipstack::EndpointBuilder::new()
.with_user_agent("repro")
.with_transport_layer(tl)
.with_cancel_token(cancel.child_token())
.build();
let inner = endpoint.inner.clone();
tokio::spawn(async move { inner.serve().await });
let dialogs = std::sync::Arc::new(rsipstack::dialog::dialog_layer::DialogLayer::new(
endpoint.inner.clone(),
));
Ok(Stack {
endpoint,
dialogs,
addr,
cancel,
})
}
}
impl Drop for Stack {
fn drop(&mut self) {
self.cancel.cancel();
}
}
/// A new server INVITE dialog, its state receiver and its (not yet handled)
/// transaction.
struct UasCall {
dialog: rsipstack::dialog::invite_dialog::InviteDialog,
states: UnboundedReceiver<DialogState>,
tx: Option<rsipstack::transaction::Transaction>,
}
impl UasCall {
fn spawn_handle(&mut self) {
if let Some(mut tx) = self.tx.take() {
let mut d = self.dialog.clone();
tokio::spawn(async move { d.handle(&mut tx).await });
}
}
}
impl Stack {
/// The usual UAS loop: in-dialog requests go to `match_dialog` + `handle()`;
/// each new INVITE gets `get_or_create_server_invite` and is handed over.
fn uas_loop(&self) -> Result<UnboundedReceiver<UasCall>, BoxError> {
use rsipstack::sip::prelude::HeadersExt;
let mut incoming = self.endpoint.incoming_transactions()?;
let dl = self.dialogs.clone();
let (call_tx, call_rx) = unbounded_channel();
tokio::spawn(async move {
while let Some(mut tx) = incoming.recv().await {
let to_tag = tx
.original
.to_header()
.ok()
.and_then(|h| h.tag().ok().flatten());
if to_tag.is_some() {
if let Some(mut d) = dl.match_dialog(&tx) {
tokio::spawn(async move { d.handle(&mut tx).await });
}
} else if tx.original.method == rsipstack::sip::Method::Invite {
let (state_tx, states) = unbounded_channel();
if let Ok(dialog) = dl.get_or_create_server_invite(&tx, state_tx, None, None) {
let _ = call_tx.send(UasCall {
dialog,
states,
tx: Some(tx),
});
}
}
}
});
Ok(call_rx)
}
}
fn state_name(s: &DialogState) -> String {
match s {
DialogState::Calling(_) => "Calling".into(),
DialogState::Trying(_) => "Trying".into(),
DialogState::Early(..) => "Early".into(),
DialogState::WaitAck(..) => "WaitAck".into(),
DialogState::Confirmed(..) => "Confirmed".into(),
DialogState::Terminated(_, r) => format!("Terminated({r:?})"),
_ => "Other".into(),
}
}
/// Every state queued on `rx` within `wait`.
async fn drain_states(rx: &mut UnboundedReceiver<DialogState>, wait: Duration) -> Vec<String> {
let mut out = Vec::new();
let deadline = Instant::now() + wait;
while let Ok(Some(s)) = tokio::time::timeout_at(deadline.into(), rx.recv()).await {
out.push(state_name(&s));
}
out
}
/// The far end: a raw UDP socket that writes and reads SIP text by hand.
struct Peer {
sock: tokio::net::UdpSocket,
addr: SocketAddr,
}
/// One datagram received by the peer.
struct Rx {
text: String,
from: SocketAddr,
}
impl Rx {
fn first_line(&self) -> &str {
self.text.lines().next().unwrap_or("")
}
fn is_request(&self, method: &str) -> bool {
self.first_line().starts_with(&format!("{method} "))
}
fn is_response(&self, code: u16, cseq_method: &str) -> bool {
self.first_line().starts_with(&format!("SIP/2.0 {code} "))
&& header(&self.text, "CSeq").is_some_and(|c| c.ends_with(cseq_method))
}
}
/// The value of the first header `name` (full name, case-insensitive).
fn header(text: &str, name: &str) -> Option<String> {
let prefix = format!("{}:", name.to_ascii_lowercase());
text.split("\r\n")
.find(|l| l.to_ascii_lowercase().starts_with(&prefix))
.map(|l| l[prefix.len()..].trim().to_string())
}
impl Peer {
async fn bind() -> Result<Peer, BoxError> {
let sock = tokio::net::UdpSocket::bind("127.0.0.1:0").await?;
let addr = sock.local_addr()?;
Ok(Peer { sock, addr })
}
async fn send(&self, to: SocketAddr, text: &str) -> Result<(), BoxError> {
self.sock.send_to(text.as_bytes(), to).await?;
Ok(())
}
async fn recv(&self, wait: Duration) -> Option<Rx> {
let mut buf = vec![0u8; 65536];
let (n, from) = tokio::time::timeout(wait, self.sock.recv_from(&mut buf))
.await
.ok()?
.ok()?;
Some(Rx {
text: String::from_utf8_lossy(&buf[..n]).into_owned(),
from,
})
}
/// The first datagram matching `pred` within `wait`; others are dropped.
async fn recv_until(&self, wait: Duration, pred: impl Fn(&Rx) -> bool) -> Option<Rx> {
let deadline = Instant::now() + wait;
loop {
let rx = self
.recv(deadline.saturating_duration_since(Instant::now()))
.await?;
if pred(&rx) {
return Some(rx);
}
}
}
}
impl Peer {
/// An out-of-dialog INVITE from this peer to `uas`.
fn invite(&self, uas: SocketAddr, call_id: &str) -> String {
let me = self.addr;
format!(
"INVITE sip:bob@{uas} SIP/2.0\r\n\
Via: SIP/2.0/UDP {me};branch=z9hG4bK-{call_id}-inv;rport\r\n\
Max-Forwards: 70\r\n\
From: <sip:alice@{me}>;tag=a1\r\n\
To: <sip:bob@{uas}>\r\n\
Call-ID: {call_id}\r\n\
CSeq: 1 INVITE\r\n\
Contact: <sip:alice@{me}>\r\n\
Content-Length: 0\r\n\r\n"
)
}
/// The ACK for a 2xx (new branch, To copied from the 2xx).
fn ack_for(&self, uas: SocketAddr, ok: &Rx) -> String {
let me = self.addr;
let to = header(&ok.text, "To").unwrap_or_default();
let call_id = header(&ok.text, "Call-ID").unwrap_or_default();
format!(
"ACK sip:bob@{uas} SIP/2.0\r\n\
Via: SIP/2.0/UDP {me};branch=z9hG4bK-{call_id}-ack;rport\r\n\
Max-Forwards: 70\r\n\
From: <sip:alice@{me}>;tag=a1\r\n\
To: {to}\r\n\
Call-ID: {call_id}\r\n\
CSeq: 1 ACK\r\n\
Content-Length: 0\r\n\r\n"
)
}
}Run:
cargo new repro && cd repro # then paste the two blocks above
cargo run --release -- 1000Observed.
== issue B, WaitAck -> Calling | rsipstack/0.7.3 | 127.0.0.1 only
A (ServerInviteDialog, accept() then handle()): 200 sent: true; states: ["WaitAck", "Calling"]
A: state() 300 ms later, no ACK yet: Calling; bye() -> Ok(Err("Error: dialog b-a-lkAU3Fxh-a1 cannot send BYE in state b-a-lkAU3Fxh-a1(Calling)")); BYE on the wire: false
B (InviteDialog, accept() racing handle()): 0/1000 calls went WaitAck -> Calling
RESULT: REPRODUCED - part A backwards: true; part B backwards: 0/1000
After the 2xx has been sent, the state goes backwards to Calling, and the
state stream reports it. Until the ACK arrives, waiting_ack() is false and
bye() is refused. An application that times out the ACK by watching
WaitAck never fires.
Expected. Once the UAS has sent a 2xx, it waits for the ACK (RFC 3261
§13.3.1.4). The dialog never returns to Calling.
Suggested fix. Make ServerInviteDialog::handle_invite use the same
guard as InviteDialog (only Calling → Calling). Better, do the check and
the transition under one lock, for example a transition_if(|old| matches!(old, Calling(_)), new). That also closes the theoretical window in
InviteDialog. Alternatively, reject WaitAck | Early → Calling in
DialogInner::transition.
Related. #184 and #185 fixed a deprecated wrapper (ClientInviteDialog)
in the same way, so fixing a deprecated type has precedent.
Activity