fix(audio): make module teardown cancellation-safe
This commit is contained in:
+438
-156
@@ -33,33 +33,27 @@
|
||||
use anyhow::{Context, Result, bail};
|
||||
use std::cell::RefCell;
|
||||
use std::collections::BTreeMap;
|
||||
use std::process::Command;
|
||||
use std::io::{self, Read};
|
||||
use std::process::{Child, Command, ExitStatus, Stdio};
|
||||
use std::rc::Rc;
|
||||
use std::sync::Arc;
|
||||
use std::thread::JoinHandle;
|
||||
use std::time::Duration;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use crate::cli::HostOpts;
|
||||
use crate::host::ledger::{self, LedgerError, ModuleLedger, UnloadOutcome};
|
||||
use crate::repair::plan::{self as repair_plan, Fingerprint, Shape};
|
||||
|
||||
/// How long a single `pactl load-module` / `unload-module` may take.
|
||||
/// How long a `pactl load-module` worker may run before it is killed and reaped.
|
||||
///
|
||||
/// A bound, not a calibration: a local Pulse socket answers in milliseconds, and
|
||||
/// this exists only so a wedged server cannot hang teardown forever. It is
|
||||
/// deliberately generous because exceeding it is no longer destructive — the
|
||||
/// deliberately generous because exceeding it is no longer destructive: the
|
||||
/// ledger records the attempt, and reconciliation finds whatever the server
|
||||
/// actually did.
|
||||
/// actually did. Unloads use PulseSession's independently bounded native
|
||||
/// connect/list/unload requests instead of a second pactl connection.
|
||||
const PACTL_BUDGET: Duration = Duration::from_secs(5);
|
||||
|
||||
/// How many reconcile-then-unload rounds teardown runs.
|
||||
///
|
||||
/// Two, because one round can create exactly one new question: an ambiguous load
|
||||
/// resolves to a module that then needs unloading, and an uncertain unload
|
||||
/// resolves to a module that is either gone or still there. A second round
|
||||
/// settles either. Anything still unresolved after that is left to `--repair`
|
||||
/// rather than looped over.
|
||||
const TEARDOWN_ROUNDS: usize = 2;
|
||||
const PACTL_REAP_BUDGET: Duration = Duration::from_secs(1);
|
||||
|
||||
/// Owns the pactl-loaded modules plus, when filtering is active, the
|
||||
/// libpipewire stream-router thread. Drop unloads modules as a backstop;
|
||||
@@ -86,6 +80,17 @@ impl Routing {
|
||||
let pid = std::process::id();
|
||||
let sink_name = repair_plan::sink_name_for(pid);
|
||||
let ledger = ModuleLedger::new();
|
||||
// Construct the owner before the first mutation. Any error or cancellation
|
||||
// below now drops a real `Routing`, whose backstop closes, quiesces,
|
||||
// reconciles, and unloads this ledger. Previously the owner did not exist
|
||||
// until both initial modules had loaded, so constructor failure leaked
|
||||
// everything loaded up to that point.
|
||||
let mut routing = Self {
|
||||
ledger: Arc::clone(&ledger),
|
||||
sink_name: sink_name.clone(),
|
||||
stream_router: None,
|
||||
event_task: None,
|
||||
};
|
||||
|
||||
// Every module this host loads carries an ownership token, minted per
|
||||
// load, so `--repair` can tell whose pid the name refers to instead of
|
||||
@@ -119,13 +124,6 @@ impl Routing {
|
||||
"audio routing: null-sink ready (loopback skipped in strict app mode)"
|
||||
);
|
||||
|
||||
let mut routing = Self {
|
||||
ledger: Arc::clone(&ledger),
|
||||
sink_name: sink_name.clone(),
|
||||
stream_router: None,
|
||||
event_task: None,
|
||||
};
|
||||
|
||||
if let Some(app) = &opts.app {
|
||||
let (router, mut event_rx) = StreamRouter::spawn(app.clone(), sink_name.clone())?;
|
||||
let ledger_for_task = Arc::clone(&ledger);
|
||||
@@ -138,7 +136,8 @@ impl Routing {
|
||||
tracing::info!(
|
||||
"audio routing: first stream routed → unloading default-sink loopback"
|
||||
);
|
||||
unload_module(&ledger_for_task, Shape::LoopbackIntoCapture).await;
|
||||
let mirror_absent =
|
||||
unload_module(&ledger_for_task, Shape::LoopbackIntoCapture).await;
|
||||
// Mirror the routed app back to the sharer's own
|
||||
// speakers so they hear the content they're sharing.
|
||||
// Loaded *after* the default-sink loopback is gone so
|
||||
@@ -146,7 +145,15 @@ impl Routing {
|
||||
// sourced from the null-sink monitor — the chosen app
|
||||
// only, never the desktop/call — so it can't echo into
|
||||
// the capture.
|
||||
ensure_loaded(&ledger_for_task, Shape::LoopbackOutOfCapture, pid).await;
|
||||
if mirror_absent {
|
||||
ensure_loaded(&ledger_for_task, Shape::LoopbackOutOfCapture, pid)
|
||||
.await;
|
||||
} else {
|
||||
tracing::warn!(
|
||||
"audio routing: default-sink mirror absence was not confirmed; \
|
||||
refusing to load the inverse local monitor"
|
||||
);
|
||||
}
|
||||
// Tell the front-end the chosen app's audio is live.
|
||||
output::emit(output::Event::AppAudio {
|
||||
state: AppAudioState::Routed,
|
||||
@@ -161,7 +168,8 @@ impl Routing {
|
||||
// The shared app is gone, so its null-sink is silent:
|
||||
// stop mirroring it to the sharer's speakers. Re-loads
|
||||
// on the next FirstRoutedStream if the app resumes.
|
||||
unload_module(&ledger_for_task, Shape::LoopbackOutOfCapture).await;
|
||||
let local_monitor_absent =
|
||||
unload_module(&ledger_for_task, Shape::LoopbackOutOfCapture).await;
|
||||
if strict {
|
||||
// Strict mode: do NOT restore the whole-desktop
|
||||
// loopback. Viewers hear silence until the app
|
||||
@@ -172,6 +180,13 @@ impl Routing {
|
||||
);
|
||||
continue;
|
||||
}
|
||||
if !local_monitor_absent {
|
||||
tracing::warn!(
|
||||
"audio routing: local-monitor absence was not confirmed; \
|
||||
refusing to restore the inverse default-sink mirror"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
// Best-effort mode: restore the default-sink loopback
|
||||
// so the viewer hears system audio again instead of
|
||||
// silence. Already loaded is not an error — the ledger
|
||||
@@ -216,6 +231,10 @@ impl Routing {
|
||||
/// module — but only a path that then reconciles can actually clean it up.
|
||||
/// `Drop` cannot await, which is why it is the narrower backstop.
|
||||
pub async fn shutdown(mut self) {
|
||||
// Closing is synchronous and happens first: after this point the event
|
||||
// task cannot register another mutation even if it receives one last
|
||||
// router event while shutdown is in progress.
|
||||
self.ledger.close();
|
||||
if let Some(router) = self.stream_router.take() {
|
||||
// ⚠️ Still an unbounded join: a wedged PipeWire thread parks this
|
||||
// task indefinitely. That is the pre-existing defect S3b exists for.
|
||||
@@ -240,22 +259,23 @@ impl Routing {
|
||||
}
|
||||
}
|
||||
|
||||
for _ in 0..TEARDOWN_ROUNDS {
|
||||
if let Err(e) = ledger::reconcile_pending(&self.ledger).await {
|
||||
tracing::warn!("audio routing: could not reconcile the module ledger: {e:#}");
|
||||
}
|
||||
let loaded = self.ledger.loaded();
|
||||
if loaded.is_empty() {
|
||||
break;
|
||||
}
|
||||
for fp in loaded {
|
||||
unload_module(&self.ledger, fp.shape).await;
|
||||
}
|
||||
// A cancelled `spawn_blocking` await detaches its worker. Every worker is
|
||||
// registered before it can be spawned, so this is a real ordering
|
||||
// boundary: reconciliation cannot overtake late module creation/removal.
|
||||
let ledger_for_wait = Arc::clone(&self.ledger);
|
||||
if let Err(e) = tokio::task::spawn_blocking(move || {
|
||||
ledger_for_wait.wait_for_operations();
|
||||
})
|
||||
.await
|
||||
{
|
||||
tracing::warn!("audio routing: module-operation wait task failed: {e}");
|
||||
}
|
||||
cleanup_modules(&self.ledger).await;
|
||||
|
||||
if !self.ledger.is_settled() {
|
||||
if !self.ledger.is_clean() {
|
||||
tracing::warn!(
|
||||
"audio routing: some audio modules could not be accounted for; \
|
||||
settled = self.ledger.is_settled(),
|
||||
"audio routing: some audio modules could not be removed safely; \
|
||||
`pixelpass --repair` will clean up anything left behind"
|
||||
);
|
||||
}
|
||||
@@ -270,22 +290,19 @@ impl Drop for Routing {
|
||||
///
|
||||
/// After a completed `shutdown` the ledger holds nothing and this does nothing.
|
||||
fn drop(&mut self) {
|
||||
self.ledger.close();
|
||||
if let Some(router) = self.stream_router.take() {
|
||||
router.shutdown();
|
||||
}
|
||||
if let Some(task) = self.event_task.take() {
|
||||
task.abort();
|
||||
}
|
||||
for fp in self.ledger.loaded() {
|
||||
if self.ledger.begin_unload(fp.shape).is_none() {
|
||||
continue;
|
||||
}
|
||||
let outcome = blocking_unload(fp.id);
|
||||
self.ledger.finish_unload(fp.shape, outcome);
|
||||
}
|
||||
if !self.ledger.is_settled() {
|
||||
self.ledger.close_and_wait();
|
||||
cleanup_modules_blocking(&self.ledger);
|
||||
if !self.ledger.is_clean() {
|
||||
tracing::warn!(
|
||||
"audio routing: torn down without settling the module ledger; \
|
||||
settled = self.ledger.is_settled(),
|
||||
"audio routing: torn down with modules that could not be removed safely; \
|
||||
run `pixelpass --repair` to clean up anything left behind"
|
||||
);
|
||||
}
|
||||
@@ -408,65 +425,61 @@ async fn load_module(ledger: &Arc<ModuleLedger>, shape: Shape, pid: u32) -> Resu
|
||||
.begin_load(shape, pid, owner.clone())
|
||||
.map_err(anyhow::Error::new)
|
||||
.with_context(|| format!("cannot load the {} module", shape.label()))?;
|
||||
let args = shape.render_args(pid, Some(&owner));
|
||||
|
||||
let mut cmd = tokio::process::Command::new("pactl");
|
||||
cmd.arg("load-module")
|
||||
.arg(shape.module_name())
|
||||
.args(shape.render_args(pid, Some(&owner)))
|
||||
.kill_on_drop(true);
|
||||
// The affine permit moves into the blocking worker. Dropping this await does
|
||||
// not cancel `spawn_blocking`; the worker remains registered, owns and reaps
|
||||
// its child, and settles the slot before teardown's quiescence barrier opens.
|
||||
tokio::task::spawn_blocking(move || -> Result<Fingerprint> {
|
||||
let output = permit.with_server_operation(|| {
|
||||
let mut command = Command::new("pactl");
|
||||
command
|
||||
.arg("load-module")
|
||||
.arg(shape.module_name())
|
||||
.args(args);
|
||||
bounded_output(&mut command, PACTL_BUDGET)
|
||||
});
|
||||
let output = match output {
|
||||
Ok(output) => output,
|
||||
Err(e) => {
|
||||
// Whether the server was reached is unknown. Leaving the permit
|
||||
// unsettled makes its Drop create a reconciliation question.
|
||||
drop(permit);
|
||||
return Err(e).context("failed to run pactl load-module");
|
||||
}
|
||||
};
|
||||
|
||||
// Deliberately **not** `select!`ed against a cancellation signal: a completed
|
||||
// load whose index was then dropped on the floor is precisely the defect the
|
||||
// ledger exists to prevent. Cancellation here happens by dropping this whole
|
||||
// future, and the permit's `Drop` turns that into a question reconciliation
|
||||
// can answer, rather than into silence.
|
||||
let output = match tokio::time::timeout(PACTL_BUDGET, cmd.output()).await {
|
||||
Ok(Ok(output)) => output,
|
||||
Ok(Err(e)) => {
|
||||
// Spawning or reading failed. Whether the server was ever reached is
|
||||
// not knowable from here, so leave the permit unsettled: an
|
||||
// unnecessary reconcile costs one listing, a missed one costs an
|
||||
// orphan.
|
||||
drop(permit);
|
||||
return Err(e).context("failed to run pactl load-module");
|
||||
}
|
||||
Err(_) => {
|
||||
if output.timed_out {
|
||||
drop(permit);
|
||||
bail!("pactl load-module did not finish within {PACTL_BUDGET:?}");
|
||||
}
|
||||
};
|
||||
|
||||
if !output.status.success() {
|
||||
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
|
||||
if output.status.code().is_some() {
|
||||
// pactl exited of its own accord, having reported the server's
|
||||
// refusal: nothing was created, so there is nothing to reconcile.
|
||||
permit.abandon();
|
||||
} else {
|
||||
// Killed by a signal, which may have arrived *after* the server
|
||||
// created the module.
|
||||
drop(permit);
|
||||
if !output.status.success() {
|
||||
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
|
||||
if output.status.code().is_some() {
|
||||
// pactl exited normally and reported the server's refusal.
|
||||
permit.abandon();
|
||||
} else {
|
||||
drop(permit);
|
||||
}
|
||||
bail!("pactl load-module failed: {stderr}");
|
||||
}
|
||||
bail!("pactl load-module failed: {stderr}");
|
||||
}
|
||||
|
||||
let id_str = String::from_utf8_lossy(&output.stdout).trim().to_string();
|
||||
// Genuinely 32-bit, unlike `object.serial`: this is a PulseAudio module
|
||||
// index (`pa_module.index`, `uint32_t`), which `pactl unload-module` takes
|
||||
// back verbatim. Do not widen it.
|
||||
let Ok(index) = id_str.parse::<u32>() else {
|
||||
// The load may well have succeeded — we simply cannot say which module it
|
||||
// produced, which is exactly what reconciliation is for.
|
||||
drop(permit);
|
||||
bail!("pactl returned unexpected module ID: {id_str:?}");
|
||||
};
|
||||
let fp = permit.commit(index)?;
|
||||
tracing::info!(
|
||||
module = fp.id,
|
||||
shape = shape.label(),
|
||||
"audio routing: loaded pactl module"
|
||||
);
|
||||
Ok(fp)
|
||||
let id_str = String::from_utf8_lossy(&output.stdout).trim().to_string();
|
||||
// Genuinely 32-bit: this is a Pulse module index, not object.serial.
|
||||
let Ok(index) = id_str.parse::<u32>() else {
|
||||
drop(permit);
|
||||
bail!("pactl returned unexpected module ID: {id_str:?}");
|
||||
};
|
||||
let fp = permit.commit(index)?;
|
||||
tracing::info!(
|
||||
module = fp.id,
|
||||
shape = shape.label(),
|
||||
"audio routing: loaded pactl module"
|
||||
);
|
||||
Ok(fp)
|
||||
})
|
||||
.await
|
||||
.context("the pactl load worker failed")?
|
||||
}
|
||||
|
||||
/// Load `shape` unless the slot already holds it.
|
||||
@@ -496,49 +509,273 @@ async fn ensure_loaded(ledger: &Arc<ModuleLedger>, shape: Shape, pid: u32) {
|
||||
/// A no-op for a slot holding nothing. An outcome that cannot be confirmed is
|
||||
/// recorded as uncertain rather than assumed done, so the module keeps being
|
||||
/// named until the server is asked about it.
|
||||
async fn unload_module(ledger: &Arc<ModuleLedger>, shape: Shape) {
|
||||
let Some(fp) = ledger.begin_unload(shape) else {
|
||||
return;
|
||||
};
|
||||
let mut cmd = tokio::process::Command::new("pactl");
|
||||
cmd.arg("unload-module")
|
||||
.arg(fp.id.to_string())
|
||||
.kill_on_drop(true);
|
||||
let outcome = match tokio::time::timeout(PACTL_BUDGET, cmd.output()).await {
|
||||
Ok(Ok(output)) if output.status.success() => {
|
||||
tracing::info!(module = fp.id, "audio routing: unloaded pactl module");
|
||||
UnloadOutcome::Confirmed
|
||||
}
|
||||
Ok(Ok(output)) => UnloadOutcome::Uncertain(format!(
|
||||
"pactl unload-module exited {}: {}",
|
||||
output.status,
|
||||
String::from_utf8_lossy(&output.stderr).trim()
|
||||
)),
|
||||
Ok(Err(e)) => UnloadOutcome::Uncertain(format!("could not run pactl unload-module: {e}")),
|
||||
Err(_) => {
|
||||
UnloadOutcome::Uncertain(format!("pactl unload-module exceeded {PACTL_BUDGET:?}"))
|
||||
}
|
||||
};
|
||||
ledger.finish_unload(shape, outcome);
|
||||
async fn unload_module(ledger: &Arc<ModuleLedger>, shape: Shape) -> bool {
|
||||
unload_module_inner(ledger, shape, false).await
|
||||
}
|
||||
|
||||
/// The blocking unload [`Drop`] uses, since it has no runtime to await on.
|
||||
fn blocking_unload(id: u32) -> UnloadOutcome {
|
||||
match Command::new("pactl")
|
||||
.arg("unload-module")
|
||||
.arg(id.to_string())
|
||||
.output()
|
||||
{
|
||||
Ok(output) if output.status.success() => {
|
||||
tracing::info!(module = id, "audio routing: unloaded pactl module");
|
||||
UnloadOutcome::Confirmed
|
||||
async fn unload_module_inner(ledger: &Arc<ModuleLedger>, shape: Shape, cleanup: bool) -> bool {
|
||||
let begun = if cleanup {
|
||||
ledger.begin_cleanup_unload(shape)
|
||||
} else {
|
||||
ledger.begin_unload(shape)
|
||||
};
|
||||
let permit = match begun {
|
||||
Ok(Some(permit)) => permit,
|
||||
Ok(None) => return true,
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
shape = shape.label(),
|
||||
"audio routing: refusing module unload: {e}"
|
||||
);
|
||||
return false;
|
||||
}
|
||||
};
|
||||
match tokio::task::spawn_blocking(move || finish_verified_unload(permit)).await {
|
||||
Ok(confirmed_absent) => confirmed_absent,
|
||||
Err(e) => {
|
||||
// A panicking worker drops its affine permit and therefore leaves an
|
||||
// unload reconciliation question behind.
|
||||
tracing::warn!(
|
||||
shape = shape.label(),
|
||||
"audio routing: unload worker failed: {e}"
|
||||
);
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn finish_verified_unload(permit: ledger::UnloadPermit) -> bool {
|
||||
let fp = permit.fingerprint().clone();
|
||||
let result = permit.with_server_operation(|| verified_unload(&fp));
|
||||
match result {
|
||||
Ok(()) => {
|
||||
permit.finish(UnloadOutcome::Confirmed);
|
||||
true
|
||||
}
|
||||
Err(e) => {
|
||||
permit.finish(UnloadOutcome::Uncertain(format!("{e:#}")));
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum UnloadPresence {
|
||||
Exact,
|
||||
Absent,
|
||||
Replaced,
|
||||
}
|
||||
|
||||
fn unload_presence(fp: &Fingerprint, current: &[repair_plan::ModuleObservation]) -> UnloadPresence {
|
||||
match current.iter().find(|module| module.id == fp.id) {
|
||||
None => UnloadPresence::Absent,
|
||||
Some(observed) if fp.still_matches(observed) => UnloadPresence::Exact,
|
||||
Some(_) => UnloadPresence::Replaced,
|
||||
}
|
||||
}
|
||||
|
||||
/// Re-list and unload through one verified-local Pulse connection. The exact
|
||||
/// fingerprint is checked immediately before destruction; if the index vanished
|
||||
/// or was reused, our module is already absent and the replacement is left alone.
|
||||
fn verified_unload(fp: &Fingerprint) -> Result<()> {
|
||||
let mut session = crate::repair::introspect::PulseSession::connect()
|
||||
.context("could not connect to verify a module unload")?;
|
||||
let current = session
|
||||
.list_modules()
|
||||
.context("could not list modules immediately before unload")?;
|
||||
match unload_presence(fp, ¤t) {
|
||||
UnloadPresence::Exact => {}
|
||||
UnloadPresence::Absent => {
|
||||
tracing::info!(
|
||||
module = fp.id,
|
||||
shape = fp.shape.label(),
|
||||
"audio routing: tracked module was already absent"
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
UnloadPresence::Replaced => {
|
||||
tracing::warn!(
|
||||
module = fp.id,
|
||||
shape = fp.shape.label(),
|
||||
"audio routing: module index was reused; leaving the replacement alone"
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
session
|
||||
.unload_module(fp.id)
|
||||
.with_context(|| format!("the server did not confirm unloading module #{}", fp.id))?;
|
||||
tracing::info!(
|
||||
module = fp.id,
|
||||
shape = fp.shape.label(),
|
||||
"audio routing: unloaded verified Pulse module"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// One bounded child result. On deadline the child is killed and synchronously
|
||||
/// reaped before this returns, so server reconciliation cannot overtake a late
|
||||
/// `pactl` request merely because its async waiter was cancelled.
|
||||
struct BoundedOutput {
|
||||
status: ExitStatus,
|
||||
stdout: Vec<u8>,
|
||||
stderr: Vec<u8>,
|
||||
timed_out: bool,
|
||||
}
|
||||
|
||||
/// Ensures every early-return and panic after spawn kills and reaps the child.
|
||||
/// The normal path marks it reaped after `try_wait`/`wait` obtained the status.
|
||||
struct ReapedChild {
|
||||
child: Child,
|
||||
reaped: bool,
|
||||
}
|
||||
|
||||
impl Drop for ReapedChild {
|
||||
fn drop(&mut self) {
|
||||
if self.reaped {
|
||||
return;
|
||||
}
|
||||
let _ = self.child.kill();
|
||||
match self.reap_within(PACTL_REAP_BUDGET) {
|
||||
Ok(Some(_)) => {}
|
||||
Ok(None) => tracing::warn!(
|
||||
"audio routing: killed pactl child was not reaped within {PACTL_REAP_BUDGET:?}"
|
||||
),
|
||||
Err(e) => tracing::warn!("audio routing: could not reap killed pactl child: {e}"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ReapedChild {
|
||||
fn reap_within(&mut self, budget: Duration) -> io::Result<Option<ExitStatus>> {
|
||||
let deadline = Instant::now() + budget;
|
||||
loop {
|
||||
if let Some(status) = self.child.try_wait()? {
|
||||
self.reaped = true;
|
||||
return Ok(Some(status));
|
||||
}
|
||||
if Instant::now() >= deadline {
|
||||
return Ok(None);
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(5));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn bounded_output(command: &mut Command, budget: Duration) -> io::Result<BoundedOutput> {
|
||||
command.stdout(Stdio::piped()).stderr(Stdio::piped());
|
||||
let mut child = ReapedChild {
|
||||
child: command.spawn()?,
|
||||
reaped: false,
|
||||
};
|
||||
let stdout = child
|
||||
.child
|
||||
.stdout
|
||||
.take()
|
||||
.ok_or_else(|| io::Error::other("pactl stdout was not piped"))?;
|
||||
let stderr = child
|
||||
.child
|
||||
.stderr
|
||||
.take()
|
||||
.ok_or_else(|| io::Error::other("pactl stderr was not piped"))?;
|
||||
let stdout_reader = std::thread::Builder::new()
|
||||
.name("pixelpass-pactl-stdout".to_string())
|
||||
.spawn(move || {
|
||||
let mut bytes = Vec::new();
|
||||
let mut stdout = stdout;
|
||||
stdout.read_to_end(&mut bytes).map(|_| bytes)
|
||||
})?;
|
||||
let stderr_reader = std::thread::Builder::new()
|
||||
.name("pixelpass-pactl-stderr".to_string())
|
||||
.spawn(move || {
|
||||
let mut bytes = Vec::new();
|
||||
let mut stderr = stderr;
|
||||
stderr.read_to_end(&mut bytes).map(|_| bytes)
|
||||
})?;
|
||||
|
||||
let deadline = Instant::now() + budget;
|
||||
let (status, timed_out) = loop {
|
||||
if let Some(status) = child.child.try_wait()? {
|
||||
child.reaped = true;
|
||||
break (status, false);
|
||||
}
|
||||
if Instant::now() >= deadline {
|
||||
let _ = child.child.kill();
|
||||
let Some(status) = child.reap_within(PACTL_REAP_BUDGET)? else {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::TimedOut,
|
||||
format!("pactl did not exit within {PACTL_REAP_BUDGET:?} after SIGKILL"),
|
||||
));
|
||||
};
|
||||
break (status, true);
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(5));
|
||||
};
|
||||
let stdout = stdout_reader
|
||||
.join()
|
||||
.map_err(|_| io::Error::other("pactl stdout reader panicked"))??;
|
||||
let stderr = stderr_reader
|
||||
.join()
|
||||
.map_err(|_| io::Error::other("pactl stderr reader panicked"))??;
|
||||
Ok(BoundedOutput {
|
||||
status,
|
||||
stdout,
|
||||
stderr,
|
||||
timed_out,
|
||||
})
|
||||
}
|
||||
|
||||
/// Async teardown: keep reconciling and unloading while a pass changes state.
|
||||
/// This replaces the arbitrary two-round count. With loads closed, the state
|
||||
/// graph is monotonic except for `Ambiguous(Unload) -> Loaded -> Ambiguous` when
|
||||
/// the same unload remains uncertain; that produces an identical snapshot and
|
||||
/// stops here for `--repair` rather than spinning.
|
||||
async fn cleanup_modules(ledger: &Arc<ModuleLedger>) {
|
||||
loop {
|
||||
let before = ledger.snapshot();
|
||||
if ledger.is_clean() {
|
||||
break;
|
||||
}
|
||||
if let Err(e) = ledger::reconcile_pending(ledger).await {
|
||||
tracing::warn!("audio routing: could not reconcile the module ledger: {e:#}");
|
||||
}
|
||||
for fp in ledger.loaded() {
|
||||
unload_module_inner(ledger, fp.shape, true).await;
|
||||
}
|
||||
if ledger.is_clean() || ledger.snapshot() == before {
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Synchronous teardown backstop. PulseSession bounds every connect/list/unload
|
||||
/// request, and `close_and_wait` has already drained any registered pactl worker.
|
||||
fn cleanup_modules_blocking(ledger: &Arc<ModuleLedger>) {
|
||||
loop {
|
||||
let before = ledger.snapshot();
|
||||
if ledger.is_clean() {
|
||||
break;
|
||||
}
|
||||
if let Err(e) = ledger::reconcile_pending_blocking(ledger) {
|
||||
tracing::warn!("audio routing: could not reconcile the module ledger: {e:#}");
|
||||
}
|
||||
for fp in ledger.loaded() {
|
||||
let permit = match ledger.begin_cleanup_unload(fp.shape) {
|
||||
Ok(Some(permit)) => permit,
|
||||
Ok(None) => continue,
|
||||
Err(e) => {
|
||||
tracing::warn!(
|
||||
shape = fp.shape.label(),
|
||||
"audio routing: refusing blocking module unload: {e}"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
finish_verified_unload(permit);
|
||||
}
|
||||
if ledger.is_clean() || ledger.snapshot() == before {
|
||||
break;
|
||||
}
|
||||
Ok(output) => UnloadOutcome::Uncertain(format!(
|
||||
"pactl unload-module exited {}: {}",
|
||||
output.status,
|
||||
String::from_utf8_lossy(&output.stderr).trim()
|
||||
)),
|
||||
Err(e) => UnloadOutcome::Uncertain(format!("could not run pactl unload-module: {e}")),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -835,6 +1072,7 @@ fn try_flush(
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::host::ledger::SlotState;
|
||||
use crate::repair::plan::{ModuleObservation, classify};
|
||||
|
||||
/// Whole-desktop routing: no app filter, so no PipeWire thread and no event
|
||||
@@ -868,6 +1106,41 @@ mod tests {
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normal_unload_requires_the_full_fingerprint_not_only_the_index() {
|
||||
fn observation(id: u32, pid: u32, nonce: u64) -> ModuleObservation {
|
||||
let token = repair_plan::OwnerToken {
|
||||
machine: "abc123".to_string(),
|
||||
boot: "def456".to_string(),
|
||||
pid_ns: 4_026_531_836,
|
||||
nonce,
|
||||
};
|
||||
ModuleObservation::new(
|
||||
id,
|
||||
Shape::LoopbackIntoCapture.module_name(),
|
||||
&repair_plan::recorded_argument(
|
||||
&Shape::LoopbackIntoCapture.render_args(pid, Some(&token)),
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
let ours = observation(5, 42, 7);
|
||||
let fp = classify(&ours).expect("the fixture is canonical");
|
||||
assert_eq!(
|
||||
unload_presence(&fp, std::slice::from_ref(&ours)),
|
||||
UnloadPresence::Exact
|
||||
);
|
||||
assert_eq!(unload_presence(&fp, &[]), UnloadPresence::Absent);
|
||||
|
||||
// Same live index, but another perfectly canonical host module. This is
|
||||
// the non-vacuous reuse case: an id-only normal unload would destroy it.
|
||||
let replacement = observation(5, 99, 8);
|
||||
assert_eq!(
|
||||
unload_presence(&fp, std::slice::from_ref(&replacement)),
|
||||
UnloadPresence::Replaced
|
||||
);
|
||||
}
|
||||
|
||||
/// A/B against the live graph: routing must leave the module table exactly
|
||||
/// as it found it. The same shape as `--repair`'s field gate, because the
|
||||
/// property is the same one — nothing of ours outlives the session.
|
||||
@@ -925,33 +1198,42 @@ mod tests {
|
||||
let task = tokio::spawn(async move {
|
||||
let _ = load_module(&ledger_for_task, Shape::LegacyCaptureSink, pid).await;
|
||||
});
|
||||
// Let the task run up to its first await — the spawned `pactl` — so the
|
||||
// abort lands mid-flight rather than before the load ever started, which
|
||||
// would make this gate vacuous.
|
||||
tokio::task::yield_now().await;
|
||||
// Wait until the affine permit is registered. A single yield is not a
|
||||
// scheduling guarantee and made the old version of this gate capable of
|
||||
// aborting before the task had started.
|
||||
tokio::time::timeout(PACTL_BUDGET, async {
|
||||
while matches!(ledger.state(Shape::LegacyCaptureSink), SlotState::Vacant) {
|
||||
tokio::task::yield_now().await;
|
||||
}
|
||||
})
|
||||
.await
|
||||
.expect("the load registers its operation");
|
||||
task.abort();
|
||||
let _ = task.await;
|
||||
|
||||
// Non-vacuity: the permit is taken *before* `pactl` is spawned, so a
|
||||
// cancelled load must leave a question behind whichever side of the spawn
|
||||
// the abort landed on. Without this the gate could pass while the abort
|
||||
// fired before the load ever began, proving nothing.
|
||||
// Cancellation detaches `spawn_blocking`; closing plus this registered-
|
||||
// operation wait is the ordering boundary that prevents reconciliation
|
||||
// from overtaking the worker's late server mutation.
|
||||
ledger.close();
|
||||
let ledger_for_wait = Arc::clone(&ledger);
|
||||
tokio::task::spawn_blocking(move || ledger_for_wait.wait_for_operations())
|
||||
.await
|
||||
.expect("the operation wait runs");
|
||||
|
||||
// The worker either committed a known module or left a reconciliation
|
||||
// question. Both are correct; vacancy here would mean the live operation
|
||||
// disappeared from the ledger.
|
||||
assert_eq!(
|
||||
ledger.pending().len(),
|
||||
ledger.pending().len() + ledger.loaded().len(),
|
||||
1,
|
||||
"the cancelled load must have left exactly one question behind"
|
||||
"the cancelled load must retain exactly one tracked outcome"
|
||||
);
|
||||
|
||||
ledger::reconcile_pending(&ledger)
|
||||
.await
|
||||
.expect("the ledger reconciles against the server");
|
||||
for fp in ledger.loaded() {
|
||||
unload_module(&ledger, fp.shape).await;
|
||||
}
|
||||
cleanup_modules(&ledger).await;
|
||||
|
||||
assert!(
|
||||
ledger.is_settled(),
|
||||
"every slot must end in a state we can explain"
|
||||
ledger.is_clean(),
|
||||
"every tracked module must be removed, not merely explained"
|
||||
);
|
||||
assert_eq!(
|
||||
module_snapshot(),
|
||||
|
||||
Reference in New Issue
Block a user