Repository navigation
Conversation
sigchain_push()/sigchain_pop() update signals[sig].n and the handler stack non-atomically. When two copies of the same signal are delivered in quick succession to different threads, both instances of the handler run the usual "cleanup; sigchain_pop(sig); raise(sig)" pattern concurrently, and both read the same s->n before either one decrements it. Each then installs s->old[s->n - 1], so a two-entry chain unwinds both entries while the dispatcher keeps pointing at a handler that was already popped: n ends at 0 while the effective disposition is still old[1]. The surviving dispatcher then runs that popped handler, whose own sigchain_pop() hits the "n < 1" guard and does nothing, and the raise() at the end redelivers to the same stuck dispatcher forever. The process neither dies nor continues; every signal-handling thread spins at 100% CPU and only SIGKILL recovers it. This is reachable in practice: "git clone --depth 1" over ssh holds two SIGTERM entries in the chain at the index-pack parking point (clone's remove_junk_on_signal, then tempfile's remove_tempfiles_on_signal from the first lockfile), and two SIGTERMs a fraction of a millisecond apart land on two threads and corrupt the chain. Rather than serializing the chain mutation, teach the "n < 1" guard in sigchain_pop() to repair the damage. Track how many pushes were ever performed for the signal; when a pop finds the chain empty but the chain bottom is known, reinstall s->old[0], the disposition that was in place before the chain was built. The stuck handler's raise() then lands on that disposition -- typically SIG_DFL -- and the process dies by it, exactly as a single delivery would have. The repair is inert on every legitimate path to the guard: if the signal was never pushed, nothing is installed; if the chain was fully unwound by regular pops, s->old[0] is already installed and reinstalling it changes nothing. The push/pop fast path is untouched. A fault-injected A/B run of the minimal reproducer, compiled from identical sources with and without this change (the gap between the "read n" and "decrement n" steps of pop widened so both racing pops reliably read the same value; 300 interleaved pairs per arm) froze the unpatched arm 121 times out of 300. The patched arm never froze: all 300 runs reinstated the pre-chain disposition and died by it, and 20/20 single-delivery control runs never reached the repair code. Signed-off-by: ian4hu <hu2008yinxiang@163.com>
Author
|
/submit |
|
Submitted as pull.2263.git.1792154272296.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
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.
During
git clone --depth 1over ssh (or any transport that parks theclient with an in-flight packfile), two SIGTERMs delivered a fraction of
a millisecond apart can corrupt the sigchain unwind: both fetch threads
end up looping inside remove_junk_on_signal() at ~200% CPU, the process
survives both signals, and further SIGTERMs have no effect (only SIGKILL
recovers it).
Mechanism (git 2.56.0):
(builtin/clone.c:1134), then on the first tempfile activation (e.g.
shallow.lock) remove_tempfiles_on_signal (tempfile.c:117):
old[0]=SIG_DFL, old[1]=remove_junk_on_signal, n=2.
concurrently; both sigchain_pop() calls read the same s->n and both
install s->old[s->n - 1]: the chain unwinds two levels for two
deliveries, ending at n=0 while the dispatcher still points at
old[1].
popped, whose own sigchain_pop() hits the
n < 1guard (a silentno-op), and whose raise() redelivers to the same stuck dispatcher
forever.
This patch takes the "repair after the fact" route instead of
serializing the chain mutation: sigchain_pop() remembers how many
pushes were ever performed for the signal, and when it finds the chain
empty while the chain bottom is known, it reinstalls s->old[0] -- the
disposition that was in place before the chain was built. The stuck
handler's raise() then lands on that disposition (typically SIG_DFL)
and the process dies by it, exactly as a single delivery would have.
The repair is inert on every legitimate path to the
n < 1guard: ifthe signal was never pushed nothing is installed, and if the chain was
fully unwound by regular pops, s->old[0] is already installed so
reinstalling it changes nothing. The push/pop fast path is untouched --
no locking, no signal-mask manipulation, no Windows-specific code.
Validation: a minimal reproducer (two threads parked in read(), the
same push/pop chain, first handler entry doing real close()+unlink()
work), compiled from identical sources with and without this change.
With a fault injector widening the gap between the "read n" and
"decrement n" steps of pop so both racing pops reliably read the same
value (300 interleaved pairs per arm), the unpatched arm froze 121/300
runs while the patched arm never froze: all 300 runs reinstated the
pre-chain disposition and died by it. 20/20 single-delivery control
runs never reached the repair code.
An alternative fix that serializes sigchain_push()/sigchain_pop() with
a per-signal spinlock exists as well; both eliminate the freeze. The
lock prevents the corruption, this patch heals it; they are independent
and either one suffices.