GNOME Bugzilla – Bug 789141
Segmentation fault when adding Google online account
Last modified: 2017-10-26 10:23:59 UTC
Created attachment 361795 [details] stacktrace Downstream bug: https://bugs.launchpad.net/ubuntu/+source/gnome-control-center/+bug/1720400 Steps to reproduce: 1) login in wayland session 2) open gnome control center 3) go to "Online Accounts" panel 4) click on Google account 5) close the dialog window 6) click anywhere on the list g-c-c should crash.
Created attachment 361797 [details] [review] Clears the current GdkGLContext if the GdkWindow used by the context was unmapped
Review of attachment 361797 [details] [review]: Thanks for the patch ::: gdk/wayland/gdkwindow-wayland.c @@ +2530,3 @@ } + current_context = gdk_gl_context_get_current(); Coding style: missing space between function and parenthesis. @@ +2531,3 @@ + current_context = gdk_gl_context_get_current(); + if (current_context && gdk_gl_context_get_window(current_context) == window) Coding style: - pointer comparisons should be explicit against NULL - missing space between function name and parenthesis
Backtrace doesn't look specific to the Online Accounts panel; even more so after looking at the patch. Reassigning to gtk+.
Created attachment 361806 [details] [review] Clears the current GdkGLContext if the GdkWindow used by the context was unmapped
Ok I fixed the style and as suggested by Emanuele I made the logic generic and moved the code inside GdkWindow (gdk_window_withdraw).
Review of attachment 361806 [details] [review]: Looks sane to me
Review of attachment 361806 [details] [review]: Commit message nits: 1) Format should be something like "gdk: Action" thus probably something like "gdk: Clear GL context when window is withdrawn" 2) The commit message body should describe what this fixes. The change looks sane to me though.
Created attachment 361918 [details] [review] gdk: Clear GL context when window is withdrawn Some clients (e.g. gnome-online-accounts) quickly unmap and map a window. With some backends the backend surface will be replaced causing the application to crash because the GL context is still using the old surface. Clearing the GL context when a window is withdrawn fixes this.
Review of attachment 361918 [details] [review]: Not really good in wording here, so I'm ok with this :)
Review of attachment 361918 [details] [review]: ::: gdk/gdkwindow.c @@ +3926,1 @@ _gdk_make_event (window, GDK_UNMAP, NULL, FALSE); Fly-by comment: this line makes me think that this patch was spun against master while gnome-control-center is using gtk-3-22. Did you actually check whether this fixes the crash?
(In reply to Debarshi Ray from comment #10) > Review of attachment 361918 [details] [review] [review]: > > ::: gdk/gdkwindow.c > @@ +3926,1 @@ > _gdk_make_event (window, GDK_UNMAP, NULL, FALSE); > > Fly-by comment: this line makes me think that this patch was spun against > master while gnome-control-center is using gtk-3-22. Did you actually check > whether this fixes the crash? Indeed we tried.
(In reply to Andrea Azzarone from comment #11) > (In reply to Debarshi Ray from comment #10) > > Review of attachment 361918 [details] [review] [review] [review]: > > > > ::: gdk/gdkwindow.c > > @@ +3926,1 @@ > > _gdk_make_event (window, GDK_UNMAP, NULL, FALSE); > > > > Fly-by comment: this line makes me think that this patch was spun against > > master while gnome-control-center is using gtk-3-22. Did you actually check > > whether this fixes the crash? > > Indeed we tried. Ok, thanks. It seemed to me that the crash wasn't 100% reliable - it readily crashed on the first attempt, then it didn't, so I wasn't sure.