pthread_cancel async-signal-safe

pthread_cancel async-signal-safe

[Adhemerval Zanella]

POSIX states that pthread_cancel should be async-signal-safe [1].
IMU there are two main issues in current implementation that prevents
it:

  1. The pthread_cancel_init for shared builds,
  2. And the SIGCANCEL raise for asynchronous cancellation.

For 2. it would be possible to use a safer raise implementation and I 
have sent a patch where it fixes for pthread_kill [2].  It should be 
simple to use __pthread_kill_internal on pthread_cancel as well.

For 1. I don't see a easier solution without moving pthread_cancel_init
to out of pthread_cancel and this has its own drawbacks (by always
forcing loading libgcc_s.so when libpthread is linked).   

Florian, I recall that you had investigate some issue regarding C++
exceptions and signal handler that you presented on previous GNU 
Cauldron, so I wonder if you might have an idea how we can fix it.

Maybe linking libgcc_s.a statically with libpthread and remove the
dynamic loading of libgcc_s.so? I am not sure of this implication 
of this strategy.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
[2] https://sourceware.org/pipermail/libc-alpha/2020-December/120419.html

Re: pthread_cancel async-signal-safe

[Florian Weimer]

* Adhemerval Zanella via Libc-alpha:

> Florian, I recall that you had investigate some issue regarding C++
> exceptions and signal handler that you presented on previous GNU 
> Cauldron, so I wonder if you might have an idea how we can fix it.

I didn't realize that pthread_cancel was required to be
async-signal-safe.  We don't have an async-signal-safe dlopen and we
can't block signals during dlopen because that breaks the web browser
sandboxes.

Are sure that pthread_cancel is on the list?  I don't see it in
signal-safety(7).

Re: pthread_cancel async-signal-safe

[Adhemerval Zanella]

On 04/12/2020 15:29, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> Florian, I recall that you had investigate some issue regarding C++
>> exceptions and signal handler that you presented on previous GNU 
>> Cauldron, so I wonder if you might have an idea how we can fix it.
> 
> I didn't realize that pthread_cancel was required to be
> async-signal-safe.  We don't have an async-signal-safe dlopen and we
> can't block signals during dlopen because that breaks the web browser
> sandboxes.
> 
> Are sure that pthread_cancel is on the list?  I don't see it in
> signal-safety(7).
> 

It is on the 2.9.5 Thread Cancellation at 'Async-Cancel Safety' entry
in POSIX manual [1]:

  "The pthread_cancel(), pthread_setcancelstate(), and pthread_setcanceltype() 
   functions are defined to be async-cancel safe."

Also, neither pthread_setcancelstate nor pthread_setcanceltype are 
async-signal-safe due __do_cancel usage (which calls _Unwind_ForcedUnwind
which in turn might call pthread_cancel_init).

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html

Re: pthread_cancel async-signal-safe

[Florian Weimer]

* Adhemerval Zanella:

> On 04/12/2020 15:29, Florian Weimer wrote:
>> * Adhemerval Zanella via Libc-alpha:
>> 
>>> Florian, I recall that you had investigate some issue regarding C++
>>> exceptions and signal handler that you presented on previous GNU 
>>> Cauldron, so I wonder if you might have an idea how we can fix it.
>> 
>> I didn't realize that pthread_cancel was required to be
>> async-signal-safe.  We don't have an async-signal-safe dlopen and we
>> can't block signals during dlopen because that breaks the web browser
>> sandboxes.
>> 
>> Are sure that pthread_cancel is on the list?  I don't see it in
>> signal-safety(7).
>> 
>
> It is on the 2.9.5 Thread Cancellation at 'Async-Cancel Safety' entry
> in POSIX manual [1]:
>
>   "The pthread_cancel(), pthread_setcancelstate(), and pthread_setcanceltype() 
>    functions are defined to be async-cancel safe."

Oh, sneaky.

I think we would have to write our own unwinder to comply with that.

> Also, neither pthread_setcancelstate nor pthread_setcanceltype are 
> async-signal-safe due __do_cancel usage (which calls _Unwind_ForcedUnwind
> which in turn might call pthread_cancel_init).

I think that's actually not a problem because you can get there only
if pthread_cancel has previously initialized cancellation.

Re: pthread_cancel async-signal-safe

[Adhemerval Zanella]

On 04/12/2020 15:43, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 04/12/2020 15:29, Florian Weimer wrote:
>>> * Adhemerval Zanella via Libc-alpha:
>>>
>>>> Florian, I recall that you had investigate some issue regarding C++
>>>> exceptions and signal handler that you presented on previous GNU 
>>>> Cauldron, so I wonder if you might have an idea how we can fix it.
>>>
>>> I didn't realize that pthread_cancel was required to be
>>> async-signal-safe.  We don't have an async-signal-safe dlopen and we
>>> can't block signals during dlopen because that breaks the web browser
>>> sandboxes.
>>>
>>> Are sure that pthread_cancel is on the list?  I don't see it in
>>> signal-safety(7).
>>>
>>
>> It is on the 2.9.5 Thread Cancellation at 'Async-Cancel Safety' entry
>> in POSIX manual [1]:
>>
>>   "The pthread_cancel(), pthread_setcancelstate(), and pthread_setcanceltype() 
>>    functions are defined to be async-cancel safe."
> 
> Oh, sneaky.
> 
> I think we would have to write our own unwinder to comply with that.

Can't we move pthread_cancel_init to __pthread_initialize_minimal_internal?

> 
>> Also, neither pthread_setcancelstate nor pthread_setcanceltype are 
>> async-signal-safe due __do_cancel usage (which calls _Unwind_ForcedUnwind
>> which in turn might call pthread_cancel_init).
> 
> I think that's actually not a problem because you can get there only
> if pthread_cancel has previously initialized cancellation.
> 

Indeed, do_cancel would be called only if pthread_cancel has the 
CANCELED_BITMASK previously.

Re: pthread_cancel async-signal-safe

[Florian Weimer]

* Adhemerval Zanella:

> Can't we move pthread_cancel_init to __pthread_initialize_minimal_internal?

In the future, that will be equivalent to dynamically linking glibc
against libgcc_s.  In the sense that every process will load libgcc_s.

Re: pthread_cancel async-signal-safe

[Adhemerval Zanella]

On 04/12/2020 16:32, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> Can't we move pthread_cancel_init to __pthread_initialize_minimal_internal?
> 
> In the future, that will be equivalent to dynamically linking glibc
> against libgcc_s.  In the sense that every process will load libgcc_s.

Yes, this should be simpler until we write own unwinder (which I think 
it is overkill and would require a lot of work) or came with a different
mechanism to implement pthread cancellation (maybe by decoupling from
C++ exception, which I am not sure it would be possible due compatibility).

Re: pthread_cancel async-signal-safe

[Joseph Myers]

On Fri, 4 Dec 2020, Adhemerval Zanella via Libc-alpha wrote:

> For 1. I don't see a easier solution without moving pthread_cancel_init
> to out of pthread_cancel and this has its own drawbacks (by always
> forcing loading libgcc_s.so when libpthread is linked).   

libgcc_s.so should indeed be loaded on startup (loading it later has the 
problem of introducing failures, if loading fails, in a context where 
failures aren't expected).

> Maybe linking libgcc_s.a statically with libpthread and remove the
> dynamic loading of libgcc_s.so? I am not sure of this implication 
> of this strategy.

I'd expect bootstrapping problems, and possibly also problems working with 
unwind information coming from newer GCC versions than the one libgcc_eh.a 
came from.

We have a long comment in Makeconfig about linking with libgcc.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: pthread_cancel async-signal-safe

[Adhemerval Zanella]

On 04/12/2020 15:32, Joseph Myers wrote:
> On Fri, 4 Dec 2020, Adhemerval Zanella via Libc-alpha wrote:
> 
>> For 1. I don't see a easier solution without moving pthread_cancel_init
>> to out of pthread_cancel and this has its own drawbacks (by always
>> forcing loading libgcc_s.so when libpthread is linked).   
> 
> libgcc_s.so should indeed be loaded on startup (loading it later has the 
> problem of introducing failures, if loading fails, in a context where 
> failures aren't expected).

Indeed, it might be required to even loading it early as pthread_cancel 
currently does or just add libgcc_s.so in DT_NEEDED for at least
libpthread (and once we move all libpthread implementation to libc.so
it will make programs always load libgcc_s.so).

> 
>> Maybe linking libgcc_s.a statically with libpthread and remove the
>> dynamic loading of libgcc_s.so? I am not sure of this implication 
>> of this strategy.
> 
> I'd expect bootstrapping problems, and possibly also problems working with 
> unwind information coming from newer GCC versions than the one libgcc_eh.a 
> came from.
> 
> We have a long comment in Makeconfig about linking with libgcc.
> 

Indeed this does not seems the best option, neither packing our own unwinder
since it will have the inherent unwind information mismatch issues.

The pthread_cancel interoperability with C++ seems a heavy burden, but I'm
not sure which better options we do have here.

Re: pthread_cancel async-signal-safe

[Florian Weimer]

* Adhemerval Zanella:

> The pthread_cancel interoperability with C++ seems a heavy burden, but I'm
> not sure which better options we do have here.

It's not just C++, it's needed for -fexception support in C and other
languages.

Furthermore, the out-of-line DWARF unwinding tables can be made
considerably safer than on-stack pointers used by the longjmp-based
mechanism.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: pthread_cancel async-signal-safe

[Adhemerval Zanella]

On 07/12/2020 10:06, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> The pthread_cancel interoperability with C++ seems a heavy burden, but I'm
>> not sure which better options we do have here.
> 
> It's not just C++, it's needed for -fexception support in C and other
> languages.
> 
> Furthermore, the out-of-line DWARF unwinding tables can be made
> considerably safer than on-stack pointers used by the longjmp-based
> mechanism.

Do you mean for pthread_cleanup_push? I do agree, but I haven't check exactly
what need to be done to accomplish neither it if would be be possible to
accomplish for !_GNU case.  So do you think packing an unwinder on glibc
could be an improvement?  If so, which would be possible issues?

Re: pthread_cancel async-signal-safe

[Florian Weimer]

* Adhemerval Zanella:

> On 07/12/2020 10:06, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> The pthread_cancel interoperability with C++ seems a heavy burden, but I'm
>>> not sure which better options we do have here.
>> 
>> It's not just C++, it's needed for -fexception support in C and other
>> languages.
>> 
>> Furthermore, the out-of-line DWARF unwinding tables can be made
>> considerably safer than on-stack pointers used by the longjmp-based
>> mechanism.
>
> Do you mean for pthread_cleanup_push? I do agree, but I haven't check exactly
> what need to be done to accomplish neither it if would be be possible to
> accomplish for !_GNU case.  So do you think packing an unwinder on glibc
> could be an improvement?  If so, which would be possible issues?

I think the issues are largely political (renegotiating the GCC vs glibc
split).

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: pthread_cancel async-signal-safe

[Adhemerval Zanella]

On 07/12/2020 10:33, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 07/12/2020 10:06, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> The pthread_cancel interoperability with C++ seems a heavy burden, but I'm
>>>> not sure which better options we do have here.
>>>
>>> It's not just C++, it's needed for -fexception support in C and other
>>> languages.
>>>
>>> Furthermore, the out-of-line DWARF unwinding tables can be made
>>> considerably safer than on-stack pointers used by the longjmp-based
>>> mechanism.
>>
>> Do you mean for pthread_cleanup_push? I do agree, but I haven't check exactly
>> what need to be done to accomplish neither it if would be be possible to
>> accomplish for !_GNU case.  So do you think packing an unwinder on glibc
>> could be an improvement?  If so, which would be possible issues?
> 
> I think the issues are largely political (renegotiating the GCC vs glibc
> split).

So what do you suggest to progress in the stalemate or the fix to require
adequate an improve glibc pthread cancellation (which is plagued with
diverse issues)?

We might try to copy gcc current unwinder code and avoid loading libgcc_so,
it should fix the async-signal-safe issue and improve the possible failures
trying to load libgcc_s.so late.  If I recall correctly from your Caudron
presentation, gcc developers are not willing neither plan to actually use
glibc unwinder; so it would be used solely for our own use (at first).

Re: pthread_cancel async-signal-safe

[Florian Weimer]

* Adhemerval Zanella:

> On 07/12/2020 10:33, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> On 07/12/2020 10:06, Florian Weimer wrote:
>>>> * Adhemerval Zanella:
>>>>
>>>>> The pthread_cancel interoperability with C++ seems a heavy burden, but I'm
>>>>> not sure which better options we do have here.
>>>>
>>>> It's not just C++, it's needed for -fexception support in C and other
>>>> languages.
>>>>
>>>> Furthermore, the out-of-line DWARF unwinding tables can be made
>>>> considerably safer than on-stack pointers used by the longjmp-based
>>>> mechanism.
>>>
>>> Do you mean for pthread_cleanup_push? I do agree, but I haven't check exactly
>>> what need to be done to accomplish neither it if would be be possible to
>>> accomplish for !_GNU case.  So do you think packing an unwinder on glibc
>>> could be an improvement?  If so, which would be possible issues?
>> 
>> I think the issues are largely political (renegotiating the GCC vs glibc
>> split).
>
> So what do you suggest to progress in the stalemate or the fix to require
> adequate an improve glibc pthread cancellation (which is plagued with
> diverse issues)?
>
> We might try to copy gcc current unwinder code and avoid loading libgcc_so,
> it should fix the async-signal-safe issue and improve the possible failures
> trying to load libgcc_s.so late.  If I recall correctly from your Caudron
> presentation, gcc developers are not willing neither plan to actually use
> glibc unwinder; so it would be used solely for our own use (at first).

I think the unwinder question should not block fixing other issues that
aren't directly related to async-signal-safety of the pthread_cancel
call.

The GCC unwinder has its own set of async-signal-safety issues, so the
dlopen call really isn't the only problem here.  It also means that just
copying the code isn't going to fix much at this point.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: pthread_cancel async-signal-safe

[Adhemerval Zanella]

On 07/12/2020 10:41, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 07/12/2020 10:33, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> On 07/12/2020 10:06, Florian Weimer wrote:
>>>>> * Adhemerval Zanella:
>>>>>
>>>>>> The pthread_cancel interoperability with C++ seems a heavy burden, but I'm
>>>>>> not sure which better options we do have here.
>>>>>
>>>>> It's not just C++, it's needed for -fexception support in C and other
>>>>> languages.
>>>>>
>>>>> Furthermore, the out-of-line DWARF unwinding tables can be made
>>>>> considerably safer than on-stack pointers used by the longjmp-based
>>>>> mechanism.
>>>>
>>>> Do you mean for pthread_cleanup_push? I do agree, but I haven't check exactly
>>>> what need to be done to accomplish neither it if would be be possible to
>>>> accomplish for !_GNU case.  So do you think packing an unwinder on glibc
>>>> could be an improvement?  If so, which would be possible issues?
>>>
>>> I think the issues are largely political (renegotiating the GCC vs glibc
>>> split).
>>
>> So what do you suggest to progress in the stalemate or the fix to require
>> adequate an improve glibc pthread cancellation (which is plagued with
>> diverse issues)?
>>
>> We might try to copy gcc current unwinder code and avoid loading libgcc_so,
>> it should fix the async-signal-safe issue and improve the possible failures
>> trying to load libgcc_s.so late.  If I recall correctly from your Caudron
>> presentation, gcc developers are not willing neither plan to actually use
>> glibc unwinder; so it would be used solely for our own use (at first).
> 
> I think the unwinder question should not block fixing other issues that
> aren't directly related to async-signal-safety of the pthread_cancel
> call.

Agree.

> 
> The GCC unwinder has its own set of async-signal-safety issues, so the
> dlopen call really isn't the only problem here.  It also means that just
> copying the code isn't going to fix much at this point.

With a proper fix for BZ#12683, I think moving either the initialization
of libgcc_so.so loading or using a in-tree unwinder would make pthread_cancel
async-signal-safe.  The trick would make the signal handler not actually
call __do_cancel, but rather set the return PC to __do_cancel.

Re: pthread_cancel async-signal-safe

[Florian Weimer]

* Adhemerval Zanella:

> On 07/12/2020 10:41, Florian Weimer wrote:
>> The GCC unwinder has its own set of async-signal-safety issues, so the
>> dlopen call really isn't the only problem here.  It also means that just
>> copying the code isn't going to fix much at this point.
>
> With a proper fix for BZ#12683, I think moving either the initialization
> of libgcc_so.so loading or using a in-tree unwinder would make pthread_cancel
> async-signal-safe.  The trick would make the signal handler not actually
> call __do_cancel, but rather set the return PC to __do_cancel.

I'm still worried about lock ordering issues for the various locks used
by the GCC unwinder.  Acting on cancellation while the dynamic loader is
running is invalid and not a concern, but there could be other issues.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: pthread_cancel async-signal-safe

[Joseph Myers]

On Mon, 7 Dec 2020, Adhemerval Zanella via Libc-alpha wrote:

> On 04/12/2020 15:32, Joseph Myers wrote:
> > On Fri, 4 Dec 2020, Adhemerval Zanella via Libc-alpha wrote:
> > 
> >> For 1. I don't see a easier solution without moving pthread_cancel_init
> >> to out of pthread_cancel and this has its own drawbacks (by always
> >> forcing loading libgcc_s.so when libpthread is linked).   
> > 
> > libgcc_s.so should indeed be loaded on startup (loading it later has the 
> > problem of introducing failures, if loading fails, in a context where 
> > failures aren't expected).
> 
> Indeed, it might be required to even loading it early as pthread_cancel 
> currently does or just add libgcc_s.so in DT_NEEDED for at least
> libpthread (and once we move all libpthread implementation to libc.so
> it will make programs always load libgcc_s.so).

Note that if you add it in DT_NEEDED, it should be done in a way that 
doesn't require the real libgcc_s.so to be built before libc.so is, to 
avoid bootstrapping issues.

-- 
Joseph S. Myers
joseph@codesourcery.com