From a0350aed12dd46783d856ec3fba5e2eb3a9326d4 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Tue, 25 Sep 2018 12:05:53 -0300 Subject: [PATCH 1/2] lib: fix a memory leak in FreeBSD Two important changes: * Centralize the thread teardown procedure; * Save and restore thread mutex context to avoid losing the memory pointer; Signed-off-by: Rafael Zalamena (cherry picked from commit 6655966d2c2fe7a29ca29af4366fa3e044ad1170) --- lib/thread.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/lib/thread.c b/lib/thread.c index 52bc79ffe6..78e416f363 100644 --- a/lib/thread.c +++ b/lib/thread.c @@ -57,6 +57,7 @@ pthread_key_t thread_current; pthread_mutex_t masters_mtx = PTHREAD_MUTEX_INITIALIZER; static struct list *masters; +static void thread_free(struct thread_master *master, struct thread *thread); /* CLI start ---------------------------------------------------------------- */ static unsigned int cpu_record_hash_key(struct cpu_thread_history *a) @@ -537,6 +538,8 @@ static struct thread *thread_trim_head(struct thread_list *list) /* Move thread to unuse list. */ static void thread_add_unuse(struct thread_master *m, struct thread *thread) { + pthread_mutex_t mtxc = thread->mtx; + assert(m != NULL && thread != NULL); assert(thread->next == NULL); assert(thread->prev == NULL); @@ -545,10 +548,15 @@ static void thread_add_unuse(struct thread_master *m, struct thread *thread) memset(thread, 0, sizeof(struct thread)); thread->type = THREAD_UNUSED; - if (m->unuse.count < THREAD_UNUSED_DEPTH) + /* Restore the thread mutex context. */ + thread->mtx = mtxc; + + if (m->unuse.count < THREAD_UNUSED_DEPTH) { thread_list_add(&m->unuse, thread); - else - XFREE(MTYPE_THREAD, thread); + return; + } + + thread_free(m, thread); } /* Free all unused thread. */ @@ -559,9 +567,8 @@ static void thread_list_free(struct thread_master *m, struct thread_list *list) for (t = list->head; t; t = next) { next = t->next; - XFREE(MTYPE_THREAD, t); + thread_free(m, t); list->count--; - m->alloc--; } } @@ -575,8 +582,7 @@ static void thread_array_free(struct thread_master *m, t = thread_array[index]; if (t) { thread_array[index] = NULL; - XFREE(MTYPE_THREAD, t); - m->alloc--; + thread_free(m, t); } } XFREE(MTYPE_THREAD_POLL, thread_array); @@ -587,9 +593,8 @@ static void thread_queue_free(struct thread_master *m, struct pqueue *queue) int i; for (i = 0; i < queue->size; i++) - XFREE(MTYPE_THREAD, queue->array[i]); + thread_free(m, queue->array[i]); - m->alloc -= queue->size; pqueue_delete(queue); } @@ -607,8 +612,7 @@ void thread_master_free_unused(struct thread_master *m) { struct thread *t; while ((t = thread_trim_head(&m->unuse)) != NULL) { - pthread_mutex_destroy(&t->mtx); - XFREE(MTYPE_THREAD, t); + thread_free(m, t); } } pthread_mutex_unlock(&m->mtx); @@ -727,6 +731,17 @@ static struct thread *thread_get(struct thread_master *m, uint8_t type, return thread; } +static void thread_free(struct thread_master *master, struct thread *thread) +{ + /* Update statistics. */ + assert(master->alloc > 0); + master->alloc--; + + /* Free allocated resources. */ + pthread_mutex_destroy(&thread->mtx); + XFREE(MTYPE_THREAD, thread); +} + static int fd_poll(struct thread_master *m, struct pollfd *pfds, nfds_t pfdsize, nfds_t count, const struct timeval *timer_wait) { From 3081869800508e418a4f8f6cef69cf3dfd1f5997 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Mon, 1 Oct 2018 13:38:34 -0300 Subject: [PATCH 2/2] lib: refactor thread_execute Don't allocate threads in the stack, but use the standardized `thread_get` and `thread_add_unused` to avoid creating corner cases in the thread API. This fixes a thread mutex memory leak in FreeBSD. Signed-off-by: Rafael Zalamena (cherry picked from commit c4345fbf71fcc7ef4b64d95979f252c613dd3ebf) --- lib/thread.c | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/lib/thread.c b/lib/thread.c index 78e416f363..2eefe99146 100644 --- a/lib/thread.c +++ b/lib/thread.c @@ -1645,25 +1645,27 @@ void funcname_thread_execute(struct thread_master *m, int (*func)(struct thread *), void *arg, int val, debugargdef) { - struct cpu_thread_history tmp; - struct thread dummy; + struct thread *thread; - memset(&dummy, 0, sizeof(struct thread)); + /* Get or allocate new thread to execute. */ + pthread_mutex_lock(&m->mtx); + { + thread = thread_get(m, THREAD_EVENT, func, arg, debugargpass); - pthread_mutex_init(&dummy.mtx, NULL); - dummy.type = THREAD_EVENT; - dummy.add_type = THREAD_EXECUTE; - dummy.master = NULL; - dummy.arg = arg; - dummy.u.val = val; + /* Set its event value. */ + pthread_mutex_lock(&thread->mtx); + { + thread->add_type = THREAD_EXECUTE; + thread->u.val = val; + thread->ref = &thread; + } + pthread_mutex_unlock(&thread->mtx); + } + pthread_mutex_unlock(&m->mtx); - tmp.func = dummy.func = func; - tmp.funcname = dummy.funcname = funcname; - dummy.hist = hash_get(m->cpu_record, &tmp, - (void *(*)(void *))cpu_record_hash_alloc); + /* Execute thread doing all accounting. */ + thread_call(thread); - dummy.schedfrom = schedfrom; - dummy.schedfrom_line = fromln; - - thread_call(&dummy); + /* Give back or free thread. */ + thread_add_unuse(m, thread); }