Skip to content

fix: keep Telegram receiver locks valid across exec - #113

Merged
Freudator86 merged 3 commits into
mainfrom
fm/fm-tg-recv-arm-identity-race
Aug 16, 2026
Merged

fix: keep Telegram receiver locks valid across exec#113
Freudator86 merged 3 commits into
mainfrom
fm/fm-tg-recv-arm-identity-race

Conversation

@Freudator86

Copy link
Copy Markdown
Collaborator

Intent

Fix a process-identity race in bin/fm-tg-recv-arm.sh that makes a healthy direct-Telegram receiver report FAILED forever.

The wrapper forked the receiver and fingerprinted it two lines later with fm_pid_identity, which mixes the process start time with the process image (cmdline). At that instant the child is still walking its shebang chain, so the recorded image is one that ceases to exist as soon as the chain's last execve lands. The identity written into the lock therefore described a process image that no longer existed, and the comparison could never match again for the life of that receiver. The consequence is worse than a wrong string: every later arm reports FAILED against a healthy receiver, and firstmate's operating instructions define FAILED as an alarm to repair before the turn ends - so the false alarm points at a 'repair' that kills a working receiver. One home has already had that accident.

The captain left the choice of fix open but required that it close the race rather than narrow it, and explicitly ruled out a fixed sleep as a non-fix. Three options were weighed and the choice is deliberate: fingerprint by something that survives the exec. Waiting for a stable image was rejected because the wrapper cannot derive a termination condition - it does not know the receiver's interpreter, so it cannot tell the shebang chain's last hop from an intermediate one. Having the child report its own settled identity was rejected because it needs a shim process between wrapper and receiver, which buys correctness at the price of a new failure mode: kill the shim and the recorded pid is dead while the receiver lives, so a later arm starts a second receiver.

So bin/fm-wake-lib.sh now separates the two things fm_pid_identity had merged. fm_pid_incarnation returns only what tells a process apart from a later one reusing its pid - Linux /proc stat field 22, or ps lstart on the portable fallback - which is fixed at fork and unchanged by every execve, so it is already correct for a child just forked. fm_pid_identity keeps returning the incarnation plus the image, with byte-identical output (verified against the previous implementation on a live process), and now carries a warning naming exactly this trap. Every other caller of fm_pid_identity in the repo fingerprints itself ($$ or BASHPID) or reads back an already-settled process, which is where it is sound; the receiver arm was the unique site fingerprinting a just-forked child.

The receiver lock now records pid-incarnation instead of pid-identity. fm_pid_incarnation_matches_record also accepts a record written as a full identity, because the incarnation is that record's leading fields - so a lock left behind by the older wrapper, whose image half has since exec'd away, still names a receiver this wrapper can confirm rather than raising the same false alarm once on update. This backwards compatibility is deliberate and tested, because the un-compatible version would reproduce the exact accident being fixed during the update itself.

One trade-off is accepted knowingly and is stated in the commit message: dropping the image from this one comparison gives up the tick-collision tiebreak that the old comment called out. A false match now needs pid reuse AND the same 10ms start tick, on top of the lock's existing fm-home and receiver-path bindings. That residual is negligible next to a false FAILED that is certain, permanent, and points at killing the receiver. This weakening is confined to the receiver arm; the watcher, delivery listener, and away-mode daemon locks still compare full fm_pid_identity.

Failure mode required by the captain: if the incarnation genuinely cannot be established, the wrapper refuses and says so ('telegram receiver: FAILED - could not identify receiver process') and leaves no lock behind - never a silent pass, and never a FAILED against a healthy receiver. That refusal is covered by a new test.

The captain also required a test that fails against the old code and exercises the exec transition rather than asserting on an already-settled process, and warned that this home is not currently affected so 'it works here' is not evidence. The race was therefore reproduced deliberately first: a receiver reached through a slow interpreter hop showed recorded cmdline-hex 'bash ' against live 'bash ' with an identical starttime, and the re-arm printed the exact reported FAILED line. The tests use that same fixture - tests/fm-tg-recv-arm.test.sh asserts the receiver actually left its interpreter image before re-arming three times plus once against a legacy-format record, and tests/fm-watcher-lock.test.sh drives a real gated exec and asserts the incarnation is stable across it while the identity is not. Both were verified to fail against the pre-fix code with the reported symptom.

Deliberate scope decisions a reviewer would not see in the diff: no docs/ change, because no doc referenced the lock's identity file (checked) and the firstmate coding guidelines route exact mechanics to script headers, which is where the explanation went; no AGENTS.md change, for the same reason. The comment on fm_pid_identity is load-bearing - it is the durable guard against a future caller repeating this - and is not decoration.

Verification run before committing: bin/fm-lint.sh clean on all four changed files, and nine suites green (fm-tg-recv-arm, fm-watcher-lock, fm-delivery, fm-daemon, fm-guard-stale-banner, fm-turnend-guard, fm-continuity-pretool-check, fm-pr-check-security, fm-afk-launch). One pre-existing failure in tests/fm-watcher-lock.test.sh (test_guard_warnings) also fails on the base commit and is unrelated to this change.

What Changed

  • Split exec-stable process incarnation data from image-sensitive process identity helpers.
  • Record Telegram receiver incarnations in locks and accept legacy full-identity records when reattaching.
  • Add exec-transition, repeated re-arm, legacy-lock, and identification-failure regression coverage, and update lock metadata documentation.

Risk Assessment

✅ Low: The change durably closes the exec-transition race by matching the receiver on fork-stable incarnation, preserves legacy lock compatibility, confines the accepted collision trade-off to this receiver lock, and adds behavioral regression coverage for the required failure paths.

Testing

The supplied baseline verification was reviewed, both focused automated suites passed, and a manual end-user CLI run demonstrated that a receiver survives its exec transition, re-arms attach to the same process without duplication, and cleanup removes the lock; no actionable issues were found.

Evidence: Receiver exec-transition and re-arm transcript

--- initial arm --- telegram receiver: started pid=2269914 --- re-arm after final exec --- telegram receiver: attached pid=2269914 --- persisted receiver starts --- 1 --- lock after receiver exit --- absent

--- initial arm ---
telegram receiver: started pid=2269914
--- re-arm after final exec ---
telegram receiver: attached pid=2269914
--- persisted receiver starts ---
1
--- lock after receiver exit ---
absent
Evidence: Process-incarnation and watcher-lock test log
ok - simultaneous watcher starts leave exactly one live process
ok - fm_pid_identity is locale-invariant across LC_ALL/LC_TIME
ok - process incarnation survives the exec that replaces the process image
ok - incarnation matching accepts a full-identity record without accepting a different process
ok - Linux process identity ignores simulated btime changes
ok - Linux process identity detects pid reuse
ok - killed watcher stale lock is reclaimed
ok - live watcher lock with stale heartbeat is actionable
ok - guard banner leads when down with pending wakes, stays silent when fresh, and names a down delivery listener with a healthy daemon
ok - lock publication failure returns promptly without entering steal recursion
ok - lock steal recursion stops at its configured hard depth bound
ok - lock wait returns a clear error at its configured total-time bound
ok - concurrent fm_lock_try_acquire yields exactly one winner
ok - dead-pid stale lock is reclaimed by a single acquirer
ok - concurrent stale-lock steal yields exactly one winner
ok - live steal mutex is not reclaimed
ok - live-held lock is not stolen
ok - empty mid-acquire lock keeps a minimum grace
ok - late original claimant cannot claim a recreated lock
ok - paused mid-acquire claimant backs off to active stealer
ok - watcher self-evicts when the lock pid no longer names it
ok - watcher stays owned across a failed/empty ps parent-pid read

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • Inspected git diff --unified=40 8725f8124d786b1f4cdbf54ab25153d40d8bb412 b801beeadcf8d833a31013aaecaee8e2ca9bc213 -- bin/fm-wake-lib.sh bin/fm-tg-recv-arm.sh tests/fm-tg-recv-arm.test.sh tests/fm-watcher-lock.test.sh
  • tests/fm-tg-recv-arm.test.sh — exercised delayed shebang exec, three healthy re-arms, legacy full-identity lock compatibility, duplicate prevention, identification failure, and lock cleanup
  • tests/fm-watcher-lock.test.sh — exercised a gated real exec, stable incarnation versus changing identity, legacy-record matching, PID reuse, and existing lock behavior
  • Manual delayed-interpreter receiver run captured in receiver-exec-rearm-transcript.txt — observed started, then attached to the same PID, exactly one receiver start, and no lock after exit
  • Reviewed the author-supplied pre-commit verification context; lint was not repeated because this test phase forbids linters
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Bridge CI added 3 commits August 16, 2026 21:50
bin/fm-tg-recv-arm.sh forked the receiver and fingerprinted it two lines
later with fm_pid_identity, which mixes the process start time with the
process image. At that instant the child is still walking its shebang
chain, so the recorded image is one that ceases to exist as soon as the
chain's last execve lands. The identity in the lock therefore described a
process image that no longer existed, and the comparison could never match
again for the life of that receiver.

The consequence was worse than a wrong string: every later arm reported
FAILED against a healthy receiver, and the emitted operating instructions
define FAILED as an alarm to repair before the turn ends - so the false
alarm pointed at a "repair" that kills a working receiver. One home has
already had that accident.

Of the three ways to close this, fingerprinting by something that survives
the exec is the one that removes the race rather than narrowing its window.
Waiting for a stable image needs a termination condition the wrapper cannot
derive, because it does not know the receiver's interpreter and so cannot
tell the chain's last hop from an intermediate one. Having the child report
its own settled identity needs a shim process between the wrapper and the
receiver, which buys correctness at the price of a new failure mode: kill
the shim and the recorded pid is dead while the receiver lives, so a later
arm starts a second one.

So fm-wake-lib.sh now separates the two things fm_pid_identity had merged:

  fm_pid_incarnation  what tells this process apart from a later one that
                      reuses its pid - Linux stat field 22, or ps lstart -
                      fixed at fork and unchanged by every execve, so it is
                      already correct for a child just forked.
  fm_pid_identity     the incarnation plus the image, unchanged in output.
                      It now carries a warning naming exactly this trap,
                      and every other caller fingerprints itself (or reads
                      back a settled process), which is where it is sound.

The receiver lock records pid-incarnation. fm_pid_incarnation_matches_record
also accepts a record written as a full identity, because the incarnation is
that record's leading fields - so a lock left by the older wrapper, whose
image half has since exec'd away, names a receiver this one can still
confirm rather than raising the same false alarm once on update.

Dropping the image from this one comparison gives up the tick-collision
tiebreak: a false match now needs pid reuse AND the same 10ms start tick,
on top of the lock's existing home and receiver-path bindings. That residual
is negligible next to a false FAILED that is certain, permanent, and points
at killing the receiver.

When the incarnation genuinely cannot be established the wrapper still
refuses and says so, rather than recording a lock nothing can confirm.

Tests drive the exec transition itself - a receiver behind a deliberately
slow interpreter hop, asserted to have left that image before the re-arms -
because a settled process is precisely the case that hides this defect. Both
new tests fail against the previous code with the reported symptom.
@Freudator86
Freudator86 merged commit ac2c943 into main Aug 16, 2026
10 checks passed
@Freudator86
Freudator86 deleted the fm/fm-tg-recv-arm-identity-race branch August 16, 2026 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant