From 5892dac2f7de185c2b0f2c9c00fdd2c5e985a9dc Mon Sep 17 00:00:00 2001
From: liubo <liubo254@huawei.com>
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 <liubo254@huawei.com>
---
 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 <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include <malloc.h>
 #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, &timespec) != 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