Re: [PATCH] sigchain: recover from concurrent double-pops of the same signal

[Phillip Wood <phillip.wood123@gmail.com>]

A couple more thoughts ...

On 28/10/2026 16:39, Phillip Wood wrote:
> On 28/10/2026 14:47, ian4hu wrote:
>>
>> If you prefer the thread-level fix, I am happy to build it: a
>> git_pthread_create() wrapper that blocks SIGINT/SIGHUP/SIGTERM/
>> SIGQUIT around pthread_create()
> 
> I think that is probably the safest solution (though it does not help 
> for threads created external library code - I'm not sure if we have any 
> instances of that). I'd be interested to hear what others think, so I'd 
> wait a couple of days to see if anyone else comments before spending 
> much time on it.

We have a number of places where we spin up a bunch of threads to 
parallelize some work, for example in preload_index() we create some 
threads in a loop to read the index. In cases like that, where the loop 
body is just setting up some per-thread data and creating a thread, it 
would be more efficient to block signals before the loop and unblock 
them after, rather than blocking and unblocking each time we create a 
thread.

>> - leaving SIGPIPE alone, since it is
>> a synchronous signal and blocking it in workers turns deaths into
>> EPIPE
> 
> Does it? I thought that happened when its disposition is set to SIG_IGN. 
> Blocking a signal delays its delivery until it is unblocked, it does not 
> ignore the signal.

To clarify, in a multi threaded process the delivery of the signal is 
only delayed if it is blocked in all threads. If there's one or more 
threads where the signal isn't blocked it will be delivered to one of 
those threads.

Thanks

Phillip

>> - plus a sigaction()-based chain that masks all signals while
>> a handler runs.
> 
> When I wrote my last email I was thinking about blocking the signal that 
> is being handled as sigaction() defers the signal that is being handled 
> unless SA_NODEFER is passed. However, sigchain_push_common() sets the 
> same handler for all the common signals so it is probably safer to block 
> at least those as well as I'm not sure all our handlers are reentrant.
> 
>>  I would then drop this patch, or keep it as a
>> separate last-resort series if you think the belt and braces are
>> worth it.
> 
> I'd be inclined to drop it - if we're handling signals in a single 
> thread that should avoid the issue.
> 
> Thanks
> 
> Phillip