From 5892dac2f7de185c2b0f2c9c00fdd2c5e985a9dc Mon Sep 17 00:00:00 2001 From: liubo Date: Wed, 31 May 2023 17:12:57 +0800 Subject: [PATCH 3/4] etmem: fix memory leak and fd leak fix memory leak and fp leak. When user stop the task while task is running, the routine will run pthread_cancel to stop the task. memory of vmas struct may leak when this happens. Set thread cannot be canceled when main routine is running. Signed-off-by: liubo --- etmem/src/etmemd_src/etmemd_migrate.c | 3 +++ etmem/src/etmemd_src/etmemd_rpc.c | 7 +++++- etmem/src/etmemd_src/etmemd_scan.c | 8 ++----- etmem/src/etmemd_src/etmemd_slide.c | 29 +++++++++++------------ etmem/src/etmemd_src/etmemd_threadpool.c | 1 + etmem/src/etmemd_src/etmemd_threadtimer.c | 13 +++++++--- 6 files changed, 36 insertions(+), 25 deletions(-) diff --git a/etmem/src/etmemd_src/etmemd_migrate.c b/etmem/src/etmemd_src/etmemd_migrate.c index a143c5e..5f41876 100644 --- a/etmem/src/etmemd_src/etmemd_migrate.c +++ b/etmem/src/etmemd_src/etmemd_migrate.c @@ -98,6 +98,7 @@ static int etmemd_migrate_mem(const char *pid, const char *grade_path, struct pa etmemd_log(ETMEMD_LOG_DEBUG, "migrate failed for pid %s, check if etmem_swap.ko installed\n", pid); free(swap_str); fclose(fp); + fp = NULL; return -1; } free(swap_str); @@ -105,6 +106,8 @@ static int etmemd_migrate_mem(const char *pid, const char *grade_path, struct pa } fclose(fp); + fp = NULL; + return 0; } diff --git a/etmem/src/etmemd_src/etmemd_rpc.c b/etmem/src/etmemd_src/etmemd_rpc.c index 780ddce..100a7bd 100644 --- a/etmem/src/etmemd_src/etmemd_rpc.c +++ b/etmem/src/etmemd_src/etmemd_rpc.c @@ -22,6 +22,7 @@ #include #include #include +#include #include "securec.h" #include "etmemd_rpc.h" #include "etmemd_project.h" @@ -162,7 +163,11 @@ static struct obj_cmd_item obj_remove_items[] = { static enum opt_result do_obj_remove(GKeyFile *config) { - return do_obj_cmd(config, obj_remove_items, ARRAY_SIZE(obj_remove_items), false); + enum opt_result ret; + + ret = do_obj_cmd(config, obj_remove_items, ARRAY_SIZE(obj_remove_items), false); + (void)malloc_trim(0); + return ret; } static enum opt_result handle_obj_cmd(char *file_name, enum cmd_type type) diff --git a/etmem/src/etmemd_src/etmemd_scan.c b/etmem/src/etmemd_src/etmemd_scan.c index e06ba92..699b1cd 100644 --- a/etmem/src/etmemd_src/etmemd_scan.c +++ b/etmem/src/etmemd_src/etmemd_scan.c @@ -347,12 +347,7 @@ struct vmas *get_vmas_with_flags(const char *pid, char *vmflags_array[], int vmf size_t len; char *maps_file = NULL; - if (vmflags_num == 0) { - maps_file = MAPS_FILE; - } else { - maps_file = SMAPS_FILE; - } - + maps_file = (vmflags_num == 0) ? MAPS_FILE : SMAPS_FILE; ret_vmas = (struct vmas *)calloc(1, sizeof(struct vmas)); if (ret_vmas == NULL) { etmemd_log(ETMEMD_LOG_ERR, "malloc for vmas fail\n"); @@ -397,6 +392,7 @@ struct vmas *get_vmas_with_flags(const char *pid, char *vmflags_array[], int vmf } fclose(fp); + fp = NULL; return ret_vmas; } diff --git a/etmem/src/etmemd_src/etmemd_slide.c b/etmem/src/etmemd_src/etmemd_slide.c index a3c474b..1a11f45 100644 --- a/etmem/src/etmemd_src/etmemd_slide.c +++ b/etmem/src/etmemd_src/etmemd_slide.c @@ -216,11 +216,10 @@ static void *slide_executor(void *arg) return NULL; } - /* register cleanup function in case of unexpected cancellation detected, - * and register for memory_grade first, because it needs to clean after page_refs is cleaned */ - pthread_cleanup_push(clean_memory_grade_unexpected, &memory_grade); - pthread_cleanup_push(clean_page_refs_unexpected, &page_refs); - pthread_cleanup_push(clean_page_sort_unexpected, &page_sort); + if (pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL) != 0) { + etmemd_log(ETMEMD_LOG_ERR, "failed to set pthread cancel state.\n"); + return NULL; + } page_refs = etmemd_do_scan(tk_pid, tk_pid->tk); if (page_refs == NULL) { @@ -237,15 +236,10 @@ static void *slide_executor(void *arg) memory_grade = slide_policy_interface(&page_sort, tk_pid); scan_out: - /* clean up page_sort linked array */ - pthread_cleanup_pop(1); + clean_page_sort_unexpected(&page_sort); - /* no need to use page_refs any longer. - * pop the cleanup function with parameter 1, because the items in page_refs list will be moved - * into the at least on list of memory_grade after polidy function called if no problems happened, - * but mig_policy_func() may fails to move page_refs in rare cases. - * It will do nothing if page_refs is NULL */ - pthread_cleanup_pop(1); + /* no need to use page_refs any longer. */ + clean_page_refs_unexpected(&page_refs); if (memory_grade == NULL) { etmemd_log(ETMEMD_LOG_DEBUG, "pid %u memory grade is empty\n", tk_pid->pid); @@ -261,12 +255,17 @@ scan_out: } exit: - /* clean memory_grade here */ - pthread_cleanup_pop(1); + clean_memory_grade_unexpected(&memory_grade); + if (malloc_trim(0) == 0) { etmemd_log(ETMEMD_LOG_INFO, "malloc_trim to release memory for pid %u fail\n", tk_pid->pid); } + if (pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL) != 0) { + etmemd_log(ETMEMD_LOG_DEBUG, "pthread_setcancelstate PTHREAD_CANCEL_ENABLE failed.\n"); + } + pthread_testcancel(); + return NULL; } diff --git a/etmem/src/etmemd_src/etmemd_threadpool.c b/etmem/src/etmemd_src/etmemd_threadpool.c index dac42d1..4375ca1 100644 --- a/etmem/src/etmemd_src/etmemd_threadpool.c +++ b/etmem/src/etmemd_src/etmemd_threadpool.c @@ -48,6 +48,7 @@ static void threadpool_cancel_unlock(void *arg) } etmemd_log(ETMEMD_LOG_DEBUG, "unlock for threadpool once\n"); pthread_mutex_unlock(g_pool_lock); + g_pool_lock = NULL; } static void *threadpool_routine(void *arg) diff --git a/etmem/src/etmemd_src/etmemd_threadtimer.c b/etmem/src/etmemd_src/etmemd_threadtimer.c index d18b3e0..4014c72 100644 --- a/etmem/src/etmemd_src/etmemd_threadtimer.c +++ b/etmem/src/etmemd_src/etmemd_threadtimer.c @@ -37,7 +37,11 @@ static void *thread_timer_routine(void *arg) expired_time = timer->expired_time; - pthread_cleanup_push(threadtimer_cancel_unlock, &timer->cond_mutex); + if (pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL) != 0) { + etmemd_log(ETMEMD_LOG_ERR, "failed to set pthread cancel state.\n"); + return NULL; + } + pthread_mutex_lock(&timer->cond_mutex); while (!timer->down) { if (clock_gettime(CLOCK_MONOTONIC, ×pec) != 0) { @@ -60,9 +64,12 @@ static void *thread_timer_routine(void *arg) break; } } - /* unlock th timer->cond_mutex */ - pthread_cleanup_pop(1); + threadtimer_cancel_unlock(&timer->cond_mutex); + if (pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL) != 0) { + etmemd_log(ETMEMD_LOG_DEBUG, "pthread_setcancelstate PTHREAD_CANCEL_ENABLE failed.\n"); + } + pthread_testcancel(); pthread_exit(NULL); } -- 2.33.0