Bug 77072

Summary: Weston crashes after using context menus in weston-terminal
Product: Wayland Reporter: Bryan Cain <bryancain3+fdo>
Component: westonAssignee: Wayland bug list <wayland-bugs>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium    
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: Proposed patch

Description Bryan Cain 2014-04-04 22:36:15 UTC 
Running Weston complied from the latest git head (47928d871503) as a windowed X11 client, I consistently get a SIGSEGV in Weston when I do the following:

1) open weston-terminal
2) select some text, right-click in weston-terminal, and select "Copy" or "Paste" from the context menu
3) quickly click on an X11 window outside of the Weston compositor, making the compositor lose keyboard focus

I did some digging, and found out what was happening in the Weston shell.  When an option in the context menu is selected, weston-terminal (via toytoolkit) calls xdg_popup_destroy() to close the context menu.  The Weston shell implements this with the shell_destroy_shell_surface() function, which sets the backing shsurf->resource to NULL.  However, the surface stays in shseat->popup_grab.surfaces_list and isn't actually destroyed until its closing fade animation is finished.  If the Weston compositor loses the keyboard focus before the fade animation is finished, it tries to forcefully end the popup's mouse grab by calling shell_surface_send_popup_done() on the popup whose destructor has already been called, which causes a segfault.

I've made a small patch to fix the bug by removing the surface from the popup_grab list in shell_destroy_shell_surface.
Comment 1 Bryan Cain 2014-04-04 22:38:50 UTC 
Created attachment 96921 [details] [review]
Proposed patch
Comment 2 Kristian Høgsberg 2014-04-07 05:48:08 UTC 
Pushed to master, thanks.

commit a46b946cb3842a6e6296ed7c4c7b0303096a2b00
Author: Bryan Cain <bryancain3@gmail.com>
Date:   Fri Apr 4 17:41:24 2014 -0500

    shell: Fix segfault from trying to access a destroyed popup shell surface
    
    The shell_destroy_shell_surface function only set the backing resource to
    NULL, leaving an unusable surface in the popup_grab list until the surface's
    fading animation finished and it could be freed. This caused a segfault if
    the shell tried to forcibly break the grab during that time interval due to
    the compositor losing the keyboard focus.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=77072

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.