Git development
 help / color / mirror / Atom feed
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


  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