Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(snapshot): fix memleak in lv_snapshot #6147

Merged
merged 1 commit into from May 21, 2024

Conversation

terry0012
Copy link
Contributor

Description of the feature or fix

When we take a snapshot for a lv_obj, if there is a LV_LAYER_TYPE_TRANSFORM object in the children.
A new layer named new_layer will be malloced in refr_obj function.
But it will not be freed after lv_snapshot_take_to_buf done as we call lv_draw_dispatch_layer(NULL, &layer);

Because the new_layer is freed only when disp is not NULL as the below code.

                /*Remove the layer from  the display's*/
                if(disp) {
                    lv_layer_t * l2 = disp->layer_head;
                    while(l2) {
                        if(l2->next == layer_drawn) {
                            l2->next = layer_drawn->next;
                            break;
                        }
                        l2 = l2->next;
                    }

                    if(disp->layer_deinit) disp->layer_deinit(disp, layer_drawn);
                    lv_free(layer_drawn);
                }

Notes

@kisvegabor
Copy link
Member

Wonderful, thank you!

Could you add a test too? Having a screenshot for a transformed layer and an assert for the memory leak would be great.

@terry0012
Copy link
Contributor Author

Wonderful, thank you!

Could you add a test too? Having a screenshot for a transformed layer and an assert for the memory leak would be great.

OK.
I add a transform obj under active screen in function test_snapshot_should_not_leak_memory.
And test_snapshot can detect the memory leak without this patch.

By the way, I found test_snapshot will stuck in LVGL_CI_USING_SYS_HEAP mode if lv_obj_set_style_transform_rotation is added.
I will continue to check it further.

@kisvegabor
Copy link
Member

I've checked it an it seems it really stuck in lv_snapshot_take only when lv_obj_set_style_transform_rotation is enabled and only in the tests. Running it normally (not in tests) works well even with the very same lv_conf.h as in the tests.

@terry0012
Copy link
Contributor Author

I've checked it an it seems it really stuck in lv_snapshot_take only when lv_obj_set_style_transform_rotation is enabled and only in the tests. Running it normally (not in tests) works well even with the very same lv_conf.h as in the tests.

Yes. it's a dead lock problem.

@terry0012
Copy link
Contributor Author

terry0012 commented May 6, 2024

In the test case, we have three draw task:

  • draw rect for screen obj on snapshot layer (Task 1)
  • draw label for label obj on a transform layer (Task 2)
  • draw transform layer on snapshot layer (Task 3)

For Task 3, the init state is LV_DRAW_TASK_STATE_WAITING as it need to wait Task 2 done.
But when we call lv_draw_dispatch_layer in lv_snapshot_take_to_draw_buf, it only traverse the snapshot layer to change the task state.

At last, when lv_draw_dispatch_wait_for_request called, it'll call lv_draw_get_next_available_task to search a draw task, but Task 3 is not available because it is in LV_DRAW_TASK_STATE_WAITING state.

At this moment, layer.draw_task_head is not NULL , LVGL thread is waiting a signal, Render thread is also waiting a signal as u->task_act is NULL. Then, a dead lock happened.

Copy link
Member

@kisvegabor kisvegabor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the investigation! I see no issue with running a full dispatching here.

tests/src/test_cases/test_snapshot.c Show resolved Hide resolved
tests/src/test_cases/test_snapshot.c Show resolved Hide resolved
Copy link
Member

@kisvegabor kisvegabor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@FASTSHIFT FASTSHIFT merged commit 34d5f2b into lvgl:master May 21, 2024
19 checks passed
kisvegabor pushed a commit to kisvegabor/lvgl_upstream that referenced this pull request May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants