Not use the lv_obj_is_valid() in ui_theme_manager.c

kanbaiStaok · July 31, 2024, 6:33am

What do you want to achieve?

this API is not safe, see:

github.com/lvgl/lvgl

Crash in _lv_event_mark_deleted() when call lv_obj_del() or lv_obj_del_async

opened 06:15AM - 07 Apr 24 UTC

closed 08:17AM - 23 Apr 24 UTC

t-walker-wei

stale

### LVGL version v8.3.10 ### What happened? I work with Platformio in VS Code… **Board**: ESP32-DOIT-DEVKIT-V1 **Platform**: espressif32 **Framework**: arduino I'm creating a GUI with creat new widgets and delete old widgets to save memory in ESP32-DOIT-DEVKIT-V1 with 320KB ram. When I called lv_obj_del() or lv_obj_del_async() in button clicked event callback, I got my device crash frequently with panic ifno: ``` Guru Meditation Error: Core 1 panic'ed (LoadProhibited). Exception was unhandled. Core 1 register dump: PC : 0x400ee8a6 PS : 0x00060530 A0 : 0x800f1d90 A1 : 0x3ffd0600 A2 : 0x3ffbc464 A3 : 0x00040018 A4 : 0x000001c5 A5 : 0x3f40f874 A6 : 0x3f40f818 A7 : 0x00000012 A8 : 0x800ee8a6 A9 : 0x3ffd05a0 A10 : 0x00004f69 A11 : 0x3f40f530 A12 : 0x0000009d A13 : 0x3f40f64c A14 : 0x3f40f550 A15 : 0x00000012 SAR : 0x00000004 EXCCAUSE: 0x0000001c EXCVADDR: 0x0004001c LBEG : 0x4008b301 LEND : 0x4008b311 LCOUNT : 0xfffffffd Backtrace: 0x400ee8a3:0x3ffd0600 0x400f1d8d:0x3ffd0620 0x400f2065:0x3ffd0640 0x400f2072:0x3ffd0660 0x400f6fc7:0x3ffd0680 0x400f6f53:0x3ffd06a0 0x400f7098:0x3ffd06c0 0x400dd49d:0x3ffd06e0 0x400ee77d:0x3ffd0700 0x400ee83e:0x3ffd0720 0x400ef5be:0x3ffd0760 0x400f00c2:0x3ffd0780 0x401019a9:0x3ffd07c0 0x40101a51:0x3ffd07e0 0x400d4a68:0x3ffd0800 0x40179b94:0x3ffd0820 #0 0x400ee8a3:0x3ffd0600 in _lv_event_mark_deleted at lib/lvgl/src/core/lv_event.c:157 #1 0x400f1d8d:0x3ffd0620 in lv_obj_destructor at lib/lvgl/src/core/lv_obj.c:454 #2 0x400f2065:0x3ffd0640 in _lv_obj_destruct at lib/lvgl/src/core/lv_obj_class.c:137 (discriminator 1) #3 0x400f2072:0x3ffd0660 in _lv_obj_destruct at lib/lvgl/src/core/lv_obj_class.c:144 #4 0x400f6fc7:0x3ffd0680 in obj_del_core at lib/lvgl/src/core/lv_obj_tree.c:397 #5 0x400f6f53:0x3ffd06a0 in obj_del_core at lib/lvgl/src/core/lv_obj_tree.c:374 #6 0x400f7098:0x3ffd06c0 in lv_obj_del at lib/lvgl/src/core/lv_obj_tree.c:68 #7 0x400dd49d:0x3ffd06e0 in k99_cancel_trans_btn_clicked_event_cb(_lv_event_t*) at src/ui/ui.cpp:2225 #8 0x400ee77d:0x3ffd0700 in event_send_core at lib/lvgl/src/core/lv_event.c:469 #9 0x400ee83e:0x3ffd0720 in lv_event_send at lib/lvgl/src/core/lv_event.c:75 #10 0x400ef5be:0x3ffd0760 in indev_keypad_proc at lib/lvgl/src/core/lv_indev.c:532 #11 0x400f00c2:0x3ffd0780 in lv_indev_read_timer_cb at lib/lvgl/src/core/lv_indev.c:104 #12 0x401019a9:0x3ffd07c0 in lv_timer_exec at lib/lvgl/src/misc/lv_timer.c:313 (discriminator 2) #13 0x40101a51:0x3ffd07e0 in lv_timer_handler at lib/lvgl/src/misc/lv_timer.c:109 #14 0x400d4a68:0x3ffd0800 in loop() at lib/lvgl/src/lv_api_map.h:37 (inlined by) loop() at src/main.cpp:2020 #15 0x40179b94:0x3ffd0820 in loopTask(void*) at C:/Users/offic/.platformio/packages/framework-arduinoespressif32/cores/esp32/main.cpp:50 ``` So I googled about **_lv_event_mark_deleted** and **lv_obj_del()**, but I didn't resolved my issue. I have tried few ways: 1. Try to use lv_obj_del_async() 2. Remove the timers for changing the subwidgets in this widget 3. I saw somebody said use mutex, but I call this lv_obj_del() function in event callback not task, so I cannot use mutex 4. Change to create two pages with parent null, use lv_scr_load_anim() to load new page and delete old page by set auto_del parameter to true After a bit search in the LVGL source code, I try to log what happen by modify _lv_event_mark_deleted() to log the event and event code and some where call by lv_obj_del() ( What I can do is only like this because I think maybe have some memory issue in my code, but I cannot debug with debug mode because I don't have JTAG/cmsis-dap/jlink ) ``` void _lv_event_mark_deleted(lv_obj_t * obj) { LV_LOG_WARN( "WALKER: _lv_event_mark_deleted() obj : %p", obj ); lv_event_t * e = event_head; while(e) { LV_LOG_WARN( "WALKER: _lv_event_mark_deleted() event -> code : %p -> %d", e, e->code ); if(e->current_target == obj || e->target == obj) e->deleted = 1; e = e->prev; } } ``` What I got when crash: ``` [Warn] (136.487, +10) _lv_event_mark_deleted: WALKER: _lv_event_mark_deleted() obj : 0x3ffbc6c8 (in lv_event.c line #153) [Warn] (136.498, +11) _lv_event_mark_deleted: WALKER: _lv_event_mark_deleted() event : 0x3ffd0724, 7 (in lv_event.c line #157) [Warn] (136.509, +11) _lv_event_mark_deleted: WALKER: _lv_event_mark_deleted() event : 0x3ffde8e4, -16777216 (in lv_event.c line #157) [Warn] (136.521, +12) _lv_event_mark_deleted: WALKER: _lv_event_mark_deleted() event : 0x3ffdea0c, 133821 (in lv_event.c line #157) [Warn] (136.533, +12) _lv_event_mark_deleted: WALKER: _lv_event_mark_deleted() event : 0x3ffdea0c, 133821 (in lv_event.c line #157) [Warn] (136.545, +12) _lv_event_mark_deleted: WALKER: _lv_event_mark_deleted() event : 0x3ffdea0c, 133821 (in lv_event.c line #157) [Warn] (136.556, +11) _lv_event_mark_deleted: WALKER: _lv_event_mark_deleted() event : 0x3ffdea0c, 133821 (in lv_event.c line #157) [Warn] (136.568, +12) _lv_event_mark_deleted: WALKER: _lv_event_mark_deleted() event : 0x3ffdea0c, 133821 (in lv_event.c line #157) [Warn] (136.580, +12) _lv_event_mark_deleted: WALKER: _lv_event_mark_deleted() event : 0x3ffdea0c, 133821 (in lv_event.c line #157) [Warn] (136.591, +11) _lv_event_mark_deleted: WALKER: _lv_event_mark_deleted() event : 0x3ffdea0c, 133821 (in lv_event.c line #157) [Warn] (136.603, +12) _lv_event_mark_deleted: WALKER: _lv_event_mark_deleted() event : 0x3ffdea0c, 133821 (in lv_event.c line #157) [Warn] (136.615, +12) _lv_event_mark_deleted: WALKER: _lv_event_mark_deleted() event : 0x3ffdea0c, 133821 (in lv_event.c line #157) [Warn] (136.627, +12) _lv_event_mark_deleted: WALKER: _lv_event_mark_deleted() event : 0x3ffdea0c, 133821 (in lv_event.c line #157) ``` The obj **0x3ffbc6c8** is first child (lv_label) of the widget I want to delete. I check the event codes in lv_event_code_t enum, the event code should be 0 to 45 and 0x80(int: 128), but what I got event code is -16777216 and 133821. And it seems like go inside deadloop by e->prev point to e itself. Is it somewhere overflow in the memory when try to walk through the linked list of events? I am a newbie in C/C++, I don't know how to debug this issue, but I make a trick in the code, I make a condition in while(), it works for now, but I don't if it would cause another bug. I change the _lv_event_mark_deleted() to: ``` void _lv_event_mark_deleted(lv_obj_t * obj) { lv_event_t * e = event_head; while(e) { if ( e -> code >= LV_EVENT_ALL && e -> code <= LV_EVENT_PREPROCESS ){ if(e->current_target == obj || e->target == obj) e->deleted = 1; e = e->prev; } else { e = NULL; //Exit the while loop when got the event code not in lv_event_code_t enum } } } ``` If someone can give me some solution for this issue? Thank you very much in advance. ### How to reproduce? ``` static lv_obj_t *main_page; static lv_obj_t *status_page; static void exit_status_btn_clicked_event_cb( lv_event_t *e ){ if( lv_event_get_code(e) != LV_EVENT_CLICKED ){ return; } lv_obj_clear_flag( main_page, LV_OBJ_FLAG_HIDDEN ); // Show the main_page if( status_page != NULL && lv_obj_is_valid( status_page ) ){ lv_obj_del(status_page); // Crash here status_page = NULL; } } static void load_status_btn_clicked_event_cb( lv_event_t *e ){ if( lv_event_get_code(e) != LV_EVENT_CLICKED ){ return; } lv_obj_add_flag( main_page, LV_OBJ_FLAG_HIDDEN ); // Not delete the main_page create_status_page(); } void create_status_page(){ status_page = lv_obj_create(lv_scr_act()); /**Codes for create other widgets*/ lv_obj_t *exit_btn = lv_btn_create( status_page ); lv_obj_add_event_cb(exit_btn, exit_status_btn_clicked_event_cb, LV_EVENT_CLICKED, NULL ); } void create_main_page(){ main_page = lv_obj_create(lv_scr_act()); /**Codes for create other widgets*/ lv_obj_t *status_btn = lv_btn_create( main_page ); lv_obj_add_event_cb(status_btn, load_status_btn_clicked_event_cb, LV_EVENT_CLICKED, NULL ); } ```

lv_obj_is_valid is not safe to use this way. It can happen that a widgget wwas freed and on it’s exact memory address an other
widget was allocateed.
lv_obj_is_valid will say that it’s valid, but it’s still not the widget thit you wanted.

and Alternative solutions for the implementation of lv_obj_is_valid() · Issue #6395 · lvgl/lvgl · GitHub

this API is very easy lead to crash, so I manually deleted this, like:

What have you tried so far?

Screenshot or video

Others

SquareLine Studio version:
Operating system:
Target hardware:

Hermit · August 1, 2024, 10:53pm

We know about the post you linked, the current limitation of LVGL about checking validity of objects, and the possibility that a new object very rarely (according to LVGL authors) takes the memory-location of the old one. That’s why we use NULL-checking beside lv_obj_is_valid(). By removing lv_obj_is_valid() you just lose an extra check I think. But thanks for pointing it out that there’s a new pointer auto-nullifying function just added to LVGL which is to circumvent this object-validity check discrepancy in the future.

kanbaiStaok · August 2, 2024, 1:22am

Discussion on the replacement of this API

github.com/lvgl/lvgl

Alternative solutions for the implementation of lv_obj_is_valid()

opened 05:40PM - 19 Jun 24 UTC

closed 01:52AM - 24 Jul 24 UTC

bhspyder

stale

### Introduce the problem The function lv_obj_is_valid() as it is currently i…mplemented searches every object (and it's chidren recursively) in every screen on every display looking for a match. This does not scale well on big projects, especially if all of the screens and objects are created on bootup and exist for lifetime of the application. I ran into this performance bottleneck while using the theme support provided by SquareLine studio generated code (I understand SquareLine is not affiliated with LVGL any more so this is more of an aside) which uses lv_obj_is_valid() to check for valid objects in it's theme style linked list. Are there any alternative solutions that can be considered to improve the performance of this function? ### Proposal One idea that came to mind (probably a bad idea) would be to NULL obj->class_p in the lv_obj_destructor() and changing lv_obj_is_valid() to something like this: ``` bool lv_obj_is_valid(const lv_obj_t * obj) { if (obj == NULL) { return false; } return lv_obj_has_class(obj, &lv_obj_class); } ``` Things to consider: * Accessing dangling pointers is a bad idea * If the object was deleted and the freed block of memory was unused, it's class_p would be NULL and be considered invalid. * If the object was deleted and the block of feed memory was reused by another lv_obj_t starting at the same offset, it would have a valid class_p and be considered valid. * If the object was deleted and the block of freed memory was reused by something else or an lv_obj_t with a different offset, the odds of the data lining up to produce a valid class_p that is or inherits from lv_obj_class are low, but not impossible? * This would only indicate that the obj has not been deleted, it would not indicate that the obj was a screen or a child of a screen. Since following base_class pointers to check for lv_obj_class inheritance on potentially freed obj pointers is not a good idea, comparing obj->class_p to a list of valid class pointers instead of using lv_obj_has_class() may be better, but still not good. This solution was only provided to get a discussion going. Please suggest alternative solutions.

Hermit · November 16, 2024, 10:31pm

Find my latest thoughts about this here: