<div dir="ltr"><br><div class="gmail_quote"><div dir="ltr">On Tue, May 29, 2018 at 3:30 AM Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Thu, 10 May 2018 00:53:38 -0700<br>
Dima Ryazanov <<a href="mailto:dima@gmail.com" target="_blank">dima@gmail.com</a>> wrote:<br>
<br>
> In GNOME (but not in Weston), if a window loses focus, the client first receives<br>
> the focus event, then the unlock/unconfine event. This causes toytoolkit to<br>
> dereference a NULL window when unlocking or unconfining the pointer.<br>
> <br>
> To repro:<br>
> - Run weston-confine<br>
> - Click the window<br>
> - Alt-Tab away from it<br>
> <br>
> Result:<br>
> <br>
> [1606837.869] wl_keyboard@19.modifiers(63944, 524352, 0, 0, 0)<br>
> [1606837.926] wl_keyboard@19.leave(63945, wl_surface@15)<br>
> [1606837.945] wl_pointer@18.leave(63946, wl_surface@15)<br>
> [1606837.956] wl_pointer@18.frame()<br>
> [1606837.961] zwp_confined_pointer_v1@26.unconfined()<br>
> Segmentation fault (core dumped)<br>
> <br>
> To fix this, get the input from the window instead of the other way around.<br>
> <br>
> Signed-off-by: Dima Ryazanov <<a href="mailto:dima@gmail.com" target="_blank">dima@gmail.com</a>><br>
> ---<br>
> clients/window.c | 23 +++++++++++++----------<br>
> 1 file changed, 13 insertions(+), 10 deletions(-)<br>
> <br>
<br>
Hi Dima,<br>
<br>
thank you for the patch, and sorry, I know you have some very old<br>
patches still waiting for reviews.<br></blockquote><div><br></div><div>Haha, no worries!<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> diff --git a/clients/window.c b/clients/window.c<br>
> index bcf2b017..dee4455f 100644<br>
> --- a/clients/window.c<br>
> +++ b/clients/window.c<br>
> @@ -286,6 +286,7 @@ struct window {<br>
> confined_pointer_unconfined_handler_t pointer_unconfined_handler;<br>
> <br>
> struct zwp_confined_pointer_v1 *confined_pointer;<br>
> + struct input *confined_input;<br>
> struct widget *confined_widget;<br>
> bool confined;<br>
> <br>
> @@ -4788,8 +4789,8 @@ static void<br>
> locked_pointer_locked(void *data,<br>
> struct zwp_locked_pointer_v1 *locked_pointer)<br>
> {<br>
> - struct input *input = data;<br>
> - struct window *window = input->pointer_focus;<br>
> + struct window *window = data;<br>
> + struct input *input = window->locked_input;<br>
> <br>
> window->pointer_locked = true;<br>
> <br>
> @@ -4804,8 +4805,8 @@ static void<br>
> locked_pointer_unlocked(void *data,<br>
> struct zwp_locked_pointer_v1 *locked_pointer)<br>
> {<br>
> - struct input *input = data;<br>
> - struct window *window = input->pointer_focus;<br>
> + struct window *window = data;<br>
> + struct input *input = window->locked_input;<br>
> <br>
> window_unlock_pointer(window);<br>
> <br>
> @@ -4860,7 +4861,7 @@ window_lock_pointer(struct window *window, struct input *input)<br>
> ZWP_POINTER_CONSTRAINTS_V1_LIFETIME_ONESHOT);<br>
> zwp_locked_pointer_v1_add_listener(locked_pointer,<br>
> &locked_pointer_listener,<br>
> - input);<br>
> + window);<br>
<br>
Now that this object will have a pointer to window, how do we safeguard<br>
against window_destroy() getting called before we dispatch a locked or<br>
unlocked event? Wouldn't that be necessary?<br></blockquote><div><br></div><div>Ah, interesting, I did not think about that. I'll see if I can make it happen. I'm guessing window_destroy should call window_unconfine_pointer and<br>window_unlock_pointer.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> <br>
> window->locked_input = input;<br>
> window->locked_pointer = locked_pointer;<br>
> @@ -4902,8 +4903,8 @@ static void<br>
> confined_pointer_confined(void *data,<br>
> struct zwp_confined_pointer_v1 *confined_pointer)<br>
> {<br>
> - struct input *input = data;<br>
> - struct window *window = input->pointer_focus;<br>
> + struct window *window = data;<br>
> + struct input *input = window->confined_input;<br>
> <br>
> window->confined = true;<br>
> <br>
> @@ -4918,8 +4919,8 @@ static void<br>
> confined_pointer_unconfined(void *data,<br>
> struct zwp_confined_pointer_v1 *confined_pointer)<br>
> {<br>
> - struct input *input = data;<br>
> - struct window *window = input->pointer_focus;<br>
> + struct window *window = data;<br>
> + struct input *input = window->confined_input;<br>
> <br>
> window_unconfine_pointer(window);<br>
> <br>
> @@ -4984,8 +4985,9 @@ window_confine_pointer_to_rectangles(struct window *window,<br>
> <br>
> zwp_confined_pointer_v1_add_listener(confined_pointer,<br>
> &confined_pointer_listener,<br>
> - input);<br>
> + window);<br>
<br>
The same safeguard question here.<br>
<br>
> <br>
> + window->confined_input = input;<br>
<br>
I was also wondering about what would clean up this field when 'input'<br>
gets destroyed, but I suppose a proper event sequence will clean things<br>
up before 'input' is destroyed.<br></blockquote><div><br></div><div>Interesting. I can't convince myself that cleanup will happen correctly.</div><div>Now that I think about it, I wonder if I should've kept "input" in event handlers - but instead of accessing input->pointer_focus, I should've added new variables (confined_window, locked_window?).</div><div><br></div><div>Anyways, I'll try a few things. Still trying to figure out how all of this works.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> window->confined_pointer = confined_pointer;<br>
> window->confined_widget = NULL;<br>
> <br>
> @@ -5046,6 +5048,7 @@ window_unconfine_pointer(struct window *window)<br>
> zwp_confined_pointer_v1_destroy(window->confined_pointer);<br>
> window->confined_pointer = NULL;<br>
> window->confined = false;<br>
> + window->confined_input = NULL;<br>
> }<br>
> <br>
> static void<br>
<br>
However, I do think this is better than not, so pushed:<br>
da188835..e0dc5d47 master -> master<br>
<br>
I also filed a spec bug:<br>
<a href="https://bugs.freedesktop.org/show_bug.cgi?id=106704" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=106704</a><br>
<br>
<br>
Thanks,<br>
pq<br>
</blockquote></div></div>