Re: [PATCH weston] xwm: Check whether the seat is NULL sometimes in weston_wm_handle_button
On Mon, 01 Sep 2014 20:07:46 +0800
Boyan Ding stu_...@126.com wrote:
On Mon, 2014-09-01 at 12:14 +0300, Pekka Paalanen wrote:
On Fri, 29 Aug 2014 22:10:32 +0800
Boyan Ding stu_...@126.com wrote:
Under some certain circumstances, pointer button may have been released
when frame is still being resized/moved. When this happens, the picked
seat is NULL and it will segfault when moving/resizing surfaces. Check
whether the seat is NULL and ignore move/resize in that case.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=82827
Signed-off-by: Boyan Ding stu_...@126.com
---
xwayland/window-manager.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index a216b76..f633324 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -1648,12 +1648,14 @@ weston_wm_handle_button(struct weston_wm *wm,
xcb_generic_event_t *event)
weston_wm_window_schedule_repaint(window);
if (frame_status(window-frame) FRAME_STATUS_MOVE) {
- shell_interface-move(window-shsurf, seat);
+ if (seat != NULL)
+ shell_interface-move(window-shsurf, seat);
frame_status_clear(window-frame, FRAME_STATUS_MOVE);
}
if (frame_status(window-frame) FRAME_STATUS_RESIZE) {
- shell_interface-resize(window-shsurf, seat, location);
+ if (seat != NULL)
+ shell_interface-resize(window-shsurf, seat, location);
frame_status_clear(window-frame, FRAME_STATUS_RESIZE);
}
Hi,
do you know if this condition is something that should be silently
ignored like in your patch, or should we at least print an error that
something unexpected is happening and being papered over?
Looking at how the seat is found:
static struct weston_seat *
weston_wm_pick_seat_for_window(struct weston_wm_window *window)
{
struct weston_wm *wm = window-wm;
struct weston_seat *seat, *s;
seat = NULL;
wl_list_for_each(s, wm-server-compositor-seat_list, link) {
if (s-pointer != NULL
s-pointer-focus == window-view
s-pointer-button_count 0
(seat == NULL ||
s-pointer-grab_serial -
seat-pointer-grab_serial (1 30)))
seat = s;
}
return seat;
}
and that gets called as a response to an XCB input event via
weston_wm_handle_button...
The function will return NULL if there are no buttons pressed, even if
the pointer is focused on the window. Does that make sense in general?
Does it not cause every last-button-up event to hit seat==NULL?
So why don't we see this problem more often?
Could there be a problem in the shared frame code, maybe it makes
assumptions that won't work with X11?
Sorry I was in a rush when replying the last mail before leaving my
table. Now I think it may be because the race in X and wayland.
FRAME_STATUS_RESIZE (and so on) are set in frame.c, using wayland event
handling, and are cleared in xwm, using XCB. The pointer may have been
released between the two events. So the picked seat may be NULL while we
have FRAME_STATUS_RESIZE set. I think we can safely ignore the event if
pointer is released between the two.
Okay, so you believe the patch is really correct then?
Could you amend the commit message to include your understanding of
what is happening, rather than just some certain circumstances?
You don't have to be 100% confident that you are right.
That should make the patch perfect. :-)
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] xwm: Check whether the seat is NULL sometimes in weston_wm_handle_button
On Fri, 29 Aug 2014 22:10:32 +0800
Boyan Ding stu_...@126.com wrote:
Under some certain circumstances, pointer button may have been released
when frame is still being resized/moved. When this happens, the picked
seat is NULL and it will segfault when moving/resizing surfaces. Check
whether the seat is NULL and ignore move/resize in that case.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=82827
Signed-off-by: Boyan Ding stu_...@126.com
---
xwayland/window-manager.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index a216b76..f633324 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -1648,12 +1648,14 @@ weston_wm_handle_button(struct weston_wm *wm,
xcb_generic_event_t *event)
weston_wm_window_schedule_repaint(window);
if (frame_status(window-frame) FRAME_STATUS_MOVE) {
- shell_interface-move(window-shsurf, seat);
+ if (seat != NULL)
+ shell_interface-move(window-shsurf, seat);
frame_status_clear(window-frame, FRAME_STATUS_MOVE);
}
if (frame_status(window-frame) FRAME_STATUS_RESIZE) {
- shell_interface-resize(window-shsurf, seat, location);
+ if (seat != NULL)
+ shell_interface-resize(window-shsurf, seat, location);
frame_status_clear(window-frame, FRAME_STATUS_RESIZE);
}
Hi,
do you know if this condition is something that should be silently
ignored like in your patch, or should we at least print an error that
something unexpected is happening and being papered over?
Looking at how the seat is found:
static struct weston_seat *
weston_wm_pick_seat_for_window(struct weston_wm_window *window)
{
struct weston_wm *wm = window-wm;
struct weston_seat *seat, *s;
seat = NULL;
wl_list_for_each(s, wm-server-compositor-seat_list, link) {
if (s-pointer != NULL
s-pointer-focus == window-view
s-pointer-button_count 0
(seat == NULL ||
s-pointer-grab_serial -
seat-pointer-grab_serial (1 30)))
seat = s;
}
return seat;
}
and that gets called as a response to an XCB input event via
weston_wm_handle_button...
The function will return NULL if there are no buttons pressed, even if
the pointer is focused on the window. Does that make sense in general?
Does it not cause every last-button-up event to hit seat==NULL?
So why don't we see this problem more often?
Could there be a problem in the shared frame code, maybe it makes
assumptions that won't work with X11?
If no-one knows (i.e. no-one replies assuring one way or the other), I
can merge this patch if someone at least confirms it fixes an issue.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] xwm: Check whether the seat is NULL sometimes in weston_wm_handle_button
On Mon, 2014-09-01 at 12:14 +0300, Pekka Paalanen wrote:
On Fri, 29 Aug 2014 22:10:32 +0800
Boyan Ding stu_...@126.com wrote:
Under some certain circumstances, pointer button may have been released
when frame is still being resized/moved. When this happens, the picked
seat is NULL and it will segfault when moving/resizing surfaces. Check
whether the seat is NULL and ignore move/resize in that case.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=82827
Signed-off-by: Boyan Ding stu_...@126.com
---
xwayland/window-manager.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index a216b76..f633324 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -1648,12 +1648,14 @@ weston_wm_handle_button(struct weston_wm *wm,
xcb_generic_event_t *event)
weston_wm_window_schedule_repaint(window);
if (frame_status(window-frame) FRAME_STATUS_MOVE) {
- shell_interface-move(window-shsurf, seat);
+ if (seat != NULL)
+ shell_interface-move(window-shsurf, seat);
frame_status_clear(window-frame, FRAME_STATUS_MOVE);
}
if (frame_status(window-frame) FRAME_STATUS_RESIZE) {
- shell_interface-resize(window-shsurf, seat, location);
+ if (seat != NULL)
+ shell_interface-resize(window-shsurf, seat, location);
frame_status_clear(window-frame, FRAME_STATUS_RESIZE);
}
Hi,
do you know if this condition is something that should be silently
ignored like in your patch, or should we at least print an error that
something unexpected is happening and being papered over?
Looking at how the seat is found:
static struct weston_seat *
weston_wm_pick_seat_for_window(struct weston_wm_window *window)
{
struct weston_wm *wm = window-wm;
struct weston_seat *seat, *s;
seat = NULL;
wl_list_for_each(s, wm-server-compositor-seat_list, link) {
if (s-pointer != NULL
s-pointer-focus == window-view
s-pointer-button_count 0
(seat == NULL ||
s-pointer-grab_serial -
seat-pointer-grab_serial (1 30)))
seat = s;
}
return seat;
}
and that gets called as a response to an XCB input event via
weston_wm_handle_button...
The function will return NULL if there are no buttons pressed, even if
the pointer is focused on the window. Does that make sense in general?
Does it not cause every last-button-up event to hit seat==NULL?
So why don't we see this problem more often?
Could there be a problem in the shared frame code, maybe it makes
assumptions that won't work with X11?
At least I think weston_wm_pick_seat_for_window makes sense where it is
used. If no button is down, things like resizing can be safely ignored.
And I don't think it has anything to do with X11 since input handling
here (and in Xwayland) is pure wayland.
I guess (it's only a guess) maybe it's caused by some race conditions in
frame and the event handling here?
Thanks,
Boyan Ding
If no-one knows (i.e. no-one replies assuring one way or the other), I
can merge this patch if someone at least confirms it fixes an issue.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] xwm: Check whether the seat is NULL sometimes in weston_wm_handle_button
On Mon, 2014-09-01 at 12:14 +0300, Pekka Paalanen wrote:
On Fri, 29 Aug 2014 22:10:32 +0800
Boyan Ding stu_...@126.com wrote:
Under some certain circumstances, pointer button may have been released
when frame is still being resized/moved. When this happens, the picked
seat is NULL and it will segfault when moving/resizing surfaces. Check
whether the seat is NULL and ignore move/resize in that case.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=82827
Signed-off-by: Boyan Ding stu_...@126.com
---
xwayland/window-manager.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index a216b76..f633324 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -1648,12 +1648,14 @@ weston_wm_handle_button(struct weston_wm *wm,
xcb_generic_event_t *event)
weston_wm_window_schedule_repaint(window);
if (frame_status(window-frame) FRAME_STATUS_MOVE) {
- shell_interface-move(window-shsurf, seat);
+ if (seat != NULL)
+ shell_interface-move(window-shsurf, seat);
frame_status_clear(window-frame, FRAME_STATUS_MOVE);
}
if (frame_status(window-frame) FRAME_STATUS_RESIZE) {
- shell_interface-resize(window-shsurf, seat, location);
+ if (seat != NULL)
+ shell_interface-resize(window-shsurf, seat, location);
frame_status_clear(window-frame, FRAME_STATUS_RESIZE);
}
Hi,
do you know if this condition is something that should be silently
ignored like in your patch, or should we at least print an error that
something unexpected is happening and being papered over?
Looking at how the seat is found:
static struct weston_seat *
weston_wm_pick_seat_for_window(struct weston_wm_window *window)
{
struct weston_wm *wm = window-wm;
struct weston_seat *seat, *s;
seat = NULL;
wl_list_for_each(s, wm-server-compositor-seat_list, link) {
if (s-pointer != NULL
s-pointer-focus == window-view
s-pointer-button_count 0
(seat == NULL ||
s-pointer-grab_serial -
seat-pointer-grab_serial (1 30)))
seat = s;
}
return seat;
}
and that gets called as a response to an XCB input event via
weston_wm_handle_button...
The function will return NULL if there are no buttons pressed, even if
the pointer is focused on the window. Does that make sense in general?
Does it not cause every last-button-up event to hit seat==NULL?
So why don't we see this problem more often?
Could there be a problem in the shared frame code, maybe it makes
assumptions that won't work with X11?
Sorry I was in a rush when replying the last mail before leaving my
table. Now I think it may be because the race in X and wayland.
FRAME_STATUS_RESIZE (and so on) are set in frame.c, using wayland event
handling, and are cleared in xwm, using XCB. The pointer may have been
released between the two events. So the picked seat may be NULL while we
have FRAME_STATUS_RESIZE set. I think we can safely ignore the event if
pointer is released between the two.
Regards,
Boyan Ding
If no-one knows (i.e. no-one replies assuring one way or the other), I
can merge this patch if someone at least confirms it fixes an issue.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
