Skip to content

Commit

Permalink
PTHREAD_KEYS_MAX cleanup.
Browse files Browse the repository at this point in the history
I fixed this bug a while back, but didn't remove it from the list,
could have added a better test, and could have written clearer code
that didn't require a comment.

Change-Id: Iebdf0f9a54537a7d5cbca254a5967b1543061f3d
  • Loading branch information
enh-google committed Dec 12, 2013
1 parent 38fcbbb commit 1887621
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 10 deletions.
3 changes: 0 additions & 3 deletions ABI-bugs.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,5 @@ KNOWN ABI BUGS
sigset_t is too small on ARM and x86 (but correct on MIPS), so support
for real-time signals is broken. http://b/5828899

Too few TLS slots mean we can't allocate 128 pthread_key_t instances,
which POSIX says should be the minimum.

atexit(3) handlers registered by a shared library aren't called on
dlclose(3); this only affects ARM. http://b/4998315
15 changes: 8 additions & 7 deletions libc/private/bionic_tls.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#define __BIONIC_PRIVATE_BIONIC_TLS_H_

#include <sys/cdefs.h>
#include <sys/limits.h>
#include "__get_tls.h"

__BEGIN_DECLS
Expand Down Expand Up @@ -74,21 +75,21 @@ enum {
};

/*
* Maximum number of elements in the TLS array.
* POSIX says this must be at least 128, but Android has traditionally had only 64, minus those
* ones used internally by bionic itself.
* There are two kinds of slot used internally by bionic --- there are the well-known slots
* enumerated above, and then there are those that are allocated during startup by calls to
* pthread_key_create; grep for GLOBAL_INIT_THREAD_LOCAL_BUFFER to find those. We need to manually
* maintain that second number, but pthread_test will fail if we forget.
*/
#define GLOBAL_INIT_THREAD_LOCAL_BUFFER_COUNT 4

#define BIONIC_ALIGN(x, a) (((x) + (a - 1)) & ~(a - 1))

/*
* This is PTHREAD_KEYS_MAX + TLS_SLOT_FIRST_USER_SLOT + GLOBAL_INIT_THREAD_LOCAL_BUFFER_COUNT
* rounded up to maintain stack alignment.
* Maximum number of elements in the TLS array.
* This includes space for pthread keys and our own internal slots.
* We need to round up to maintain stack alignment.
*/
#define BIONIC_ALIGN(x, a) (((x) + (a - 1)) & ~(a - 1))
#define BIONIC_TLS_SLOTS BIONIC_ALIGN(128 + TLS_SLOT_FIRST_USER_SLOT + GLOBAL_INIT_THREAD_LOCAL_BUFFER_COUNT, 4)
#define BIONIC_TLS_SLOTS BIONIC_ALIGN(PTHREAD_KEYS_MAX + TLS_SLOT_FIRST_USER_SLOT + GLOBAL_INIT_THREAD_LOCAL_BUFFER_COUNT, 4)

__END_DECLS

Expand Down
5 changes: 5 additions & 0 deletions tests/pthread_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ TEST(pthread, pthread_key_create) {

#if !defined(__GLIBC__) // glibc uses keys internally that its sysconf value doesn't account for.
TEST(pthread, pthread_key_create_lots) {
// POSIX says PTHREAD_KEYS_MAX should be at least 128.
ASSERT_GE(PTHREAD_KEYS_MAX, 128);
// sysconf shouldn't return a smaller value.
ASSERT_GE(sysconf(_SC_THREAD_KEYS_MAX), PTHREAD_KEYS_MAX);

// We can allocate _SC_THREAD_KEYS_MAX keys.
std::vector<pthread_key_t> keys;
for (int i = 0; i < sysconf(_SC_THREAD_KEYS_MAX); ++i) {
Expand Down

0 comments on commit 1887621

Please sign in to comment.