bluetoothd: wait for the daemon to exit before unmounting its storage

[mcfbytes]

start-stop-daemon -K only sends SIGTERM and returns; it does not wait. Stock's /bin/bluetoothd unmounts on the very next line, so the umount races a bluetoothd that still has /var/lib/bluetooth open — it fails EBUSY and leaves the loop mount up over a possibly-dirty ext4 image.

This is reachable from the OSD, not merely at shutdown. renew (Main:menu.cpp:7166, an ABI entry point per docs/abi-contract.md §7.6) runs stop → umount → rm $BTIMG → start, so losing the race deletes the image file while it is still loop-mounted. Stock hedges with fixed sleeps — sleep 2 in renew, sleep 1 in restart — which is a guess, not a wait.

The fix

Upstream Buildroot's, from package/bluez5_utils/S40bluetoothd's stop(): poll start-stop-daemon --stop --test until the process is gone, then remove the pidfile.

One change from upstream — ours is bounded at 50 × 0.1 s = 5 s, then unmounts anyway. Upstream's loop is infinite, which on this board would hang an OSD menu item forever if bluetoothd ever ignored SIGTERM. Giving up after 5 s is no worse than stock's behaviour today and correct in every other case.

This is the only deviation in the file. Every other line, and the whole start/stop/restart/reload/renew/hcireset verb set, remains stock's — so abi-contract S2 (the S45bluetooth → /bin/bluetoothd symlink shape, the 2 MiB ext4 loop mount) and §7.6 (hcireset/renew as Main_MiSTer entry points) are untouched. I confirmed our vendored copy still matched upstream byte-for-byte before patching, against a freshly fetched addon.tar.

Verified against stock's own binaries, not just ours

Upstream MiSTer builds on a much older Buildroot, so its BusyBox is a different vintage and its start-stop-daemon/sleep cannot be assumed to behave like ours. Stock's /usr/bin/busybox, extracted from rootfs.tar.bz2 in MiSTer-devel/Linux_Image_creator_MiSTer, is v1.33.1 (2022-12-24); ours is 1.38.0. Both run under qemu-arm:

	stock 1.33.1	ours 1.38.0
-K -t -q -p on a live pid	exit 0	exit 0
…on a reaped pid	exit 1	exit 1
…stderr on that probe	warning: killing process N: No such process	identical
sleep 0.1	works, ~0.1 s	works, ~0.1 s

So -t is in the base option string (not behind CONFIG_FEATURE_START_STOP_DAEMON_FANCY) and fractional sleep is built into both: the same code is valid on stock, which matters because the intent is to hand it upstream. The patched script also parses under both busybox sh -n.

The 2>/dev/null is load-bearing, on both, and it's the part that reading the source gets wrong. BusyBox's pidfile reader does not check liveness before building found_procs, so on the final probe the dead pid reaches the bb_perror_msg branch (:391) rather than the silent !G.found_procs path (:381-385), and that message is not gated on -q. An earlier draft of the doc claimed the opposite from a source read; running both binaries disproved it. Without the redirect, every stop/restart/renew would print a spurious warning.

Loop behaviour measured: dead pid → 0 iterations, 44 ms; a daemon that exits 0.6 s after SIGTERM → 5 iterations, 0.61 s; one that never exits → 50 iterations, then proceeds.

Docs

bluetooth-parity.md gains §3.1 with the cross-vintage table and the correction. Its "differences from stock: none found" claim and init-parity.md's "byte-identical" claim are updated to name this one deviation.

Upstream

The intent is to remove the deviation by fixing stock. Note for whoever does it: usr/bin/bluetoothd is not tracked as source upstream — it ships inside addon.tar, a binary blob committed to Linux_Image_creator_MiSTer (the repo tracks only tarballs plus create_img.sh). So the upstream route is a report against that repo, not an ordinary source PR.

Nothing has been sent upstream.

Not done here

No hardware test. The race is timing-dependent and I can't reproduce it on a board from here; the loop primitives are verified under emulation on both BusyBox vintages, but "OSD → Bluetooth → renew still works end to end" wants a real check before this is trusted.

🤖 Generated with Claude Code

https://claude.ai/code/session_014SBpBTbtBsFwjAgKCx5T9S

[Copilot]

🟡 Changes recommended

The new bounded-wait path can still remove the pidfile even if bluetoothd is still running, which can enable a subsequent start() to spawn a second daemon instance.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the MiSTer Bluetooth init control script to avoid racing umount /var/lib/bluetooth against a still-running bluetoothd, and documents the resulting (intended) divergence from stock behavior.

Changes:

• Add a bounded wait loop in stop() to poll start-stop-daemon --stop --test semantics before unmounting Bluetooth storage.
• Update parity documentation to record and explain the new stop() behavior and its rationale.
• Update the init-script parity table to reflect the single deliberate deviation from stock.

File summaries

File	Description
docs/init-parity.md	Updates the S45bluetooth row to describe the new bounded wait deviation and reference the detailed parity doc section.
docs/bluetooth-parity.md	Adds §3.1 documenting the stop-wait behavior and its cross-BusyBox validation notes.
board/mister/de10nano/rootfs-overlay/usr/bin/bluetoothd	Implements bounded polling in stop() before unmounting /var/lib/bluetooth.

Review details

• Files reviewed: 2/3 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[mcfbytes]

Tabled for now — moving to draft rather than closing, so the work and the cross-vintage verification stay available.

Rationale: PR #147 (the WiFi pre-up guard + the 91-ntp-kick dhcpcd hook) addresses the actual reported problems, and it independently speeds up Bluetooth init for WiFi users as a side effect, since S45bluetooth sat behind the 20 s wlan1 poll in S40network. That was the large, measurable Bluetooth delay.

What is left here is narrower: the stop() → umount race, which is a correctness bug reachable from the OSD renew path but not a boot-time delay. It stays parked until someone wants it, and it still needs a hardware check before it should be trusted.

Nothing has been sent to MiSTer upstream.