Skip to main content

max / alloy

Judge the helper-leak test on its own helper, not on every child The test compared the whole process's child set across converse, so any other polkit test's helper alive in that window read as this one's leak. The script now records its pid and the assertion is about that pid.
Author: Max Johnson <me@maxj.phd> · 2026-08-28 20:01 UTC
Signed with PGP, not checked
Commit: 4da96540596148b7a0e439e9e90c813232f8bd08
Parent: cdd4d8f
1 file changed, +30 insertions, -27 deletions
@@ -1583,14 +1583,22 @@
1583 1583 // happening to end.
1584 1584 #[test]
1585 1585 fn a_question_nobody_can_receive_still_closes_the_helper() {
1586 + // Writes its own pid before it prompts, so the assertion below is
1587 + // about this conversation's helper and not about whatever else the
1588 + // test binary happens to have running. `converse` cannot return until
1589 + // it has read the prompt line, so the file is on disk by then.
1586 1590 const SLEEPS: &str = "#!/bin/sh\n\
1587 1591 read -r cookie\n\
1592 + printf '%s\\n' \"$$\" > \"$0.seen\"\n\
1588 1593 printf 'PAM_PROMPT_ECHO_OFF Password: \\n'\n\
1589 1594 sleep 30\n\
1590 1595 printf 'FAILURE\\n'\n";
1591 1596
1592 1597 let helper = scripted_helper("undeliverable", SLEEPS);
1593 - let before = descendants();
1598 + let seen = helper.sidecar();
1599 + // A stale file from an earlier run would name a pid this test never
1600 + // spawned, so the read below is of this run or of nothing.
1601 + let _ = std::fs::remove_file(&seen);
1594 1602
1595 1603 let ask = // Exactly what `begin_authentication` passes when the receiver has
1596 1604 // gone: the console stopped listening.
@@ -1613,38 +1621,33 @@
1613 1621
1614 1622 // The child is closed out synchronously by `finish`, so by the time
1615 1623 // converse has returned there is nothing left to wait for.
1616 - let after = descendants();
1617 - let leaked: Vec<&u32> = after.iter().filter(|pid| !before.contains(pid)).collect();
1618 - assert!(
1619 - leaked.is_empty(),
1620 - "converse returned leaving {} helper process(es) behind: {leaked:?}",
1621 - leaked.len(),
1624 + let recorded = std::fs::read_to_string(&seen).expect("the helper recorded its pid");
1625 + let pid: u32 = recorded
1626 + .trim()
1627 + .parse()
1628 + .expect("the pid it recorded is a pid");
1629 + assert_ne!(
1630 + parent_of(pid),
1631 + Some(std::process::id()),
1632 + "converse returned leaving helper {pid} behind",
1622 1633 );
1623 1634 }
1624 1635
1625 - /// This process's children, by pid, from /proc.
1636 + /// The parent pid of `pid`, from /proc, or `None` if it is gone.
1626 1637 ///
1627 1638 /// Reads the process table rather than trusting a handle, because the thing
1628 1639 /// under test is precisely whether a handle was dropped without being
1629 - /// waited on. A zombie is still a child here, which is the point: an
1630 - /// unreaped helper is a leak even once it has stopped running.
1631 - fn descendants() -> Vec<u32> {
1632 - let me = std::process::id();
1633 - let Ok(entries) = std::fs::read_dir("/proc") else {
1634 - return Vec::new();
1635 - };
1636 - entries
1637 - .filter_map(|entry| {
1638 - let name = entry.ok()?.file_name();
1639 - let pid: u32 = name.to_str()?.parse().ok()?;
1640 - let stat = std::fs::read_to_string(format!("/proc/{pid}/stat")).ok()?;
1641 - // Field 4 is the parent pid; same hostile-comm rule as
1642 - // `start_time`, so read past the last `)`.
1643 - let after_comm = stat.rfind(')').map(|end| &stat[end + 1..])?;
1644 - let ppid: u32 = after_comm.split_whitespace().nth(1)?.parse().ok()?;
1645 - (ppid == me).then_some(pid)
1646 - })
1647 - .collect()
1640 + /// waited on. A zombie still answers here, which is the point: an unreaped
1641 + /// helper is a leak even once it has stopped running. Answering about one
1642 + /// named pid rather than about every child is what keeps a helper another
1643 + /// test spawned out of this one's verdict, since the whole binary is one
1644 + /// process.
1645 + fn parent_of(pid: u32) -> Option<u32> {
1646 + let stat = std::fs::read_to_string(format!("/proc/{pid}/stat")).ok()?;
1647 + // Field 4 is the parent pid; same hostile-comm rule as `start_time`, so
1648 + // read past the last `)`.
1649 + let after_comm = stat.rfind(')').map(|end| &stat[end + 1..])?;
1650 + after_comm.split_whitespace().nth(1)?.parse().ok()
1648 1651 }
1649 1652
1650 1653 // ---- the conversation registry ----