Add a no-op LVGL guard hook to DisplayApp::Refresh (companion to InfiniSim thread-race fix)#2447
Open
faisal-shah wants to merge 2 commits into
Open
Add a no-op LVGL guard hook to DisplayApp::Refresh (companion to InfiniSim thread-race fix)#2447faisal-shah wants to merge 2 commits into
faisal-shah wants to merge 2 commits into
Conversation
InfiniSim drives LVGL from two threads (its main thread renders the screen-off overlay and pumps lv_task_handler while the display task sleeps). AddressSanitizer showed the wake/sleep handoff causing heap-use-after-free: main-thread lv_obj_del of the overlay racing this task's lv_task_handler, crashing within ~4 wake/sleep cycles. LVGL_GUARD() is a no-op on hardware (displayapp/LvglGuard.h); the sim shadows the header with a real recursive mutex. The guard covers the two LVGL regions of Refresh() but deliberately not the blocking queue wait. Verified: 300 wake/sleep stress cycles under ASan, zero reports. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EeA4vFDz8WACLogQhg53Kd
|
Build size and comparison to main:
|
The switch inside DisplayApp::Refresh is now one scope deeper (wrapped in the LVGL_GUARD block), so clang-format re-indents its body. No behavior change; view with 'git diff -w' to see the semantic change is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Companion to InfiniTimeOrg/InfiniSim#198.
What this does
Adds
src/displayapp/LvglGuard.hwith a single macro,LVGL_GUARD(), and calls it around the two LVGL regions ofDisplayApp::Refresh(the state switch that runslv_task_handler, and the message-handling block). It deliberately does not wrap the blocking queue wait between them.On hardware the macro is a no-op:
So this changes nothing on the watch. LVGL there has a single owner, the display task, and the compiler drops the macro entirely.
Why
The value is in the simulator. InfiniSim drives LVGL from two threads: the DisplayApp task, and its SDL main thread, which renders the "Screen is OFF" overlay and runs
lv_task_handlerwhile the display task sleeps. The wake/sleep handoff between them corrupts the LVGL heap. InfiniSim shadows this header (via include-path order, the same mechanism it already uses fornrf_log.h) with a version that definesLVGL_GUARD()as a real mutex, so both threads serialize. That side of the fix, the crash reproduction, and the AddressSanitizer trace are in InfiniTimeOrg/InfiniSim#198.The guard has to live in InfiniTime because the DisplayApp task is InfiniTime code; InfiniSim cannot serialize against it from the outside. This PR is the minimal hook that lets it.
Scope
Two files, +24/-8. No behavior change on hardware. The mutex, the try-lock on the simulator's main thread, and the verification all live in the InfiniSim PR.