thingino-motors: fix S59motor stop path wedging motors-daemon - #1511
Open
catongates wants to merge 1 commit into
Open
thingino-motors: fix S59motor stop path wedging motors-daemon#1511catongates wants to merge 1 commit into
catongates wants to merge 1 commit into
Conversation
stop_daemon() could never actually stop motors-daemon: the daemon self-daemonizes under -d (double-fork + setsid), so the PID start-stop-daemon records via -m is the intermediate child, which exits immediately -- the pidfile is stale from the moment it is written. stop() didn't check stop_daemon's result and proceeded to rmmod anyway, which succeeds against a live daemon because motor_fops lacks .owner = THIS_MODULE (module use count stays 0 with a live opener; filed separately as themactep/ingenic-sdk#45). That leaves motors-daemon in permanently uninterruptible D state, immune to SIGKILL, recoverable only by a power cycle. Fix: stop_daemon now matches the real process (anchored on the binary path, since an unanchored pattern also matches the calling shell) and escalates SIGTERM -> SIGKILL with a bounded wait, returning a real status. stop() now refuses to rmmod unless the daemon is confirmed gone, converting the failure mode from an unrecoverable wedge into an ordinary loud error. Tested live on an Eufy E220 (T8410C, T31X): the exact restart that previously wedged the daemon (reproduced on both legacy and profiled motion_driver, so unrelated to acceleration settings) now completes cleanly and leaves the motor responsive. Fixes themactep#1510.
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.
Fixes #1510.
What's wrong
S59motor'sstop_daemoncould never actually killmotors-daemon: the daemon self-daemonizes under-d(double-fork +setsid), so the PIDstart-stop-daemon -mrecords is the intermediate child, which exits immediately — the pidfile is stale from the moment it is written.stop()didn't check the result and ranrmmod motoranyway, which succeeds against the still-live daemon becausemotor_fopslacks.owner = THIS_MODULE(module use count stays 0 with a live opener — filed separately as themactep/ingenic-sdk#45). That leavesmotors-daemonin permanently uninterruptibleDstate, immune toSIGKILL, recoverable only by a power cycle.Full detail and live reproduction in #1510.
What this changes
stop_daemonnow matches the real process (anchored on the binary path — an unanchoredpkill -fpattern also matches the invoking shell) and escalatesSIGTERM→SIGKILLwith a bounded wait, returning a real status.stop()now refuses tormmodunless the daemon is confirmed gone, turning the failure mode from an unrecoverable wedge into an ordinary loud error.Both changes are scoped to
stop_daemon()/stop(); nothing else in the file is touched.Testing
Live on an Eufy E220 (T8410C, T31X,
ciao+f433166): the exactS59motor restartthat previously wedged the daemon now completes cleanly, with the daemon healthy afterward andmotors -jresponsive. Reproduced (and now fixed) on bothmotion_driver: legacyandprofiled, so this is unrelated to the acceleration settings, which were the original — wrong — suspect.Investigated, tested, and drafted by an AI assistant while looking into PTZ motion behavior on this camera.