diff --git a/src/core/lv_obj_event.c b/src/core/lv_obj_event.c index 3831feeb56..cef3723873 100644 --- a/src/core/lv_obj_event.c +++ b/src/core/lv_obj_event.c @@ -132,7 +132,7 @@ bool lv_obj_remove_event_cb(lv_obj_t * obj, lv_event_cb_t event_cb) uint32_t i; for(i = 0; i < event_cnt; i++) { lv_event_dsc_t * dsc = lv_obj_get_event_dsc(obj, i); - if(dsc->cb == event_cb) { + if(dsc && dsc->cb == event_cb) { lv_obj_remove_event(obj, i); return true; } diff --git a/src/misc/lv_event.c b/src/misc/lv_event.c index ec985ed25c..134882c35c 100644 --- a/src/misc/lv_event.c +++ b/src/misc/lv_event.c @@ -27,6 +27,15 @@ * STATIC PROTOTYPES **********************/ +/* Traverse the list to delete the objects marked for deletion */ +static void cleanup_event_list(lv_event_list_t * list); +static void cleanup_event_list_core(lv_array_t * array); + +static void event_mark_deleting(lv_event_list_t * list, lv_event_dsc_t * dsc); +static bool event_is_marked_deleting(lv_event_dsc_t * dsc); +static uint32_t event_array_size(lv_event_list_t * list); +static lv_event_dsc_t ** event_array_at(lv_event_list_t * list, uint32_t index); + /********************** * STATIC VARIABLES **********************/ @@ -63,26 +72,49 @@ 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) { if(list == NULL) return LV_RESULT_OK; + if(e->deleted) return LV_RESULT_INVALID; - uint32_t i = 0; - 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; + /* When obj is deleted in its own event, it will cause the `list->array` header to be released, + * but the content still exists, which leads to memory leakage. + * Therefore, back up the header in advance, + * which can strive to release the memory and prevent used-after-free. */ + lv_array_t back_array_head = list->array; + + /* Dealing with the problem of nested event deletion event */ + const bool is_traversing = list->is_traversing; + list->is_traversing = true; + + lv_result_t res = LV_RESULT_OK; + const uint32_t size = event_array_size(list); + for(uint32_t i = 0; i < size && !e->deleted; i++) { + lv_event_dsc_t * dsc = *event_array_at(list, i); + if(dsc->cb == NULL) continue; + if(event_is_marked_deleting(dsc)) continue; + const bool is_preprocessed = (dsc->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->filter & ~LV_EVENT_PREPROCESS; if(filter == LV_EVENT_ALL || filter == e->code) { - e->user_data = dsc[i]->user_data; - dsc[i]->cb(e); - if(e->stop_processing) return LV_RESULT_OK; + e->user_data = dsc->user_data; + dsc->cb(e); + if(e->stop_processing) break; /*Stop if the object is deleted*/ - if(e->deleted) return LV_RESULT_INVALID; - + if(e->deleted) { + res = LV_RESULT_INVALID; + break; + } } } - return LV_RESULT_OK; + + if(is_traversing) return res; + + if(e->deleted) cleanup_event_list_core(&back_array_head); + else { + list->is_traversing = false; + cleanup_event_list(list); + } + + return res; } lv_event_dsc_t * lv_event_add(lv_event_list_t * list, lv_event_cb_t cb, lv_event_code_t filter, @@ -95,12 +127,12 @@ lv_event_dsc_t * lv_event_add(lv_event_list_t * list, lv_event_cb_t cb, lv_event dsc->filter = filter; dsc->user_data = user_data; - if(lv_array_size(list) == 0) { + if(event_array_size(list) == 0) { /*event list hasn't been initialized.*/ - lv_array_init(list, 1, sizeof(lv_event_dsc_t *)); + lv_array_init(&list->array, 1, sizeof(lv_event_dsc_t *)); } - lv_array_push_back(list, &dsc); + lv_array_push_back(&list->array, &dsc); return dsc; } @@ -109,12 +141,12 @@ 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); + const uint32_t size = event_array_size(list); + for(uint32_t i = 0; i < size; i++) { + lv_event_dsc_t * event = *event_array_at(list, i); + if(event == dsc) { + event_mark_deleting(list, event); + cleanup_event_list(list); return true; } } @@ -125,14 +157,13 @@ 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) { LV_ASSERT_NULL(list); - return lv_array_size(list); + return event_array_size(list); } lv_event_dsc_t * lv_event_get_dsc(lv_event_list_t * list, uint32_t index) { LV_ASSERT_NULL(list); - lv_event_dsc_t ** dsc; - dsc = lv_array_at(list, index); + lv_event_dsc_t ** dsc = event_array_at(list, index); return dsc ? *dsc : NULL; } @@ -153,19 +184,19 @@ 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); + if(dsc == NULL) return false; + event_mark_deleting(list, dsc); + cleanup_event_list(list); + return true; } 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); + const uint32_t size = event_array_size(list); + for(uint32_t i = 0; i < size; i++) + event_mark_deleting(list, *event_array_at(list, i)); + cleanup_event_list(list); } void * lv_event_get_current_target(lv_event_t * e) @@ -222,3 +253,49 @@ void lv_event_mark_deleted(void * target) /********************** * STATIC FUNCTIONS **********************/ + +static void cleanup_event_list_core(lv_array_t * array) +{ + const uint32_t size = lv_array_size(array); + uint32_t kept_count = 0; + for(uint32_t i = 0; i < size; i++) { + lv_event_dsc_t ** dsc_i = lv_array_at(array, i); + lv_event_dsc_t ** dsc_kept = lv_array_at(array, kept_count); + if(event_is_marked_deleting(*dsc_i)) lv_free(*dsc_i); + else { + *dsc_kept = *dsc_i; + kept_count++; + } + } + + if(kept_count == 0) lv_array_deinit(array); + else lv_array_resize(array, kept_count); +} + +static void cleanup_event_list(lv_event_list_t * list) +{ + if(list->is_traversing) return; + if(list->has_marked_deleting == false) return; + + cleanup_event_list_core(&list->array); + + list->has_marked_deleting = false; +} + +static void event_mark_deleting(lv_event_list_t * list, lv_event_dsc_t * dsc) +{ + list->has_marked_deleting = true; + dsc->filter |= LV_EVENT_MARKED_DELETING; +} +static bool event_is_marked_deleting(lv_event_dsc_t * dsc) +{ + return (dsc->filter & LV_EVENT_MARKED_DELETING) != 0; +} +static uint32_t event_array_size(lv_event_list_t * list) +{ + return lv_array_size(&list->array); +} +static lv_event_dsc_t ** event_array_at(lv_event_list_t * list, uint32_t index) +{ + return lv_array_at(&list->array, index); +} diff --git a/src/misc/lv_event.h b/src/misc/lv_event.h index 61518e96a1..c6610209bb 100644 --- a/src/misc/lv_event.h +++ b/src/misc/lv_event.h @@ -111,9 +111,15 @@ typedef enum { LV_EVENT_PREPROCESS = 0x8000, /** This is a flag that can be set with an event so it's processed before the class default event processing */ + LV_EVENT_MARKED_DELETING = 0x10000, } lv_event_code_t; -typedef lv_array_t lv_event_list_t; +typedef struct { + lv_array_t array; + uint8_t is_traversing: 1; /**< True: the list is being nested traversed */ + uint8_t has_marked_deleting: 1; /**< True: the list has marked deleting objects + when some of events are marked as deleting */ +} lv_event_list_t; /** * @brief Event callback. diff --git a/src/others/observer/lv_observer.c b/src/others/observer/lv_observer.c index 8d43f52de7..8538b2f11c 100644 --- a/src/others/observer/lv_observer.c +++ b/src/others/observer/lv_observer.c @@ -376,7 +376,7 @@ void lv_observer_remove(lv_observer_t * observer) void lv_obj_remove_from_subject(lv_obj_t * obj, lv_subject_t * subject) { int32_t i; - int32_t event_cnt = (int32_t)(obj->spec_attr ? lv_array_size(&obj->spec_attr->event_list) : 0); + int32_t event_cnt = (int32_t)(obj->spec_attr ? lv_event_get_count(&obj->spec_attr->event_list) : 0); for(i = event_cnt - 1; i >= 0; i--) { lv_event_dsc_t * event_dsc = lv_obj_get_event_dsc(obj, i); if(event_dsc->cb == unsubscribe_on_delete_cb) { diff --git a/tests/src/test_cases/test_event.c b/tests/src/test_cases/test_event.c index ee5186dda6..3f0dd34073 100644 --- a/tests/src/test_cases/test_event.c +++ b/tests/src/test_cases/test_event.c @@ -122,4 +122,25 @@ void test_event_stop_processing(void) TEST_ASSERT_EQUAL(post_cnt_2, 0); } +static uint32_t click_count = 0; +static void event_click_to_delete_cb(lv_event_t * e) +{ + lv_obj_t * obj = lv_event_get_target(e); + click_count++; + + if(click_count == 10) lv_obj_remove_event(obj, 0); + else if(click_count == 15) lv_obj_delete(obj); + else lv_obj_send_event(obj, LV_EVENT_CLICKED, NULL); +} + +void test_event_delete_obj_in_recursive_event_call(void) +{ + lv_obj_t * obj = lv_obj_create(lv_screen_active()); + lv_obj_set_size(obj, 200, 100); + lv_obj_add_event_cb(obj, event_click_to_delete_cb, LV_EVENT_CLICKED, NULL); + lv_obj_add_event_cb(obj, NULL, LV_EVENT_CLICKED, NULL); + lv_obj_add_event_cb(obj, event_click_to_delete_cb, LV_EVENT_CLICKED, NULL); + lv_test_mouse_click_at(30, 30); +} + #endif