Bug 87824 - Dangling pointers and memory corruption after output recreate.
Summary: Dangling pointers and memory corruption after output recreate.
Status: RESOLVED MOVED
Alias: None
Product: Wayland
Classification: Unclassified
Component: weston (show other bugs)
Version: unspecified
Hardware: All Linux (All)
: medium normal
Assignee: Wayland bug list
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-29 10:33 UTC by Andriy Prystupa
Modified: 2018-06-08 23:53 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Andriy Prystupa 2014-12-29 10:33:49 UTC
Way to reproduce:
- weston backend closes current output and creates new one.
- old weston_output structure is released. 
- compositor views have pointers to old weston_output.
- these pointers became dangling.

Fix:
in weston_compositor_remove_output(...) add resetting reference to output:
wl_list_for_each(view, &compositor->view_list, link) {
      if (view->output == remove_output) {
           view->output = NULL;
           view->surface->output = NULL;
      }
}
Comment 1 Pekka Paalanen 2015-01-02 11:18:45 UTC
I think the fix is more complicated than that. You cannot set the outputs to NULL, because we use the pointers to denote that a surface is mapped (it's ugly, yes, pending for a rewrite). Windows should usually be moved, not unmapped.

When an output is disconnected, all weston_surfaces on that output should be moved to remaining outputs, or destroyed or unmapped. This is the responsibility of the shell plugin. I think that if a shell plugin does it right, there won't be any dangling weston_output pointers in any weston_view or weston_surface.

Still, a safety measure similar to what you suggest would be nice, along with some weston_log()s to yell about the shell plugin not doing its job. However, that needs to run only after the shell has had a chance to reorganize all surfaces.

I'm not sure if you have a different use case in mind, though: removing the very last output, leaving none? If that is so, we'd need to fix the "is mapped?" problem first, then do this, and then also fix the dozens other places that assume that there is at least one output at all times. This would be nice, too.

So, is this bug about some shell plugin not doing its job, needing the safety measure, or removing the last output leaving no outputs at all?
Comment 2 Andriy Prystupa 2015-01-05 16:49:39 UTC
>> So, is this bug about some shell plugin not doing its job, needing the safety >> measure, or removing the last output leaving no outputs at all?

My case is leaving no outputs at all for a while. 
Exact scenario:
- got event from HW
- in event handler destroy output and immediately create new one.

I am not sure if shell plugin will get control between destroy and create.

And situation gets worth because of undetermined behavior of heap manager:
when we do free(output) and then immediately alloc() pointer value lefts the same(at least in my case). Its hard to prognose possible bugs. To my mind it might be more stable if weston compositor will throw off incorrect pointers and unmap view(or mark it as to be remmaped) on destroy output.
Comment 3 GitLab Migration User 2018-06-08 23:53:41 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/wayland/weston/issues/62.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.