From: Phillip Wood <phillip.wood123@gmail.com>
To: ian4hu <hu2008yinxiang@163.com>
Cc: git@vger.kernel.org, gitster@pobox.com, peff@peff.net,
phillip.wood@dunelm.org.uk
Subject: Re: [PATCH] sigchain: recover from concurrent double-pops of the same signal
Date: Thu, 29 Oct 2026 16:03:18 +0000 [thread overview]
Message-ID: <4c2728ca-5511-4c30-a2ef-6c32fe7a3e9e@gmail.com> (raw)
In-Reply-To: <20261029141521.164544-1-hu2008yinxiang@163.com>
On 29/10/2026 14:15, ian4hu wrote:
>
> I left SIGPIPE out precisely to keep the "upstream of a pipe
> dies of SIGPIPE" behaviour that common-init.c restores. The set I
> plan to block is SIGINT, SIGHUP, SIGTERM and SIGQUIT. If you
> think SIGPIPE belongs in it too, I will add it.
I think we want to block all signals when we create a thread, so that
all signals are handled by the main (initial) thread.
Everything else sounds good
Thanks
Phillip
> On the blocking pattern: agreed. I will block once around the
> spawn loop and restore before the join loop, rather than around
> each pthread_create(), via a small guard pair
>
> struct signal_mask_guard guard;
>
> block_sigchain_signals(&guard);
> for (i = 0; i < nr_threads; i++)
> pthread_create(&t[i], NULL, fn, &data[i]);
> restore_sigchain_signals(&guard);
>
> plus a git_pthread_create() wrapper on top for the single-thread
> call sites. The loops I have in mind are preload_index(),
> index-pack, pack-objects, grep, name-hash and the read-cache entry
> loader; the fsmonitor--daemon and simple-ipc threads I would leave
> alone for now, as they have their own lifecycle and signal
> handling.
>
> On sigaction: agreed as well. The handled signal is deferred by
> default, so the remaining hole is a handler being interrupted by
> one of the other common signals that share the same handler. I
> will set sa_mask to sigfillset() and save/restore the whole
> struct sigaction, so pop() does not clobber the caller's
> sa_flags/sa_mask.
>
> The one gap I cannot close is a thread created by library code
> rather than by us. libcurl's threaded resolver is the case I am
> aware of, depending on how libcurl is built; I do not know of an
> in-tree instance, and I will mention it in the cover letter rather
> than claim it is covered.
>
> So I will drop this patch and build the series along those lines.
> As you suggested, I will wait a few days for Peff and Junio to
> comment before spending much time on it; if nobody objects I will
> send the replacement as v2.
>
> Thanks,
> ian4hu
next prev parent reply other threads:[~2026-10-29 16:03 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-29 14:15 [PATCH] sigchain: recover from concurrent double-pops of the same signal ian4hu
2026-10-29 16:03 ` Phillip Wood [this message]
-- strict thread matches above, loose matches on Subject: below --
2026-10-29 14:13 ian4hu
2026-10-16 12:37 Ian Hu via GitGitGadget
2026-10-28 13:37 ` ian4hu
2026-10-28 14:29 ` Phillip Wood
2026-10-28 14:47 ` ian4hu
2026-10-28 16:39 ` Phillip Wood
2026-10-29 11:15 ` Phillip Wood
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4c2728ca-5511-4c30-a2ef-6c32fe7a3e9e@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=hu2008yinxiang@163.com \
--cc=peff@peff.net \
--cc=phillip.wood@dunelm.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox