OSDN Git Service

Remove the global thread list.
authorElliott Hughes <enh@google.com>
Wed, 4 Jan 2017 22:12:54 +0000 (14:12 -0800)
committerElliott Hughes <enh@google.com>
Sat, 7 Jan 2017 22:16:46 +0000 (14:16 -0800)
commitb0e8c565a622b5519e03d4416b0b5b1a5f20d7f5
tree8362f2754276b72e79a652885be7379127ca6e07
parentfb07c36bc061db4ca5d8348ff6bc1e60b6c53191
Remove the global thread list.

Another release, another attempt to fix this bug.

This change affects pthread_detach, pthread_getcpuclockid,
pthread_getschedparam/pthread_setschedparam, pthread_join, and pthread_kill:
instead of returning ESRCH when passed an invalid pthread_t, they'll now SEGV.

Note that this doesn't change behavior as much as you might think: the old
lookup only held the global thread list lock for the duration of the lookup,
so there was still a race between that and the dereference in the caller,
given that callers actually need the tid to pass to some syscall or other,
and sometimes update fields in the pthread_internal_t struct too.

We can't check thread->tid against 0 to see whether a pthread_t is still
valid because a dead thread gets its thread struct unmapped along with its
stack, so the dereference isn't safe.

Taking the affected functions one by one:

* pthread_getcpuclockid and pthread_getschedparam/pthread_setschedparam
  should be fine. Unsafe calls to those seem highly unlikely.

* Unsafe pthread_detach callers probably want to switch to
  pthread_attr_setdetachstate instead, or using pthread_detach(pthread_self())
  from the new thread's start routine rather than doing the detach in the
  parent.

* pthread_join calls should be safe anyway, because a joinable thread won't
  actually exit and unmap until it's joined. If you're joining an
  unjoinable thread, the fix is to stop marking it detached. If you're
  joining an already-joined thread, you need to rethink your design.

* Unsafe pthread_kill calls aren't portably fixable. (And are obviously
  inherently non-portable as-is.) The best alternative on Android is to
  use pthread_gettid_np at some point that you know the thread to be alive,
  and then call kill/tgkill directly. That's still not completely safe
  because if you're too late, the tid may have been reused, but then your
  code is inherently unsafe anyway.

If we find too much code is still broken, we can come back and disable
the global thread list lookups for anything targeting >= O and then have
another go at really removing this in P...

Bug: http://b/19636317
Test: N6P boots, bionic tests pass
Change-Id: Ia92641212f509344b99ee2a9bfab5383147fcba6
15 files changed:
libc/Android.bp
libc/bionic/libc_init_common.cpp
libc/bionic/pthread_create.cpp
libc/bionic/pthread_detach.cpp
libc/bionic/pthread_exit.cpp
libc/bionic/pthread_getcpuclockid.cpp
libc/bionic/pthread_getschedparam.cpp
libc/bionic/pthread_internal.cpp [deleted file]
libc/bionic/pthread_internal.h
libc/bionic/pthread_join.cpp
libc/bionic/pthread_kill.cpp
libc/bionic/pthread_setname_np.cpp
libc/bionic/pthread_setschedparam.cpp
tests/pthread_test.cpp
tests/time_test.cpp