flipper-btrfs.sh: clean up on signals, early exits and stale lock lines by Yury-MonZon · Pull Request #143 · flipperdevices/flipperone-linux-build-scripts

Yury-MonZon · 2026-07-29T16:04:16Z

Four defects in how the shared lib manages its mount and its lock. The first matters most.

The mount sat in the directory everybody sweeps. mount_top used a bare mktemp -d, so the
filesystem lived at /tmp/tmp.XXXX. Any rm -rf /tmp/tmp.* while a tool held its mount recursed
through the whole filesystem, and because rmdir removes an empty subvolume, it emptied and then
deleted profiles, @home and boot, leaving only the read-only _stock bases. The mount now
lives at /run/flipper-btrfs.mnt.XXXXXX, which no glob sweeps, with a fallback if /run is not
writable.
A signal left the mount behind for the rest of the uptime, because the cleanup hung off a
plain trap ... EXIT, which dash runs on exit but not on a signal. INT, TERM and HUP now
route through exit. The first umount also raced the dying children and failed silently, so it
retries and then detaches lazily.
mount_top armed cleanup after its own checks, so dying on a bad -d left the caller's temp
files behind: one leak per invocation in a recovery boot.
The lock file kept a dead holder's pid line. The flock was never stale, only the label was, so
a waiter could name a long-gone pid. It is checked with kill -0 now and blanked on exit. The file
is truncated rather than unlinked, since deleting it lets two processes lock different inodes.

Split out of the closed #141. Only the combined final state ran on hardware. The mountpoint fix has
a regression test that runs the glob against a sandbox and asserts nothing changed.

The -y/-d help lines were repeated verbatim in every tool usage(); keep them in HELP_YES/HELP_DEVICE in the shared library so the wording cannot drift. Likewise the -d/--device argument check (fail with usage if the flag has no value) becomes one helper, need_device_arg, instead of the same guard pasted into ten arg loops.

The cleanup hung off a plain EXIT trap, which dash runs on exit but not on a signal, so any interrupted tool left its subvolid=5 mount and temp directory behind for the rest of the uptime. A test run here accumulated seventeen such directories and several live mounts. INT, TERM and HUP now route through exit so the one cleanup path runs. That alone was not enough: on a signal the children (btrfs send/receive, zstd) are still dying, so the first umount can lose the race and fail with EBUSY, which the old code discarded silently. top_cleanup now retries briefly and falls back to a lazy detach, after which a group SIGTERM during a send leaves no mount and no temp directory. Note for anyone aborting a long send: signal the process group, as Ctrl-C does. Killing only the tool's pid leaves its btrfs send running, and the shell cannot run the trap until that child finishes, so the mount and the lock stay held until then.

mount_top installed the EXIT trap only after its checks passed, so dying on a bad -d argument or a missing root device left the caller's TOP_TMPFILES behind. Reproducible on the device: list-profiles -d /etc/hostname leaked two empty temp files per invocation, and the non-destructive suite noticed a pile of them. Arming the trap first fixes it, and costs nothing when there is nothing to clean: top_cleanup already guards on an unset TOP.

The flock itself was never the problem. It belongs to the open file description rather than to the process or the file, so the kernel releases it once the last descriptor referring to it is closed, which a dying tool does even under kill -9. What outlived the process was the human-readable "PID 123 (tool)" line that set_lock writes so a waiter can name who it is waiting for. That is ordinary file data in a tmpfs and nothing cleans it up, so a waiter could report a pid that had been gone for days. It could never block on one, since the line and the lock are independent, which makes this a reporting bug rather than a correctness one. Two halves: a waiter now checks the recorded pid with kill -0 and says the line is stale when that process is gone, and a tool blanks its own line on the way out, so only a crash can leave one behind at all. The file is truncated rather than unlinked on purpose. Deleting a lock file races: one process can hold a lock on the old inode while another locks a freshly created one at the same path, and both then believe they have exclusion. Worth knowing when reading this code: descriptors are inherited, so during a send the tool's shell, btrfs send, zstd and pv all hold fd 9. Killing the shell alone leaves the lock held until those children exit. That is intended, since the lock should cover the work in flight, but it does mean a hung child holds it indefinitely.

mount_top used a bare mktemp -d, which yields /tmp/tmp.XXXX. `rm -rf /tmp/tmp.*` is a common cleanup idiom, and running it while any tool holds its mount recurses through the entire filesystem: since Linux 4.18 rmdir removes an empty subvolume, so rm empties each subvolume and then deletes it, taking the profiles, @home and boot with it. Read-only subvolumes survive, which is why the _stock bases end up being the only thing left. A tool one careless glob away from deleting the user's profiles should not keep its mount in the directory everybody sweeps. /run is tmpfs, root-only, and not swept by glob. It falls back to the old location when /run is not writable, so nothing breaks in an unusual environment.

…e trap top_cleanup blanks the lock file before it unmounts, and ':' is a special builtin: when the lock path is not writable the failed redirect exits the shell outright, the unmount never runs, and the top-level mount stays behind for the rest of the uptime. '|| true' cannot catch that, since the shell is gone before any command runs, so the truncation moves into a subshell. The other four are ordinary utilities, where nothing is fatal and appending '|| true' is the whole fix: the loader-entry sweep stopped at the first entry when /boot was read-only and left the later ones behind, top_cleanup's own tmpfile removal took the trap's exit status with it, and create-profile's rollback let a failed mv pre-empt its own die, so a user whose snapshot failed was told nothing at all. A false test never aborted any of these; only a failing action did. Measured both ways against a read-only sandbox filesystem on the device.

set_lock ran before anything else and stayed held for the whole send, though the only thing here that mutates is the read-only snapshot taken of a read-write source. That blocked every writer for the length of a multi-GB send, and it deadlocked the tool's own documented pipeline: with 'send-snapshot ... - | receive-snapshot -' on one machine, the receiving side holds the lock while it blocks on stdin and the sending side blocks acquiring it, so nothing ever moves. The lock is now taken only around that snapshot and released before streaming, so a send that mutates nothing never takes it at all. A read-write source still needs it briefly, so set_lock also refuses to wait on a holder at the other end of our pipe, which can only be waiting for us, and dies with what to do instead. The test is the pipe rather than the process group: sibling jobs started from one shell share a stdin and must simply wait for each other. Measured on the device with a tiny source, so throughput plays no part. Before, a _stock, a ro snapshot and a rw subvolume all hung indefinitely through a local pipe. After, the first two complete and land correctly routed, the third exits 1 naming the holder, and a backgrounded sibling holding the lock for 8s is waited out and then succeeds.

The flock lives on the open file description and children inherit fd 9, so anything that outlives the tool keeps the lock held with no owner. Not theoretical: a killed send left an orphaned 'flock 9' holding it for 38 minutes here, and every writer tool would have blocked until reboot. The streaming pipeline now runs with 9>&-. Measured with a real 3.6 GB stream: before, killing the lock holder left three heirs and the lock stayed held; after, only the tool itself holds it and the lock is free the moment it dies.

…partial btrfs receive fails on the stream's first command when the name is already taken. For a file source the name is known before the transfer starts, so the collision is reported there, with the name and what to do about it. A piped stream carries no name to check yet. A received subvolume is made read-only only once the whole stream is in, so an interrupted receive leaves a writable partial under the sent name and the next attempt collides with it. The failure path removes it, and says so. Only while it is still writable: the lock keeps our tools out for the whole receive but not a hand-run btrfs command, and a read-only subvolume under that name is a finished one, not ours. Where no name could be read, the entry count says whether something was left behind. Verified on Flipper One: a truncated stream left a 39M writable @snapshots/<name> behind.

Yury-MonZon force-pushed the btrfs-tools-cleanup-on-exit branch from c712be3 to 729593f Compare July 31, 2026 15:14

Yury-MonZon force-pushed the btrfs-tools-cleanup-on-exit branch from 729593f to 85acf33 Compare August 6, 2026 09:31

Yury-MonZon force-pushed the btrfs-tools-cleanup-on-exit branch from 85acf33 to 7422475 Compare August 6, 2026 10:19

Yury-MonZon marked this pull request as ready for review August 6, 2026 10:22

Yury-MonZon requested a review from a team August 6, 2026 10:22

Yury-MonZon force-pushed the btrfs-tools-cleanup-on-exit branch from 7422475 to 9b15c28 Compare August 7, 2026 09:33

Base automatically changed from btrfs-tools-shared-helpers to dev August 10, 2026 07:57

alchark force-pushed the btrfs-tools-cleanup-on-exit branch from 9b15c28 to 5da0f84 Compare August 10, 2026 07:57

Yury-MonZon added 9 commits August 10, 2026 13:39

Yury-MonZon force-pushed the btrfs-tools-cleanup-on-exit branch 2 times, most recently from 6530226 to 773611f Compare August 10, 2026 12:39

alchark approved these changes Aug 11, 2026

View reviewed changes

alchark merged commit 936822d into dev Aug 11, 2026

alchark deleted the btrfs-tools-cleanup-on-exit branch August 11, 2026 08:24

Navigation Menu

flipper-btrfs.sh: clean up on signals, early exits and stale lock lines - #143

flipper-btrfs.sh: clean up on signals, early exits and stale lock lines#143
alchark merged 9 commits into
devfrom
btrfs-tools-cleanup-on-exit

Yury-MonZon commented Jul 29, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Yury-MonZon commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Yury-MonZon commented Jul 29, 2026 •

edited

Loading