Skip to content

Commit

Permalink
Fix pthread_detach for already-exited threads.
Browse files Browse the repository at this point in the history
This fix handles the case when pthread_detach is
called for threads which have already exited.
This is required to avoid any memory leak

Change-Id: I2bf7f41234d93b226132a4c51705f4186f4961c3
Reported-by: Paresh Nakhe <pnakhe@codeaurora.org>
(cherry picked from commit 452fa4e)
  • Loading branch information
enh-google authored and hyperb1iss committed Jun 2, 2014
1 parent 52b9a11 commit 38e8c79
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 0 deletions.
6 changes: 6 additions & 0 deletions libc/bionic/pthread_detach.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ int pthread_detach(pthread_t t) {
return 0; // Already being joined; silently do nothing, like glibc.
}

if (thread->tid == 0) {
// Already exited; clean up.
_pthread_internal_remove_locked(thread.get());
return 0;
}

thread->attr.flags |= PTHREAD_ATTR_FLAG_DETACHED;
return 0;
}
30 changes: 30 additions & 0 deletions tests/pthread_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include <errno.h>
#include <limits.h>
#include <malloc.h>
#include <pthread.h>
#include <unistd.h>

Expand Down Expand Up @@ -278,6 +279,35 @@ TEST(pthread, pthread_detach__no_such_thread) {
ASSERT_EQ(ESRCH, pthread_detach(dead_thread));
}

TEST(pthread, pthread_detach__leak) {
size_t initial_bytes = mallinfo().uordblks;

pthread_attr_t attr;
ASSERT_EQ(0, pthread_attr_init(&attr));
ASSERT_EQ(0, pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE));

std::vector<pthread_t> threads;
for (size_t i = 0; i < 32; ++i) {
pthread_t t;
ASSERT_EQ(0, pthread_create(&t, &attr, IdFn, NULL));
threads.push_back(t);
}

sleep(1);

for (size_t i = 0; i < 32; ++i) {
ASSERT_EQ(0, pthread_detach(threads[i])) << i;
}

size_t final_bytes = mallinfo().uordblks;

int leaked_bytes = (final_bytes - initial_bytes);

// User code (like this test) doesn't know how large pthread_internal_t is.
// We can be pretty sure it's more than 128 bytes.
ASSERT_LT(leaked_bytes, 32 /*threads*/ * 128 /*bytes*/);
}

TEST(pthread, pthread_getcpuclockid__clock_gettime) {
pthread_t t;
ASSERT_EQ(0, pthread_create(&t, NULL, SleepFn, reinterpret_cast<void*>(5)));
Expand Down

0 comments on commit 38e8c79

Please sign in to comment.