Fix `BadMatch (invalid parameter attributes)` crash

[emanuele-f]

Addresses #431 . Please note that this is only a workaround to prevent the crash by ignoring the error. Hope it is useful to troubleshoot/work around the issue.

The issue seems linked to a focus event being triggered after the gtk_widget_hide call on the now closed (not viewable) window as explained in https://tronche.com/gui/x/xlib/input/XSetInputFocus.html .

[ensky0]

I don't know this code well, but my problem is solved. Thank you very much. I hope this PR merges quickly.

[ranjan-purbey]

Here are my findings (if it helps):

1. The crash occurs only on pull up if tilda already has focus and "above" boolean is true
2. The crash doesn't occur if "above" boolean is false or if tilda doesn't have focus when pulling up.
3. Adding a dealy of few seconds before returning from tomboy filter_func (here) prevents the crash even if 1 is true

[lanoxx]

Thanks for this pull request. If we cannot resolve this properly I am willing to merge this. But I'd like to ask a few questions first. Maybe we can find the cause of this problem together. Unfortunately I am not able to reproduce this at the moment so I guess this might only be a problem on certain window managers. I am using metacity, can you tell which window manager you are using?

According to the XSetInputFocus documentation this call should generate a FocusOut event and indeed were are seeing that in the log:

(tilda:80147): tilda-DEBUG: 22:24:31.764: pull_up(): MOVED UP (tilda:80147): tilda-DEBUG: 22:24:31.765: tilda_window.c: FUNCTION ENTERED: focus_out_event_cb

If I remove the call to gtk_widget_hide in key_grabber.c, then the FocusOut event no longer occurs. You could try to comment out that line and check if that resolve the problem. If so then the question would be why hiding the window causes a BadMatch event, maybe your window manager does not like hiding a window thats already outside the visible screen area.

[lanoxx]

The only two calls to XSetInputFocus in GTK+ are guarded by error traps, so I am if we are seeing the BadMatch error as a result of GTK calling XSetInputFocus then the error should actually be catched and ignored by GTK.

https://gitlab.gnome.org/search?utf8=%E2%9C%93&search=XSetInputFocus&group_id=8&project_id=665&scope=&search_code=true&snippets=false&repository_ref=master&nav_source=navbar

One more question, are you seeing this problem only with animation active, or also when animation is disabled?

[ranjan-purbey]

@lanoxx I am facing this on MATE 1.24.1 and I think that is the case for most people facing this issue. It worked perfectly on 1.22 but with Ubuntu 20.10 release, MATE was upgraded to 1.24. Window manager is Marco

[emanuele-f]

I am using metacity, can you tell which window manager you are using?

Using Marco with MATE 1.24.1 (archlinux)

One more question, are you seeing this problem only with animation active, or also when animation is disabled?

Please note that I have the animation disabled. It occurs even when the animation is enabled.

If I remove the call to gtk_widget_hide in key_grabber.c, then the FocusOut event no longer occurs. You could try to comment out that line and check if that resolve the problem.

Commenting it fixes the crash. However, when animation is disabled, the tilda window is not pulled up anymore.

[lanoxx]

I was able to setup a Mate environment where I could reproduce this error. After some debugging I think I now understand the cause of this issue. If I am not mistaken, then the cause of this issue is because we register our own X11 error handler and consequently the solution would be to remove our error handler completely.

Let me explain this in a bit more detail. When we call gtk_widget_hide on our main window, then the window will lose focus and the window manager will try to focus something else, so it sends a WM_TAKE_FOCUS event to the application which is handled by GTK which in turn calls XSetInputFocus, which is wrapped in error traps. This means that Gtk normally ignores any BadMatch errors that occur as a result of calling XSetInputFocus.

However, since we register our own error handler, the BadMatch errors still reaches our own handler. This means that ignoring the BadMatch error in our own handler is actually a valid solution. However, an even better solution is probably to remove the error handler all together.

The error handler is only registered to catch and ignore BadWindow errors. So if we remove the handler we need to wrap a couple of X functions into error traps. A result of removing the custom error handler in tilda is that we can also remove the 150ms time filter from the key_grabber, which we only added to avoid BadMatch errors.

So to summarize, I will merge this and then backport it to the 1.5.2 branch. I think this small enough for an SRU update into Ubuntu 20.04 (maybe also 20.10). But I will prepare a subsequent commit that removes the error handler completely and adds error traps around a few relevant X calls. Then I will also remove the 150ms delay in the key_grabber.

[ranjan-purbey]

Thanks for the detailed explanation 🙏