search

evilsocket / opensnitch

OpenSnitch is a GNU/Linux interactive application firewall inspired by Little Snitch.
GNU General Public License v3.0
10.92k stars 510 forks source link

GUI crashes due to Wayland #526

Closed ryanolton closed 3 years ago

ryanolton commented 3 years ago

Describe the bug

This issue only happens primarily after a dialog opens and I make a selection and the dialog closes, but always after some time has passed (an hour or two) without any dialogs. If I leave for the day and come back the next morning, the GUI will have crashed overnight. The icon will disappear from Gnome-shell and no more alerts appear.

This appears to be a Wayland-specific issue, and was not present in version v1.4.0-rc.3 and earlier.

Include the following information:

To Reproduce

I have only seen this occur while under Wayland, in both GNOME Shell but also KDE Plasma (both using Wayland with Fedora 34).

Steps to reproduce the behavior:

  1. Boot into a Wayland session
  2. Have the Opensnitch GUI running in GNOME Shell
  3. Wait for a dialog to appear, or simply wait a few hours

Post error logs: 

Loading translations: /usr/lib/python3/dist-packages/opensnitch/i18n locale: en_US
QSocketNotifier: Can only be used with threads started with QThread
new node connected, listening for client responses... /tmp/osui.sock
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
The Wayland connection experienced a fatal error: Protocol error

This appears and the GUI closes

Again, this only became an issues starting with version v1.4.0-rc.4, and worked fine on v1.4.0-rc.3 and earlier. If I install v1.4.0-rc.3 (both GUI and daemon), it works without issue.

gustavo-iniguez-goya commented 3 years ago

Thank you @ryanolton for reporting this,

only became an issues starting with version v1.4.0-rc.4, and worked fine on v1.4.0-rc.3 and earlier

hmm, I'll take a look at those changes.

gustavo-iniguez-goya commented 3 years ago

@ryanolton could you modify this line in the file /usr/lib/python3/dist-packages/opensnitch/dialogs/stats.py: https://github.com/evilsocket/opensnitch/blob/fd40feb78ca2de49e115c77cb7a185df4deae888/ui/opensnitch/dialogs/stats.py#L550

by: self.rulesSplitter.setSizes([int(w/4), int(w/2)])

after this change the GUI does no longer crash on my f34.

ryanolton commented 3 years ago

@gustavo-iniguez-goya I have the modified code in place, running via command line now watching for the error messages originally noted. I'll reply back if the issue appears again, or if enough time passes without any issues.

Thanks!

ryanolton commented 3 years ago

@gustavo-iniguez-goya it seemed to be running well, so I deleted some rules I new would cause a dialog to appear, and after dismissing each dialog, I am seeing the same command line error as before:

qt.qpa.wayland: Wayland does not support QWindow::requestActivate()

This seems directly connected to the prompt dialog being dismissed, is that what you were targeting with your code fix?

I also just noticed another error if I open the stats window, delete a rule (such as Firefox port 443), then switch over to Firefox. This causes the stats window to lose focus which causes the GUI to crash with the following error:

qt.qpa.wayland: setGrabPopup called with a parent, QtWaylandClient::QWaylandXdgSurface(0x55aa24e7be20) which does not match the current topmost grabbing popup, QtWaylandClient::QWaylandXdgSurface(0x55aa24e78da0) According to the xdg-shell protocol, this is not allowed. The wayland QPA plugin is currently handling it by setting the parent to the topmost grabbing popup. Note, however, that this may cause positioning errors and popups closing unxpectedly because xdg-shell mandate that child popups close before parents

I'm not sure this is an Opensnitch issues, seems like a Wayland issue?

Is there something I can do to try and help?

gustavo-iniguez-goya commented 3 years ago

qt.qpa.wayland: Wayland does not support QWindow::requestActivate()

yes, I see that error as well. I guess that all these messages are just warnings. Does it still crash or now is running fine despite the errors logged to the terminal?

is that what you were targeting with your code fix?

nope, it was a different thing. Just after launching the GUI on f34, it crashed on that line.

ryanolton commented 3 years ago

Does it still crash or now is running fine despite the errors logged to the terminal?

It still crashes, unfortunately. I agree that the Wayland does not support QWindow::requestActivate() message is probably a warning only, and sometimes I see one of these warnings before it crashes, sometimes I see three before it crashes.

I'm going to monitor the command line output and see if there is anything else that could point to the issue.

ryanolton commented 3 years ago

@gustavo-iniguez-goya I've been monitoring this for a bit now, and on GNOME it will crash reliably with the previously mentioned errors, however KDE does not seem to crash, so I suspect there is simply something incorrect with versions of Qt (or Wayland + Qt) within GNOME.

gustavo-iniguez-goya commented 3 years ago

Thank you @ryanolton , at least it points me in the right direction.

Danny3 commented 3 years ago

I could never see it crash on Wayland with KDE Plasma on Kubuntu 21.04 or 21.10

ryanolton commented 3 years ago

@gustavo-iniguez-goya I decided to debug Wayland a bit and I see this right around the time that the prompt window is shown.

> WAYLAND_DEBUG=1 opensnitch-ui

Then I wait for a prompt to appear:

qt.qpa.wayland: Wayland does not support QWindow::requestActivate()
[1472369.983]  -> wl_compositor@4.create_surface(new id wl_surface@45)
[1472370.003]  -> xdg_wm_base@22.get_xdg_surface(new id xdg_surface@41, wl_surface@45)
[1472370.017]  -> xdg_surface@41.get_toplevel(new id xdg_toplevel@35)
[1472370.025]  -> xdg_toplevel@35.set_title("OpenSnitch v1.4.1")
[1472370.035]  -> xdg_toplevel@35.set_app_id("python3")
[1472370.041]  -> wl_surface@45.set_buffer_scale(2)
[1472370.049]  -> wl_surface@45.set_buffer_transform(0)
[1472370.054]  -> wl_surface@45.commit()
[1472370.075]  -> wl_compositor@4.create_region(new id wl_region@23)
[1472370.082]  -> wl_region@23.add(1021, 593, 520, 317)
[1472370.093]  -> wl_surface@45.set_opaque_region(wl_region@23)
[1472370.099]  -> wl_region@23.destroy()
[1472374.128] xdg_toplevel@35.configure(0, 0, array)
[1472374.146] xdg_surface@41.configure(1104)
[1472374.162]  -> xdg_surface@41.set_window_geometry(0, 0, 522, 356)
[1472374.177]  -> xdg_toplevel@35.set_min_size(420, 317)
[1472374.188]  -> xdg_toplevel@35.set_max_size(520, 308)
[1472374.196]  -> xdg_surface@41.ack_configure(1104)
[1472374.406]  -> wl_shm_pool@19.destroy()
[1472374.421]  -> wl_buffer@40.destroy()
[1472374.547]  -> wl_shm_pool@33.destroy()
[1472374.558]  -> wl_buffer@44.destroy()
[1472374.604]  -> wl_shm@5.create_pool(new id wl_shm_pool@49, fd 20, 2973312)
[1472374.637]  -> wl_shm_pool@49.create_buffer(new id wl_buffer@48, 0, 1044, 712, 4176, 0)
[1472381.490]  -> wl_surface@45.damage(0, 0, 522, 38)
[1472381.506]  -> wl_surface@45.damage(0, 38, 1, 317)
[1472381.515]  -> wl_surface@45.damage(521, 38, 1, 317)
[1472381.523]  -> wl_surface@45.damage(0, 355, 522, 1)
[1472381.531]  -> wl_surface@45.damage(0, 356, 1, 38)
[1472381.668]  -> wl_surface@45.frame(new id wl_callback@39)
[1472381.678]  -> wl_surface@45.attach(wl_buffer@48, 0, 0)
[1472381.683]  -> wl_surface@45.damage(1, 38, 520, 317)
[1472381.692]  -> wl_surface@45.commit()
[1472381.804]  -> wl_shm@5.create_pool(new id wl_shm_pool@46, fd 20, 2973312)
[1472381.813]  -> wl_shm_pool@46.create_buffer(new id wl_buffer@47, 0, 1044, 712, 4176, 0)
[1472383.408]  -> wl_surface@45.frame(new id wl_callback@54)
[1472383.422]  -> wl_surface@45.attach(wl_buffer@47, 0, 0)
[1472383.427]  -> wl_surface@45.damage(72, 43, 444, 80)
[1472383.436]  -> wl_surface@45.damage(6, 130, 510, 17)
[1472383.442]  -> wl_surface@45.commit()
[1472384.257] wl_display@1.delete_id(23)
[1472384.265] wl_display@1.delete_id(19)
[1472384.271] wl_display@1.delete_id(40)
[1472384.274] wl_display@1.delete_id(33)
[1472384.278] wl_display@1.delete_id(44)
[1472384.282] wl_display@1.error(wl_surface@45, 4, "Invalid min/max size")
The Wayland connection experienced a fatal error: Protocol error

Any idea what "Invalid min/max size" might be referring to?

gustavo-iniguez-goya commented 3 years ago

no idea, sorry :/

There're several places where the dialogs are resized: https://github.com/evilsocket/opensnitch/blob/c609b09f1d5d6d45c6070e256a14e58b5d9850e5/ui/opensnitch/dialogs/stats.py#L524

https://github.com/evilsocket/opensnitch/blob/c609b09f1d5d6d45c6070e256a14e58b5d9850e5/ui/opensnitch/dialogs/prompt.py#L110

https://github.com/evilsocket/opensnitch/blob/c609b09f1d5d6d45c6070e256a14e58b5d9850e5/ui/opensnitch/dialogs/prompt.py#L351

https://github.com/evilsocket/opensnitch/blob/c609b09f1d5d6d45c6070e256a14e58b5d9850e5/ui/opensnitch/dialogs/stats.py#L550

I'd start by placing a print() before and after each of these lines, at least to know where it's crashing. Anyway, I'll leave a Fedora 35 running with wayland for some days, to see if it crashes.

ryanolton commented 3 years ago

I found something relating to the error here: https://gitlab.gnome.org/GNOME/mutter/blob/d714a94d97af8b34721236bb8ca6f0e3fd6dc1c9/src/wayland/meta-wayland-legacy-xdg-shell.c

Search this document for "Invalid min/max size", but it looks like perhaps the window geometry is bigger than than max size already set?

I see a change here - 83ab1f9f5ec0391d5afb979963940003902762a0 - that seemed to cause the problem.

I have tracked it down to this line: https://github.com/evilsocket/opensnitch/blob/c609b09f1d5d6d45c6070e256a14e58b5d9850e5/ui/opensnitch/dialogs/prompt.py#L110

When I comment out this line, the UI no longer crashes! So now, I'm curious what the intent is here with this line of code?

gustavo-iniguez-goya commented 3 years ago

When a command line is too big to fit in the widgets of the pop-ups, sometimes the pop-up dialog is enlarged. As the pop-up dialog is instantiated only once, the dialog size keeps enlarging over and over.

The idea was to restore the original size every time it was shown, by getting the original size when the dialog is instantiated: https://github.com/evilsocket/opensnitch/blob/c609b09f1d5d6d45c6070e256a14e58b5d9850e5/ui/opensnitch/dialogs/prompt.py#L62

ryanolton commented 3 years ago

After reviewing the Mutter code above, as well as some additional testing, it seems this is due to either:

If I add a line of code similar to this:

self.setMinimumSize(self._width, self._height)

... right before setMaximumSize then the I do not get a crash.

This code addition causes the elements to be oddly sized, so it's clear this is not a perfect solution. For now I have the setMaximumSize commented out, but ideally we could solve the unwanted enlarging in a way that works on all DE.

gustavo-iniguez-goya commented 3 years ago

I agree Ryan, thank you for debugging it. What Gnome-Shell and mutter do you have installed? I'm testing it on Fedora 35, Gnome-Shell 41.0 7.fc35 and Mutter 41.0 4.fc35 and it works just fine.

ryanolton commented 3 years ago

What Gnome-Shell and mutter do you have installed?

@gustavo-iniguez-goya I'm also running Fedora 35, same versions of GNOME Shell and Mutter that you noted.

gustavo-iniguez-goya commented 3 years ago

If I add a line of code similar to this: self.setMinimumSize(self._width, self._height) ... right before setMaximumSize then the I do not get a crash.

if this fixes the problem then we can add it as workaround. It works fine on my machine, I'll test it in some more.

ryanolton commented 3 years ago

If I add a line of code similar to this: self.setMinimumSize(self._width, self._height) ... right before setMaximumSize then the I do not get a crash.

if this fixes the problem then we can add it as workaround. It works fine on my machine, I'll test it in some more.

@gustavo-iniguez-goya I have created a PR (https://github.com/evilsocket/opensnitch/pull/540) that seems to work better on my local machine. Setting the width and height after initially showing the prompt instead of during init seemed to retain the correct sizing for me. Please free to review and make any changes you feel necessary.

gustavo-iniguez-goya commented 3 years ago

tested! it indeed fixes a little glitch that I was seeing after use setMinimumSize()

Thank you @ryanolton ! :)