Survive logind RemoveIPC: recover when the cedar shmem segment vanishes - #548
Merged
Conversation
systemd-logind's default RemoveIPC=yes deletes every POSIX shared-memory segment the pifinder user owns the moment that user's last login session ends. An SSH logout is enough: the PiFinder services run as pifinder but hold no login session of their own. The cedar-detect server then can't open /cedar_detect_image (gRPC INTERNAL), and PFCedarDetectClient's recovery called the upstream _del_shmem, whose unlink() raised FileNotFoundError *before* _use_shmem could be disabled — so instead of falling back to inlining the image in the request, every subsequent solve repeated the crash until restart. Found via the battery-runtime bench campaign: all four discharge runs "lost solving at the cable pull", which was really the operator's SSH logout seconds beside it (~28k identical solver exceptions per night in the device logs, first hit 3 s apart on two units). - PFCedarDetectClient._del_shmem now treats an already-vanished segment as released, letting the same extract_centroids call retry with the image inlined — the frame that hits the error still gets centroids. - pifinder_setup.sh drops RemoveIPC=no into /etc/systemd/logind.conf.d so provisioned units also keep the fast shared-memory path. - Unit tests cover the vanished-segment release and the end-to-end fallback (real shm segment, faked gRPC stub). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fresh installs get the logind drop-in from pifinder_setup.sh; this delivers the same file to existing units on their next software update, via the standard one-shot migration mechanism. try-restart applies it without waiting for a reboot (logind restart does not end sessions, and RemoveIPC only acts at session end). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
|
can you asks your agent if this is also an issue in nixos? Some cedar protection has happened there but not sure if it covers this |
brickbots
marked this pull request as ready for review
July 25, 2026 22:35
The INTERNAL handler flipped _use_shmem off without a word, so the recovery this branch adds would have downgraded solving silently: the image goes inline over gRPC for the rest of the process and the only symptom is a slower extract time. That is the same blind spot that made the RemoveIPC bug take a four-run bench campaign to find -- the crash was at least loud. Warn at the flip point, naming the segment and pointing at the RemoveIPC=no drop-in, so a unit that missed the logind config says so in its logs. The flag latches, so this is one line per run, not per frame. The end-to-end test now asserts the warning is emitted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@mrosseel I believe that deleting the shared memory segments on logout is a standard logind behavior, so will probably also happen on NixOS, but the same fix should apply for NixOS if it uses logind/systemd 👍 |
The shmem request passed detect_hot_pixels; the non-shmem one silently dropped it. detect_hot_pixels is a proto3 bool, so an omitted field is false on the wire -- and per the proto's own comment, false means "hot pixels could be falsely detected as stars". PiFinder asks for true. Harmless while the fallback was unreachable (it raised instead of falling back), but this branch makes it reachable: after one SSH logout every remaining solve of the run would have hunted stars with hot-pixel rejection off. Losing the shared-memory handoff should cost throughput, not detection quality. The end-to-end test now asserts both requests carry the flag; without the kwarg the retry serializes with the field absent and it fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
systemd-logind's default
RemoveIPC=yesdeletes every POSIX shared-memory segment a user owns the moment that user's last login session ends. A plain SSH logout is enough — the PiFinder services run aspifinderbut hold no login session of their own. When that happens,/dev/shm/cedar_detect_imagevanishes under the running solver:INTERNALPFCedarDetectClient.extract_centroids's recovery path calls the upstream_del_shmem(), whoseunlink()raisesFileNotFoundErrorbefore_use_shmem = Falseis reachedAny field user who SSHes into their unit and logs out silently loses plate solving.
How it was found
The battery-runtime bench campaign: all four discharge runs across both campaigns "lost camera solving at the cable pull" — which turned out to be the operator's SSH logout seconds beside the pull. Device logs show ~28,000 identical solver exceptions per night (
FileNotFoundError: '//cedar_detect_image'+ servererrno 2), first occurrence 3 s apart on two independent units. This also retro-explains campaign 1's degraded runs, which had been attributed to IMU pseudo-motion blanking.The fix (three layers)
PFCedarDetectClient._del_shmemoverride: a segment already gone from/dev/shmis the method's goal state — treat it as released. The sameextract_centroidscall then retries with the image inlined, so even the frame that hits the error still gets centroids.pifinder_setup.sh(fresh installs): dropRemoveIPC=nointo/etc/systemd/logind.conf.d/so provisioned units keep the fast shared-memory handoff instead of falling back after the first logout.v2.6.1(existing units): delivers the same logind drop-in on the next software update via the standard one-shot mechanism inpifinder_post_update.sh, with atry-restartof logind so it applies without waiting for a reboot (logind restart does not end sessions, and RemoveIPC only acts at session end). Naming note: the migration marker is just a label — if the next release ships under a different number, feel free to renamev2.6.1before merge.Tests
python/tests/test_solver_cedar_client.py:INTERNALthen succeeds — asserts recovery in the same call,_use_shmemdisabled, pixels inlined in the retryVerified the test guards a real regression: with
_del_shmemrebound to the upstream implementation the same scenario raisesFileNotFoundErrorand never falls back.Full suite: 991 unit+smoke passed; ruff + mypy clean; both shell scripts pass
bash -n.🤖 Generated with Claude Code