mollusk/peerspeak
Public Access
1
0
Fork 0
Code Issues Pull Requests 3 Actions Packages Projects Releases 10 Wiki Activity

fix(core,audio,app): Tier B bug-sweep fixes (F-05, F-06, F-10, F-11)

Browse Source 
Four confirmed P2 findings from the 2026-06-22 adversarial bug sweep.
None change the wire format / PeerState / GOSSIP_PROTO — all local.

- F-05: re-key the A8 rejoin archive (known_peers) and RecoveryContext
  by topic_id ([u8; 32]) instead of the raw ticket string. A W7-
  restamped member ticket shares the room's topic but not its string,
  so a rejoin-from-Recents previously missed the retained bootstrap
  bucket and dropped to an empty bootstrap — the exact dead-end A8
  fixed. Topic is derived once via PeerSpeakTicket::topic_of in Join;
  a malformed ticket now fails early and clean.
- F-06: an in-call Join no longer leaks the old room's peers/chat into
  the new room, nor strands stale presence on a failed switch. Core
  captures was_in_room, clears current_room at teardown, and emits a
  new local UiEvent::RoomReset on every post-teardown failure path so
  a failed switch lands idle on Home. The UI's room-scoped clearing is
  factored into AppState::reset_room_state(), called by RoomLeft,
  RoomReset, and at the top of RoomJoined — so a successful switch
  clears+repopulates seamlessly on the Room screen (no Home bounce, no
  leave chime).
- F-10: echo-cancel virtual nodes now get per-PID-unique names
  (peerspeak_echocancel_{source,sink}.<pid>); the guard carries them
  and core targets them instead of the fixed constants. unload_stale
  only unloads our modules whose owner PID is dead (/proc check, cfg-
  gated; conservative elsewhere), so enabling AEC in one instance can
  no longer tear down another live instance's call. Pure
  pid_from_ec_args / ec_module_is_stale seams.
- F-11: a recording write failure now stops recording atomically
  (best-effort finalize via stop_recording + one UI Error) instead of
  looping the error at ~50 Hz with silent data loss. Both mixer
  branches release the recorder mutex before calling stop_recording to
  avoid a self-deadlock on the non-reentrant std::Mutex.

407 lib tests pass (+4), clippy --all-targets clean, release build
green. Tests-green only; the rejoin (F-05), in-call switch (F-06),
two-instance AEC (F-10), and disk-full (F-11) paths need a real run.
Implemented by Codex, reviewed + gates re-run by senior.

Co-Authored-By: Codex <codex@openai.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
mollusk
2026-06-22 15:38:48 -04:00
co-authored by Codex Claude Opus 4.8
parent 49bd2ba687
commit 6100abef33
4 changed files with 322 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/audio/echo_cancel.rs
+84 -13
View File
@@ -34,6 +34,18 @@ const NODE_READY_TIMEOUT: Duration = Duration::from_secs(3);
/// nodes never leak past the call that created them.
pub struct EchoCancelGuard {
module_index: String,
source_name: String,
sink_name: String,
}
impl EchoCancelGuard {
pub fn source_name(&self) -> &str {
&self.source_name
}
pub fn sink_name(&self) -> &str {
&self.sink_name
}
}
impl Drop for EchoCancelGuard {
@@ -58,12 +70,16 @@ pub fn enable(real_source: Option<&str>, real_sink: Option<&str>) -> Result<Echo
// don't stack duplicate modules / fight over the virtual node names.
unload_stale();
let owner_pid = std::process::id();
let source_name = format!("{EC_SOURCE}.{owner_pid}");
let sink_name = format!("{EC_SINK}.{owner_pid}");
let mut cmd = Command::new("pactl");
cmd.arg("load-module")
.arg("module-echo-cancel")
.arg("aec_method=webrtc")
.arg(format!("source_name={EC_SOURCE}"))
.arg(format!("sink_name={EC_SINK}"));
.arg(format!("source_name={source_name}"))
.arg(format!("sink_name={sink_name}"));
if let Some(src) = real_source.filter(|s| !s.is_empty()) {
cmd.arg(format!("source_master={src}"));
}
@@ -85,12 +101,12 @@ pub fn enable(real_source: Option<&str>, real_sink: Option<&str>) -> Result<Echo
if module_index.parse::<u64>().is_err() {
return Err(format!("unexpected pactl output: {module_index:?}"));
}
let guard = EchoCancelGuard { module_index };
let guard = EchoCancelGuard { module_index, source_name, sink_name };
// The virtual nodes appear shortly after the module loads; wait for both so
// the subsequent capture/playback streams can actually target them. If they
// never show, drop the guard (unloads) and report failure.
if !wait_for_nodes() {
if !wait_for_nodes(guard.source_name(), guard.sink_name()) {
return Err("echo-cancel virtual nodes did not appear in time".to_string());
}
@@ -102,10 +118,10 @@ pub fn enable(real_source: Option<&str>, real_sink: Option<&str>) -> Result<Echo
}
/// Polls until both virtual nodes exist or the timeout elapses.
fn wait_for_nodes() -> bool {
fn wait_for_nodes(source_name: &str, sink_name: &str) -> bool {
let deadline = Instant::now() + NODE_READY_TIMEOUT;
loop {
if node_present("sources", EC_SOURCE) && node_present("sinks", EC_SINK) {
if node_present("sources", source_name) && node_present("sinks", sink_name) {
return true;
}
if Instant::now() >= deadline {
@@ -126,8 +142,30 @@ fn node_present(kind: &str, name: &str) -> bool {
.any(|line| line.split('\t').nth(1) == Some(name))
}
/// Unloads any leftover `module-echo-cancel` instance we previously created
/// (identified by our virtual node names in its argument string). Best-effort.
fn pid_from_ec_args(args: &str) -> Option<u32> {
let source_prefix = format!("source_name={EC_SOURCE}.");
args.split_whitespace()
.find_map(|arg| arg.strip_prefix(&source_prefix))?
.parse()
.ok()
}
fn ec_module_is_stale(args: &str, is_alive: impl Fn(u32) -> bool) -> bool {
pid_from_ec_args(args).is_some_and(|pid| !is_alive(pid))
}
#[cfg(target_os = "linux")]
fn process_is_alive(pid: u32) -> bool {
std::path::Path::new("/proc").join(pid.to_string()).exists()
}
#[cfg(not(target_os = "linux"))]
fn process_is_alive(_pid: u32) -> bool {
true
}
/// Unloads leftover PeerSpeak `module-echo-cancel` instances only when their
/// owning process is gone. Best-effort and conservative on non-Linux platforms.
fn unload_stale() {
let Ok(out) = Command::new("pactl").arg("list").arg("modules").arg("short").output() else {
return;
@@ -137,7 +175,10 @@ fn unload_stale() {
let index = cols.next().unwrap_or("");
let name = cols.next().unwrap_or("");
let args = cols.next().unwrap_or("");
if name == "module-echo-cancel" && args.contains(EC_SOURCE) && index.parse::<u64>().is_ok() {
if name == "module-echo-cancel"
&& ec_module_is_stale(args, process_is_alive)
&& index.parse::<u64>().is_ok()
{
let _ = Command::new("pactl").arg("unload-module").arg(index).output();
crate::log_msg(&format!("Echo cancel: cleaned up stale module {index}"));
}
@@ -155,12 +196,42 @@ mod tests {
#[ignore]
fn enable_creates_and_unloads_nodes() {
let guard = enable(None, None).expect("module-echo-cancel should load");
assert!(node_present("sources", EC_SOURCE), "cleaned source must exist");
assert!(node_present("sinks", EC_SINK), "reference sink must exist");
let source_name = guard.source_name().to_string();
let sink_name = guard.sink_name().to_string();
assert!(node_present("sources", &source_name), "cleaned source must exist");
assert!(node_present("sinks", &sink_name), "reference sink must exist");
drop(guard);
// Give pactl a moment to tear the nodes down.
std::thread::sleep(Duration::from_millis(300));
assert!(!node_present("sources", EC_SOURCE), "source must be gone after unload");
assert!(!node_present("sinks", EC_SINK), "sink must be gone after unload");
assert!(!node_present("sources", &source_name), "source must be gone after unload");
assert!(!node_present("sinks", &sink_name), "sink must be gone after unload");
}
#[test]
fn parses_owner_pid_only_from_our_source_name() {
assert_eq!(
pid_from_ec_args(
"aec_method=webrtc source_name=peerspeak_echocancel_source.4242 sink_name=peerspeak_echocancel_sink.4242"
),
Some(4242)
);
assert_eq!(pid_from_ec_args("aec_method=webrtc"), None);
assert_eq!(
pid_from_ec_args("source_name=peerspeak_echocancel_source.not-a-pid"),
None
);
assert_eq!(pid_from_ec_args("source_name=someone_elses_source.4242"), None);
}
#[test]
fn stale_decision_keeps_live_and_foreign_modules() {
let ours = "source_name=peerspeak_echocancel_source.4242";
assert!(!ec_module_is_stale(ours, |pid| pid == 4242));
assert!(ec_module_is_stale(ours, |_| false));
assert!(!ec_module_is_stale("source_name=foreign.4242", |_| false));
assert!(!ec_module_is_stale(
"source_name=peerspeak_echocancel_source.malformed",
|_| false
));
}
}