[Wayland-bugs] [Bug 80273] SIGSEGV in weston_wm_window_destroy

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=80273

--- Comment #1 from Tyler Veness <calcmogul at gmail.com> ---
I encountered this bug on a machine of mine last night. I reproduce it with the
following:

1. Start weston with weston-launch
2. Open weston-terminal
3. Run evince from weston-terminal
4. Click on the close button on evince's window

My backtrace is essentially the same as the one already attached. At
wayland-util.c:54, there is the line "elm->prev->next = elm->next;" in which
both elm->prev and elm->next are NULL. Since wl_list_remove sets them to that
after removing them, wl_list_remove may have been called twice, the SIGSEGV
being from the second time, or wl_list_insert was never called at all and the
pointers are NULL because Linux inits memory to 0 when it's malloc'ed.

There are two instances of wl_list_remove in xwayland/window-manager.c at lines
685 and 1133 respectively. I set a breakpoint at the first and it was never
called. The second is where the SIGSEGV occurs. I also set a breakpoint at line
1384 to figure out if wl_list_insert was called in the first place, which
wasn't the case; the first branch with "xserver_map_shell_surface" was called
instead. I think the check for window->surface_id at line 1132 assumed
surface_id would be set to 0 at line 684.

There are three ways I see to avoid wl_list_remove causing a SIGSEGV at line
1133. I have patches for the first two which I know fix the SIGSEGV.

Possible Fixes:

1. Add "window->surface_id = 0;" to the if branch of the if statement in
Weston's source at xwayland/window-manager.c:1380. However, I don't know what
this affects elsewhere in the code.

2. Add NULL pointer checks to wl_list_remove in Wayland's source at
wayland-util.c:52. This would work, but it sort of acts like a band-aid for the
real problem, which is in Weston. One could argue that wayland shouldn't crash
due to the negligence of the display compositor, but the C++ STL is like
wayland in that it will crash or leak memory if improperly managed.

3. Add a way to keep track of which branch the execution path took in Weston's
source at xwayland/window-manager.c:1380. This would essentially provide a way
to determine whether the window is in the unpaired_window_list or not. I can't
think of a clean way to do it though (i.e. without a global variable).

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-bugs/attachments/20140628/255c2f32/attachment.html>