From defbd1d47812df462b39bdca6d046ea94974fd04 Mon Sep 17 00:00:00 2001 From: Kaj Schittecat Date: Sat, 3 Oct 2026 19:33:33 +0200 Subject: [PATCH] lua: wada.ui.clear() leaked a closure per button and per list row Reported by Jade as "screen memory errors, worse the longer the app has been open", with an app that rebuilds its screens on a 120 ms debounce. Button and list-row callbacks live in one registry table keyed by the LVGL object POINTER. clear() calls lv_obj_clean(), which frees every one of those objects, and bumps s_ui_gen so stale handles go inert, but it never touched the table. So each rebuild added an entry and kept the Lua closure, with its upvalues, alive forever. Unbounded, and worst for exactly the apps that are built properly: a list of twenty contacts redrawn on a timer leaks twenty closures a go. It was also a correctness bug in waiting. LVGL reuses freed addresses, so a new button created WITHOUT a callback could land on an address a dead one left behind and fire that stale closure on the next tap. clear() now replaces the table. Every entry in it describes an object lv_obj_clean just destroyed, so there is nothing there worth keeping. Found by reading the host after Jade asked what the memory budget was; the budget was never the problem. Co-Authored-By: Claude Opus 5 --- src/ui-touch/LuaAppHost.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/ui-touch/LuaAppHost.cpp b/src/ui-touch/LuaAppHost.cpp index 9aa651e..b3fd54a 100644 --- a/src/ui-touch/LuaAppHost.cpp +++ b/src/ui-touch/LuaAppHost.cpp @@ -729,13 +729,28 @@ int uiTextLines(lua_State* L) { // accessors compare generations and null it, so the existing `if (u->obj)` guard // in every method turns a stale call into a no-op instead of a crash. static int uiClear(lua_State* L) { - (void)L; if (!s_h || !s_h->body) return 0; #if CAP_LUA_SDK_EXT destroyLiveMapView(); #endif lv_obj_clean(s_h->body); // deletes the children, keeps the page itself ++s_ui_gen; + // Drop the button callbacks with the buttons they belong to. The table is + // keyed by the LVGL object POINTER, and lv_obj_clean just freed every one of + // them, so every entry here is now stale by definition. Leaving them cost an + // app a table entry and a live closure (with its upvalues) per button per + // rebuild, which is unbounded: an app with a few screens it redraws is the + // normal case, and Jade's hit "screen memory errors, worse the longer it has + // been open" exactly this way. + // + // It was also a correctness bug waiting to happen. LVGL reuses freed + // addresses, so a NEW button created without a callback could land on an + // address a dead one left behind and fire that stale closure on the next tap. + if (s_h->L && s_h->ref_btncb != LUA_NOREF) { + lua_newtable(s_h->L); + lua_rawseti(s_h->L, LUA_REGISTRYINDEX, s_h->ref_btncb); + } + (void)L; return 0; }