deps: uv: restore prior signal disposition instead of SIG_DFL (DCP-4748)#4
Open
philippe-distributive wants to merge 1 commit into
Open
deps: uv: restore prior signal disposition instead of SIG_DFL (DCP-4748)#4philippe-distributive wants to merge 1 commit into
philippe-distributive wants to merge 1 commit into
Conversation
libuv's uv__signal_unregister_handler() reset the disposition of a signal
to SIG_DFL once its last watcher was removed. On Android this is intercepted
by ART's libsigchain, which logs an ERROR-level stack trace ("Setting SIGSEGV
to SIG_DFL ...") and can destabilise ART's signal handling, because ART claims
SIGSEGV/SIGBUS/SIGFPE/SIGILL/SIGTRAP/SIGABRT for implicit null checks, stack
overflow detection, etc. On an embedded worker this fires on every
node::FreeEnvironment() during CleanupHandles -> uv_close on a UV_SIGNAL
handle (DCP-4748).
Resolve libuv's own long-standing "XXX save old action so we can restore it
later on?" TODO: uv__signal_register_handler() now captures the previous
disposition on the genuine none->installed transition (gated by a `save`
flag so the oneshot re-registration paths don't clobber it), and
uv__signal_unregister_handler() restores that instead of forcing SIG_DFL,
falling back to SIG_DFL only when nothing was captured. Platform-agnostic and
strictly more correct than a blind reset; on Android it restores ART's handler
so no SIG_DFL reset (and no libsigchain report) occurs.
Signed-off-by: Philippe Laporte <philippelaporte1@gmail.com>
Author
|
fixed upstream in libuv/libuv#5157 |
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.
Summary
When a Node environment is torn down inside an Android app process
(
node::FreeEnvironment()→uv_close()on aUV_SIGNALhandle), libuvresets the signal's disposition to
SIG_DFL. On Android thatsigactioncall is intercepted by ART's
libsigchain, which logs an ERROR-levelstack trace on every worker stop:
ART claims SIGSEGV/SIGBUS/SIGFPE/SIGILL/SIGTRAP/SIGABRT for implicit
null-checks, stack-overflow detection, etc., so resetting one to
SIG_DFLis also semantically wrong, not just noisy.
Root cause
uv__signal_unregister_handler()unconditionally installsSIG_DFLoncethe last watcher for a signal is removed. libuv never recorded what was
installed before it added its multiplexer — there's a long-standing
/* XXX save old action so we can restore it later on? */TODO at theinstall site to prove it.
Fix
Resolve that TODO.
uv__signal_register_handler()now captures theprevious
struct sigaction(straight into a per-signum slot) on thegenuine "no libuv handler → installed" transition, gated by a
saveflag so the oneshot↔regular re-registration paths never overwrite it
with libuv's own handler.
uv__signal_unregister_handler()restores thatsaved disposition instead of forcing
SIG_DFL, falling back toSIG_DFLPlatform-agnostic and strictly more correct than a blind reset; on Android
the restored handler is ART's (kept by libsigchain), so no
SIG_DFLreset— and no libsigchain report — occurs. Zero hot-path cost (only runs on the
first register / last unregister per signum); the saved table is touched
only under the existing global signal lock.
Alternatives rejected
#ifdef __ANDROID__— Android-specific andleaves libuv's multiplexer chained behind ART forever.
rt_sigaction(SIG_DFL)syscall to bypass libsigchain — unsafe:it overwrites libsigchain's kernel-level handler (the one ART chains
behind), silencing the log by breaking ART's signal handling.
Testing
we_get_signals_mixed(the oneshot↔regular path thesavegateguards),
signal_pending_on_close,signal_close_loop_alive.libnode.sofor all four Android ABIs (arm64-v8a,x86_64, armeabi-v7a, x86).
libsigchain "Setting SIGSEGV to SIG_DFL"count 1 → 0, with the cleanup pathconfirmed exercised (
FreeEnvironmentruns,libuv loop closed cleanly).Upstreaming
deps/uvhere is byte-identical to upstream libuv v1.51.0, so this is aclean candidate for an upstream libuv PR (any embedder benefits).
Rollout
Cut a tag (e.g.
dcp/3.2.1) → bumpnode-build'sGIT_TAG→ rebuild theAndroid
node_apizips → bump the URL/SHA indcp-native/externals/node_api/CMakeLists.txt→ drop the rebuiltlibnode.sointoandroid-workerjnilibs/.