feat(event): return event dsc for later to remove (#5630)

Signed-off-by: Neo Xu <neo.xu1990@gmail.com>
Signed-off-by: Xu Xingliang <xuxingliang@xiaomi.com>

.github/workflows/ccpp.yml

@@ -95,7 +95,7 @@ jobs:
 
           install: |
             apt-get update -y
-            apt-get install build-essential ccache libgcc-10-dev python3 libpng-dev ruby-full gcovr cmake libjpeg62-turbo-dev libfreetype6-dev libasan6 pngquant python3-pip libinput-dev libxkbcommon-dev pkg-config -q -y
+            apt-get install build-essential ccache libgcc-10-dev python3 libpng-dev ruby-full gcovr cmake libjpeg62-turbo-dev libfreetype6-dev libasan6 pngquant python3-pip libinput-dev libxkbcommon-dev pkg-config ninja-build -q -y
             pip install pypng lz4
             /usr/sbin/update-ccache-symlinks
             echo 'export PATH="/usr/lib/ccache:$PATH"' | tee -a ~/.bashrc
@@ -110,4 +110,3 @@ jobs:
             path: |
                 tests/ref_imgs/**/*_err.png
                 test_screenshot_error.h
-

scripts/install-prerequisites.sh

@@ -6,5 +6,5 @@
 #
 # Note: This script is run by the CI workflows.
 sudo apt update
-sudo apt install gcc python3 libpng-dev ruby-full gcovr cmake libjpeg-turbo8-dev libfreetype6-dev pngquant libinput-dev libxkbcommon-dev pkg-config
+sudo apt install gcc python3 ninja-build libpng-dev ruby-full gcovr cmake libjpeg-turbo8-dev libfreetype6-dev pngquant libinput-dev libxkbcommon-dev pkg-config
 pip3 install pypng lz4

src/core/lv_obj_event.c

@@ -92,13 +92,12 @@ lv_result_t lv_obj_event_base(const lv_obj_class_t * class_p, lv_event_t * e)
     return res;
 }
 
-void lv_obj_add_event_cb(lv_obj_t * obj, lv_event_cb_t event_cb, lv_event_code_t filter,
-                         void * user_data)
+lv_event_dsc_t * lv_obj_add_event_cb(lv_obj_t * obj, lv_event_cb_t event_cb, lv_event_code_t filter, void * user_data)
 {
     LV_ASSERT_OBJ(obj, MY_CLASS);
     lv_obj_allocate_spec_attr(obj);
 
-    lv_event_add(&obj->spec_attr->event_list, event_cb, filter, user_data);
+    return lv_event_add(&obj->spec_attr->event_list, event_cb, filter, user_data);
 }
 
 uint32_t lv_obj_get_event_count(lv_obj_t * obj)
@@ -139,6 +138,14 @@ bool lv_obj_remove_event_cb(lv_obj_t * obj, lv_event_cb_t event_cb)
     return false;
 }
 
+bool lv_obj_remove_event_dsc(lv_obj_t * obj, lv_event_dsc_t * dsc)
+{
+    LV_ASSERT_NULL(obj);
+    LV_ASSERT_NULL(dsc);
+    if(obj->spec_attr == NULL) return false;
+    return lv_event_remove_dsc(&obj->spec_attr->event_list, dsc);
+}
+
 uint32_t lv_obj_remove_event_cb_with_user_data(lv_obj_t * obj, lv_event_cb_t event_cb, void * user_data)
 {
     LV_ASSERT_NULL(obj);

src/core/lv_obj_event.h

@@ -99,9 +99,9 @@ lv_obj_t * lv_event_get_target_obj(lv_event_t * e);
  * @param filter    an event code (e.g. `LV_EVENT_CLICKED`) on which the event should be called. `LV_EVENT_ALL` can be used to receive all the events.
  * @param event_cb  the new event function
  * @param           user_data custom data data will be available in `event_cb`
+ * @return          handler to the event. It can be used in `lv_obj_remove_event_dsc`.
  */
-void lv_obj_add_event_cb(lv_obj_t * obj, lv_event_cb_t event_cb, lv_event_code_t filter,
-                         void * user_data);
+lv_event_dsc_t * lv_obj_add_event_cb(lv_obj_t * obj, lv_event_cb_t event_cb, lv_event_code_t filter, void * user_data);
 
 uint32_t lv_obj_get_event_count(lv_obj_t * obj);
 
@@ -111,6 +111,8 @@ bool lv_obj_remove_event(lv_obj_t * obj, uint32_t index);
 
 bool lv_obj_remove_event_cb(lv_obj_t * obj, lv_event_cb_t event_cb);
 
+bool lv_obj_remove_event_dsc(lv_obj_t * obj, lv_event_dsc_t * dsc);
+
 /**
  * Remove an event_cb with user_data
  * @param obj           pointer to a obj

src/misc/lv_event.c

@@ -65,16 +65,16 @@ lv_result_t lv_event_send(lv_event_list_t * list, lv_event_t * e, bool preproces
     if(list == NULL) return LV_RESULT_OK;
 
     uint32_t i = 0;
-    lv_event_dsc_t * dsc = lv_array_front(list);
+    lv_event_dsc_t ** dsc = lv_array_front(list);
     uint32_t size = lv_array_size(list);
     for(i = 0; i < size; i++) {
-        if(dsc[i].cb == NULL) continue;
-        bool is_preprocessed = (dsc[i].filter & LV_EVENT_PREPROCESS) != 0;
+        if(dsc[i]->cb == NULL) continue;
+        bool is_preprocessed = (dsc[i]->filter & LV_EVENT_PREPROCESS) != 0;
         if(is_preprocessed != preprocess) continue;
-        lv_event_code_t filter = dsc[i].filter & ~LV_EVENT_PREPROCESS;
+        lv_event_code_t filter = dsc[i]->filter & ~LV_EVENT_PREPROCESS;
         if(filter == LV_EVENT_ALL || filter == e->code) {
-            e->user_data = dsc[i].user_data;
-            dsc[i].cb(e);
+            e->user_data = dsc[i]->user_data;
+            dsc[i]->cb(e);
             if(e->stop_processing) return LV_RESULT_OK;
 
             /*Stop if the object is deleted*/
@@ -85,20 +85,41 @@ lv_result_t lv_event_send(lv_event_list_t * list, lv_event_t * e, bool preproces
     return LV_RESULT_OK;
 }
 
-void lv_event_add(lv_event_list_t * list, lv_event_cb_t cb, lv_event_code_t filter,
+lv_event_dsc_t * lv_event_add(lv_event_list_t * list, lv_event_cb_t cb, lv_event_code_t filter,
                               void * user_data)
 {
-    lv_event_dsc_t dsc = { 0 };
-    dsc.cb = cb;
-    dsc.filter = filter;
-    dsc.user_data = user_data;
+    lv_event_dsc_t * dsc = lv_malloc(sizeof(lv_event_dsc_t));
+    LV_ASSERT_NULL(dsc);
+
+    dsc->cb = cb;
+    dsc->filter = filter;
+    dsc->user_data = user_data;
 
     if(lv_array_size(list) == 0) {
         /*event list hasn't been initialized.*/
-        lv_array_init(list, 1, sizeof(lv_event_dsc_t));
+        lv_array_init(list, 1, sizeof(lv_event_dsc_t *));
     }
 
     lv_array_push_back(list, &dsc);
+    return dsc;
+}
+
+bool lv_event_remove_dsc(lv_event_list_t * list, lv_event_dsc_t * dsc)
+{
+    LV_ASSERT_NULL(list);
+    LV_ASSERT_NULL(dsc);
+
+    int size = lv_array_size(list);
+    lv_event_dsc_t ** events = lv_array_front(list);
+    for(int i = 0; i < size; i++) {
+        if(events[i] == dsc) {
+            lv_free(dsc);
+            lv_array_remove(list, i);
+            return true;
+        }
+    }
+
+    return false;
 }
 
 uint32_t lv_event_get_count(lv_event_list_t * list)
@@ -110,7 +131,9 @@ uint32_t lv_event_get_count(lv_event_list_t * list)
 lv_event_dsc_t * lv_event_get_dsc(lv_event_list_t * list, uint32_t index)
 {
     LV_ASSERT_NULL(list);
-    return lv_array_at(list, index);
+    lv_event_dsc_t ** dsc;
+    dsc = lv_array_at(list, index);
+    return dsc ? *dsc : NULL;
 }
 
 lv_event_cb_t lv_event_dsc_get_cb(lv_event_dsc_t * dsc)
@@ -129,12 +152,19 @@ void * lv_event_dsc_get_user_data(lv_event_dsc_t * dsc)
 bool lv_event_remove(lv_event_list_t * list, uint32_t index)
 {
     LV_ASSERT_NULL(list);
+    lv_event_dsc_t * dsc = lv_event_get_dsc(list, index);
+    lv_free(dsc);
     return lv_array_remove(list, index);
 }
 
 void lv_event_remove_all(lv_event_list_t * list)
 {
     LV_ASSERT_NULL(list);
+    int size = lv_array_size(list);
+    lv_event_dsc_t ** dsc = lv_array_front(list);
+    for(int i = 0; i < size; i++) {
+        lv_free(dsc[i]);
+    }
     lv_array_deinit(list);
 }
 

src/misc/lv_event.h

@@ -148,7 +148,8 @@ void _lv_event_pop(lv_event_t * e);
 
 lv_result_t lv_event_send(lv_event_list_t * list, lv_event_t * e, bool preprocess);
 
-void lv_event_add(lv_event_list_t * list, lv_event_cb_t cb, lv_event_code_t filter, void * user_data);
+lv_event_dsc_t * lv_event_add(lv_event_list_t * list, lv_event_cb_t cb, lv_event_code_t filter, void * user_data);
+bool lv_event_remove_dsc(lv_event_list_t * list, lv_event_dsc_t * dsc);
 
 uint32_t lv_event_get_count(lv_event_list_t * list);
 

tests/main.py

@@ -90,7 +90,7 @@ def build_tests(options_name, build_type, clean):
         created_build_dir = True
     os.chdir(build_dir)
     if created_build_dir:
-        subprocess.check_call(['cmake', '-DCMAKE_BUILD_TYPE=%s' % build_type,
+        subprocess.check_call(['cmake', '-GNinja', '-DCMAKE_BUILD_TYPE=%s' % build_type,
                                '-D%s=1' % options_name, '..'])
     subprocess.check_call(['cmake', '--build', build_dir,
                            '--parallel', str(os.cpu_count())])

tests/src/test_cases/test_event.c

@@ -23,4 +23,23 @@ void test_event_object_deletion(void)
     lv_obj_send_event(obj, LV_EVENT_VALUE_CHANGED, NULL);
 }
 
+/* Add and then remove event should not memory leak */
+void test_event_should_not_memory_lean(void)
+{
+    lv_mem_monitor_t monitor;
+    lv_mem_monitor(&monitor);
+    lv_obj_t * obj = lv_obj_create(lv_screen_active());
+    uint32_t initial_free_size = monitor.free_size;
+
+    for(int i = 0; i < 10; i++) {
+        lv_obj_add_event_cb(obj, NULL, LV_EVENT_ALL, NULL);
+    }
+
+    lv_obj_delete(obj);
+
+    lv_mem_monitor_t m2;
+    lv_mem_monitor(&m2);
+    TEST_ASSERT_LESS_OR_EQUAL_CHAR(initial_free_size, m2.free_size);
+}
+
 #endif

tests/src/test_cases/widgets/test_switch.c

@@ -50,12 +50,9 @@ void test_switch_should_not_leak_memory_after_deletion(void)
 {
     size_t idx = 0;
     uint32_t initial_available_memory = 0;
-    uint32_t final_available_memory = 0;
-    lv_mem_monitor_t monitor;
     lv_obj_t * switches[SWITCHES_CNT] = {NULL};
 
-    lv_mem_monitor(&monitor);
-    initial_available_memory = monitor.free_size;
+    initial_available_memory = lv_test_get_free_mem();
 
     for(idx = 0; idx < SWITCHES_CNT; idx++) {
         switches[idx] = lv_switch_create(scr);
@@ -65,10 +62,7 @@ void test_switch_should_not_leak_memory_after_deletion(void)
         lv_obj_delete(switches[idx]);
     }
 
-    lv_mem_monitor(&monitor);
-    final_available_memory = monitor.free_size;
-
-    LV_HEAP_CHECK(TEST_ASSERT_LESS_OR_EQUAL(initial_available_memory, final_available_memory));
+    LV_HEAP_CHECK(TEST_ASSERT_MEM_LEAK_LESS_THAN(initial_available_memory, 24));
 }
 
 void test_switch_animation(void)

tests/unity/unity_support.h

@@ -8,6 +8,7 @@ extern "C" {
 
 #include <stdbool.h>
 #include "../../lvgl.h"
+#include "../src/lv_test_helpers.h"
 
 bool lv_test_assert_image_eq(const char * fn_ref);
 
@@ -38,11 +39,10 @@ bool lv_test_assert_image_eq(const char * fn_ref);
 #  define TEST_ASSERT_EQUAL_COLOR32_MESSAGE(c1, c2, msg)      TEST_ASSERT_TRUE(lv_color32_eq(c1, c2), msg)
 
 
-#  define TEST_ASSERT_MEM_LEAK_LESS_THAN(prev_usage, threshold)  TEST_ASSERT_LESS_THAN(threshold, LV_ABS((int64_t)(prev_usage) - (int64_t)lv_test_get_free_mem()));
+#  define TEST_ASSERT_MEM_LEAK_LESS_THAN(prev_usage, threshold)  TEST_ASSERT_LESS_OR_EQUAL(threshold, LV_ABS((int64_t)(prev_usage) - (int64_t)lv_test_get_free_mem()));
 
 #ifdef __cplusplus
 } /*extern "C"*/
 #endif
 
 #endif /*LV_UNITY_SUPPORT_H*/
-