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

ShadowEffect class causes hangs/crashes on Wayland #970

Closed
davidmhewitt opened this issue Nov 18, 2020 · 6 comments · Fixed by #1689
Closed

ShadowEffect class causes hangs/crashes on Wayland #970

davidmhewitt opened this issue Nov 18, 2020 · 6 comments · Fixed by #1689

Comments

@davidmhewitt
Copy link
Member

davidmhewitt commented Nov 18, 2020

Opening this for tracking against the Wayland project board.

Because the ShadowEffect class uses GTK CSS, it tries to initialise GTK upon construction.

It is not possible to initialise GTK inside the Wayland compositor, so we need to go back to using Cogl/Clutter to draw shadows.

@JoseExposito
Copy link
Member

JoseExposito commented Jan 8, 2021

I've being playing with the Wayland session a little bit lately.
I also thought that the issue was using GTK on startup, but now I'm not sure anymore. I created a quick patch to load shadows before entering the multitasking view and it keeps crashing (find the patch attached).

Can't create a GtkStyleContext without a display connection

You can easily test it running Gala as a nested compositor, in my case it only crashes when the multitasking view is opened.

After a really quick look to GNOME Shell, it seem like they only use Gtk.StyleContext in the extension code, so maybe it is not possible to use it in the compositor. What do you think?

They are using https://gitlab.gnome.org/GNOME/gnome-shell/-/blob/master/src/st/st-shadow.h

wayland.patch.txt

@davidmhewitt
Copy link
Member Author

Does anything change or do you get any interesting messages if you do a Gtk.init (null) before initialising the Gtk.StyleContext with your changes?

St is basically a re-implementation of large parts of GTK to render GTK-like stuff in the compositor, and is not really a route we want to pursue. It's true that GTK itself shouldn't really work in the compositor, but you'll notice that Gala doesn't really use GTK anywhere other than for these shadows. And even then it's not really GTK, we're just using GTK APIs to effectively query the GTK stylesheet so we don't have to re-write the shadow code in Gala every time the shadow style changes in the stylesheet.

@davidmhewitt
Copy link
Member Author

Another interesting thing to test would be to see if the environment variable WAYLAND_DISPLAY is set within gala at the point where the style context is constructed.

@JoseExposito
Copy link
Member

JoseExposito commented Jan 9, 2021

Does anything change or do you get any interesting messages if you do a Gtk.init (null) before initialising the Gtk.StyleContext with your changes?

Yes, if I call init_check, to avoid initializing GTK more than once:

unowned string[] args = null;
Gtk.init_check (ref args);

My entire session freezes without showing any relevant message.

Another interesting thing to test would be to see if the environment variable WAYLAND_DISPLAY is set within gala at the point where the style context is constructed.

Mmmmm interestingly enough no, the variable is not set. Even though I can run some apps in the nested compositor:

$ GDK_BACKEND=wayland io.elementary.calculator

However, the socket in XDG_RUNTIME_DIR gets created. Apps fallback to wayland-0 if this variable is not set, which is the name of the socket, so I don't think this is the problem.


By the way, this is how I run it, so you, or anyone interested, can test it without breaking your stable Gala installation:

$ meson build --prefix=`pwd`/build/install --buildtype=debug
$ ninja -C build && \
  ninja -C build install && \
  env MUTTER_DEBUG_DUMMY_MODE_SPECS=1600x900 G_MESSAGES_DEBUG=all ./build/install/bin/gala --nested --wayland

@davidmhewitt
Copy link
Member Author

@JoseExposito Thanks, that's really helpful. You're right indeed that initializing the StyleContext (or GTK) anywhere causes a hang.

I've debugged this a little further with the help of the WAYLAND_DEBUG=1 and by doing:

string[] args = {"gala", "--gdk-debug=all"};
unowned string[] args2 = args;
Gtk.init (ref args2);

It seems that GDK is getting stuck somewhere around here while initialising the wayland display:
https://github.com/GNOME/gtk/blob/de8329b3ce4b235eac9ddcbb6cd121cc83dd9e74/gdk/wayland/gdkdisplay-wayland.c#L629-L640

At first I thought there might be some kind of deadlock where the act of the Wayland client (ShadowEffect class using GTK) waiting for the Wayland server to respond was locking the Wayland server, but spawning Gtk.init in a different thread (which is usually a terrible idea) made no difference.

I think it's probably time to look at removing GTK code from Gala.

@JoseExposito
Copy link
Member

I think that, unfortunately, that's the only solution @davidmhewitt

I need to investigate where are those shadows used in addition to the multitasking view.
I'm sure some windows are not using them, but I need to check both client and server side decorations.

so we don't have to re-write the shadow code in Gala every time the shadow style changes in the stylesheet

We could try to do something smart and parse the CSS, but I don't know how is that going to end up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
2 participants