Re: [PATCH 3/4] scoped_ignore_signal: Use sigprocmask+sigtimedwait instead of signal

[Pedro Alves <pedro@palves.net>]

On 2021-06-26 2:41 p.m., John Baldwin wrote:
> On 6/26/21 5:35 AM, Simon Marchi via Gdb-patches wrote:
>> On 2021-06-15 7:14 a.m., Pedro Alves wrote:
>>> The problem with using signal(...) to temporarily ignore a signal, is
>>> that that changes the the signal disposition for the whole process.
>>> If multiple threads do it at the same time, you have a race.
>>>
>>> Fix this by using sigprocmask + sigtimedwait to implement the ignoring
>>> instead, if available, which I think probably means everywhere except
>>> Windows nowadays.  This way, we only change the signal mask for the
>>> current thread, so there's no race.
>>
>> I tried to build on macOS today and got:
>>
>>
>>        CXX    compile/compile.o
>>      In file included from /Users/smarchi/src/binutils-gdb/gdb/compile/compile.c:46:
>>      /Users/smarchi/src/binutils-gdb/gdb/../gdbsupport/scoped_ignore_signal.h:69:4: error: use of undeclared identifier 'sigtimedwait'
>>                sigtimedwait (&set, nullptr, &zero_timeout);
>>                ^
>>
>> I didn't have time to dig yet.
> 
> It looks like macOS doesn't implement either sigtimedwait() or sigwaitinfo()
> which are part of POSIX from 1996 (*sigh*); however, macOS does provide
> sigpending() and sigwait(), so perhaps we can do something like:
> 
>    if (ConsumePending)
>      {
> #ifdef HAVE_SIGTIMEDWAIT
>        const timespec zero_timeout = {};
> 
>        sigtimedwait (&set, nullptr, &zero_timeout);
> #else
>        sigset_t pending;
> 
>        sigpending (&pending);
>        if (sigismember (&pending, SIG))
>          sigwait (&set, nullptr);
> #endif
>      }
> 

Thanks, that looks right to me.  We're using sigtimewait with zero timeout to consume
a pending signal.  sigpending + sigwait effectively does the same thing.

According to gnulib, sigpending exists everywhere but mingw:

 https://www.gnu.org/software/gnulib/manual/html_node/sigpending.html

and sigtimedwait is missing on "Mac OS X 10.13, OpenBSD 6.7, Minix 3.1.8, Cygwin 2.9, mingw, MSVC 14, Android 5.1.":

  https://www.gnu.org/software/gnulib/manual/html_node/sigtimedwait.html

So the approach you propose should work well.

Given sigpending is everywhere Unix-like, if we'd like, we could instead use the sigpending + sigwait path
unconditionally everywhere, but that's two syscalls instead of one, and the configure check is
trivial (just add sigtimedwait to AC_CHECK_FUNCS in gdbsupport/common.m4), so why not indeed save one
syscall, seems easy enough.

Let me know if you'd rather me do the leg work of writing the actual patch.

Pedro Alves