Fix SIGSEGV on failed startup: do not join the api service thread when it was never started - #912
Open
Buell (JanZachmann) wants to merge 1 commit into
Conversation
uninit_api_svc() joined g_api_svc_thread unconditionally. The handle is zero-initialized and only set by init_api_svc(), so a shutdown that never started the service passed a NULL thread descriptor to pthread_join(). On aarch64 glibc that dereferences it and the process dies with SIGSEGV (fault address 0xd0, inside the join implementation). x86 glibc happens to return ESRCH instead of faulting, so the crash only shows on arm. The agent hits this on every failed startup: HealthCheck() fails, main() goes to done, ShutdownAgent() calls uninit_api_svc() although StartupAgent() never ran. The unit restarts the agent after the failure, so the crash repeats until the systemd start limit stops it, e.g. for as long as the identity service cannot hand out connection info. Take the running flag with atomic_exchange and skip the teardown when it was already false. This also makes a second uninit a no-op, matching the guard the timer thread already has in AducTimer_Stop(). Add regression coverage: the tests interpose pthread_join and assert that no join is attempted on the zero handle, before init and after a completed uninit. Signed-off-by: Jan Zachmann 50990105+JanZachmann@users.noreply.github.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.
Fixes #911
What
uninit_api_svc()joinedg_api_svc_threadunconditionally. The handle iszero-initialized and only set by
init_api_svc(), so a shutdown that never startedthe service passed a NULL thread descriptor to
pthread_join(). On aarch64 glibcthat dereferences it and the agent dies with
SIGSEGV(fault address0xd0, insidethe join implementation); x86 glibc happens to return
ESRCH, so the crash onlyshows on arm.
The agent hits this on every failed startup:
HealthCheck()fails,main()goes todone,ShutdownAgent()callsuninit_api_svc()althoughStartupAgent()neverran. The unit restarts the agent after the failure, so the crash repeats until the
start limit stops the service.
Fix
Take the running flag with
atomic_exchange()and skip the teardown when it wasalready false. A second
uninit_api_svc()is a no-op too, matching the guard thetimer thread already has in
_stop_thread()(timer_utils/src/timer.c).Tests
apisvc_unit_tests.cpp: "uninit without init" and "uninit twice".They interpose
pthread_join()(real handles forwarded to libc viadlsym(RTLD_NEXT, ...)) and assert that no join is attempted on the zero handle.A plain return-value check would not catch the bug on x86, where the faulty join
just returns
ESRCH.apisvc_unit_tests: 26/26 assertions pass with the fix; 2 assertions fail withoutit.
ctestrun ondevelop: the failing-test set is identical before and afterthis change (80 pre-existing failures in my container setup, none in apisvc).
Root-caused from a symbolized device core; details in #911.