[v2,0/4] Fix ESRCH issues in pthread_cancel, pthread_kill

Message ID cover.1629475813.git.fweimer@redhat.com
Headers
Series Fix ESRCH issues in pthread_cancel, pthread_kill |

Message

Florian Weimer Aug. 20, 2021, 4:11 p.m. UTC
  The new version fixes the support/Makefile update and puts the generic
“give me a non-reusable TID” functionality into a generic helper
function.

Tested on i686-linux-gnu and x86_64-linux-gnu.  Built with
build-many-glibcs.py.

Thanks,
Florian

Florian Weimer (4):
  support: Add support_wait_for_thread_exit
  nptl: pthread_kill, pthread_cancel should fail after exit (bug 19193)
  nptl: Add internal __pthread_call_with_tid function
  nptl: Fix race between pthread_kill and thread exit (bug 12889)

 nptl/Makefile                                 |   5 +-
 nptl/allocatestack.c                          |   1 +
 nptl/descr.h                                  |  11 ++
 nptl/pthread_call_with_tid.c                  |  66 +++++++
 nptl/pthread_cancel.c                         |   9 +-
 nptl/pthread_create.c                         |  18 ++
 nptl/pthread_kill.c                           |  44 +++--
 nptl/tst-pthread_call_with_tid.c              | 165 ++++++++++++++++++
 support/Makefile                              |   3 +-
 support/support.h                             |   4 +
 support/support_wait_for_thread_exit.c        |  72 ++++++++
 sysdeps/nptl/pthreadP.h                       |  20 +++
 sysdeps/pthread/Makefile                      |   7 +-
 sysdeps/pthread/tst-kill4.c                   |  90 ----------
 sysdeps/pthread/tst-pthread_cancel-exited.c   |  45 +++++
 .../pthread/tst-pthread_cancel-select-loop.c  |  87 +++++++++
 sysdeps/pthread/tst-pthread_kill-exited.c     |  46 +++++
 sysdeps/pthread/tst-pthread_kill-exiting.c    | 110 ++++++++++++
 18 files changed, 682 insertions(+), 121 deletions(-)
 create mode 100644 nptl/pthread_call_with_tid.c
 create mode 100644 nptl/tst-pthread_call_with_tid.c
 create mode 100644 support/support_wait_for_thread_exit.c
 delete mode 100644 sysdeps/pthread/tst-kill4.c
 create mode 100644 sysdeps/pthread/tst-pthread_cancel-exited.c
 create mode 100644 sysdeps/pthread/tst-pthread_cancel-select-loop.c
 create mode 100644 sysdeps/pthread/tst-pthread_kill-exited.c
 create mode 100644 sysdeps/pthread/tst-pthread_kill-exiting.c
  

Comments

Adhemerval Zanella Netto Aug. 20, 2021, 6:09 p.m. UTC | #1
On 20/08/2021 13:11, Florian Weimer via Libc-alpha wrote:
> The new version fixes the support/Makefile update and puts the generic
> “give me a non-reusable TID” functionality into a generic helper
> function.
> 
> Tested on i686-linux-gnu and x86_64-linux-gnu.  Built with
> build-many-glibcs.py.

Sorry, but this approach is even more complex and convoluted than your
previous one.  I have fixed some issues and adapted your patches on top
on my pthread fixes [1].

By using another field instead of pthread::tid to synchronize thread
termination [2] it allows the synchronization required to be *way* more
simple [3]: it basically requires to block all signals (since the lock
is not recursive), lll_lock, lll_unlock (since there is no more kernel
involved on thread termination).

The lll_lock already has the logic to handle multiple waiters, so
there is no need to replicate the idea of more futex/atomic calls.

On the patches I used your tests as-is.

As a side note, now that we are synchronize the tid access, we will
need to handleother pthread functions that also access it as well:

  pthread_getaffinity_np,
  pthread_getcpuclockid
  pthread_getname_np
  pthread_getschedparam
  pthread_setaffinity_np
  pthread_setname_np
  pthread_setschedparam
  pthread_setschedprio
  pthread_sigqueue.

[1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/pthread-multiple-fixes
[2] https://sourceware.org/git/?p=glibc.git;a=commit;h=ebae3185ca5529ad2147e6f75978a13e2d459b8e
[3] https://sourceware.org/git/?p=glibc.git;a=commit;h=bb94df8eaba1af636bd6af7ec9142bc3e0e25868
  
Florian Weimer Aug. 20, 2021, 6:48 p.m. UTC | #2
* Adhemerval Zanella:

> On 20/08/2021 13:11, Florian Weimer via Libc-alpha wrote:
>> The new version fixes the support/Makefile update and puts the generic
>> “give me a non-reusable TID” functionality into a generic helper
>> function.
>> 
>> Tested on i686-linux-gnu and x86_64-linux-gnu.  Built with
>> build-many-glibcs.py.
>
> Sorry, but this approach is even more complex and convoluted than your
> previous one.  I have fixed some issues and adapted your patches on top
> on my pthread fixes [1].

> [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/pthread-multiple-fixes

Hmm.  Are you going to repost this series?

Thanks,
Florian
  
Adhemerval Zanella Netto Aug. 23, 2021, 12:19 p.m. UTC | #3
On 20/08/2021 15:48, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 20/08/2021 13:11, Florian Weimer via Libc-alpha wrote:
>>> The new version fixes the support/Makefile update and puts the generic
>>> “give me a non-reusable TID” functionality into a generic helper
>>> function.
>>>
>>> Tested on i686-linux-gnu and x86_64-linux-gnu.  Built with
>>> build-many-glibcs.py.
>>
>> Sorry, but this approach is even more complex and convoluted than your
>> previous one.  I have fixed some issues and adapted your patches on top
>> on my pthread fixes [1].
> 
>> [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/pthread-multiple-fixes
> 
> Hmm.  Are you going to repost this series?

Yes, I will add a patch to a fix for the tid members on the 
functions that uses it and repost it.
  
Florian Weimer Aug. 23, 2021, 12:21 p.m. UTC | #4
* Adhemerval Zanella:

> On 20/08/2021 15:48, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> On 20/08/2021 13:11, Florian Weimer via Libc-alpha wrote:
>>>> The new version fixes the support/Makefile update and puts the generic
>>>> “give me a non-reusable TID” functionality into a generic helper
>>>> function.
>>>>
>>>> Tested on i686-linux-gnu and x86_64-linux-gnu.  Built with
>>>> build-many-glibcs.py.
>>>
>>> Sorry, but this approach is even more complex and convoluted than your
>>> previous one.  I have fixed some issues and adapted your patches on top
>>> on my pthread fixes [1].
>> 
>>> [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/pthread-multiple-fixes
>> 
>> Hmm.  Are you going to repost this series?
>
> Yes, I will add a patch to a fix for the tid members on the 
> functions that uses it and repost it.

Looking forward to it.  I will try to review it.

Florian