[PATCH weston] RDP compositor: make the seat dynamic and don't destroy it on removal

[Pekka Paalanen]

On Thu, 24 Sep 2015 23:40:26 +0200
David FORT <rdp.effort at gmail.com> wrote:

> This patch makes the seat dynamic and leak it on purpose during seat removal to
> prevent the ghost object case.
> ---
>  src/compositor-rdp.c | 39 +++++++++++++++++++++++++++------------
>  1 file changed, 27 insertions(+), 12 deletions(-)

Hi David,

this patch is still missing the whole explanation of what is going on in
here. My questions from
http://lists.freedesktop.org/archives/wayland-devel/2015-May/022055.html
are still unanswered, and today I understand even less than then. :-)

Or is there already a comment in the code explaining why rdp-backend
does funny stuff with seats? I couldn't find it on a quick look.

Why leak it?
What is the ghost object problem?
Why you must use only part of weston_seat_release()?

I have some very vague memories of wl_seat missing destructor protocol
or something, is this related?


Thanks,
pq

> diff --git a/src/compositor-rdp.c b/src/compositor-rdp.c
> index c221eb9..7abffee 100644
> --- a/src/compositor-rdp.c
> +++ b/src/compositor-rdp.c
> @@ -110,7 +110,7 @@ enum peer_item_flags {
>  struct rdp_peers_item {
>  	int flags;
>  	freerdp_peer *peer;
> -	struct weston_seat seat;
> +	struct weston_seat *seat;
>  
>  	struct wl_list link;
>  };
> @@ -628,6 +628,7 @@ static void
>  rdp_peer_context_free(freerdp_peer* client, RdpPeerContext* context)
>  {
>  	int i;
> +	struct weston_seat *seat;
>  	if (!context)
>  		return;
>  
> @@ -638,9 +639,17 @@ rdp_peer_context_free(freerdp_peer* client, RdpPeerContext* context)
>  	}
>  
>  	if (context->item.flags & RDP_PEER_ACTIVATED) {
> -		weston_seat_release_keyboard(&context->item.seat);
> -		weston_seat_release_pointer(&context->item.seat);
> -		weston_seat_release(&context->item.seat);
> +		seat = context->item.seat;
> +		weston_seat_release_keyboard(seat);
> +		weston_seat_release_pointer(seat);
> +
> +		/* picked from weston_seat_release(context->item.seat); */
> +		wl_list_remove(&seat->link);
> +		if (seat->saved_kbd_focus)
> +			wl_list_remove(&seat->saved_kbd_focus_listener.link);
> +		wl_global_destroy(seat->global);
> +		wl_signal_emit(&seat->destroy_signal, seat);
> +
>  	}
>  
>  	Stream_Free(context->encode_stream, TRUE);
> @@ -910,9 +919,15 @@ xf_peer_activate(freerdp_peer* client)
>  	else
>  		snprintf(seat_name, sizeof(seat_name), "RDP peer @%s", settings->ClientAddress);
>  
> -	weston_seat_init(&peersItem->seat, b->compositor, seat_name);
> -	weston_seat_init_keyboard(&peersItem->seat, keymap);
> -	weston_seat_init_pointer(&peersItem->seat);
> +	peersItem->seat = zalloc(sizeof(*peersItem->seat));
> +	if (!peersItem->seat) {
> +		weston_log("unable to allocate the seat for %s", seat_name);
> +		return FALSE;
> +	}
> +
> +	weston_seat_init(peersItem->seat, &b->compositor, seat_name);
> +	weston_seat_init_keyboard(peersItem->seat, keymap);
> +	weston_seat_init_pointer(peersItem->seat);
>  
>  	peersItem->flags |= RDP_PEER_ACTIVATED;
>  
> @@ -953,7 +968,7 @@ xf_mouseEvent(rdpInput *input, UINT16 flags, UINT16 x, UINT16 y)
>  		if (x < output->base.width && y < output->base.height) {
>  			wl_x = wl_fixed_from_int((int)x);
>  			wl_y = wl_fixed_from_int((int)y);
> -			notify_motion_absolute(&peerContext->item.seat, weston_compositor_get_time(),
> +			notify_motion_absolute(peerContext->item.seat, weston_compositor_get_time(),
>  					wl_x, wl_y);
>  		}
>  	}
> @@ -966,7 +981,7 @@ xf_mouseEvent(rdpInput *input, UINT16 flags, UINT16 x, UINT16 y)
>  		button = BTN_MIDDLE;
>  
>  	if (button) {
> -		notify_button(&peerContext->item.seat, weston_compositor_get_time(), button,
> +		notify_button(peerContext->item.seat, weston_compositor_get_time(), button,
>  			(flags & PTR_FLAGS_DOWN) ? WL_POINTER_BUTTON_STATE_PRESSED : WL_POINTER_BUTTON_STATE_RELEASED
>  		);
>  	}
> @@ -982,7 +997,7 @@ xf_mouseEvent(rdpInput *input, UINT16 flags, UINT16 x, UINT16 y)
>  		if (flags & PTR_FLAGS_WHEEL_NEGATIVE)
>  			axis = -axis;
>  
> -		notify_axis(&peerContext->item.seat, weston_compositor_get_time(),
> +		notify_axis(peerContext->item.seat, weston_compositor_get_time(),
>  					    WL_POINTER_AXIS_VERTICAL_SCROLL,
>  					    axis);
>  	}
> @@ -1001,7 +1016,7 @@ xf_extendedMouseEvent(rdpInput *input, UINT16 flags, UINT16 x, UINT16 y)
>  	if (x < output->base.width && y < output->base.height) {
>  		wl_x = wl_fixed_from_int((int)x);
>  		wl_y = wl_fixed_from_int((int)y);
> -		notify_motion_absolute(&peerContext->item.seat, weston_compositor_get_time(),
> +		notify_motion_absolute(peerContext->item.seat, weston_compositor_get_time(),
>  				wl_x, wl_y);
>  	}
>  
> @@ -1064,7 +1079,7 @@ xf_input_keyboard_event(rdpInput *input, UINT16 flags, UINT16 code)
>  
>  		/*weston_log("code=%x ext=%d vk_code=%x scan_code=%x\n", code, (flags & KBD_FLAGS_EXTENDED) ? 1 : 0,
>  				vk_code, scan_code);*/
> -		notify_key(&peerContext->item.seat, weston_compositor_get_time(),
> +		notify_key(peerContext->item.seat, weston_compositor_get_time(),
>  					scan_code - 8, keyState, STATE_UPDATE_AUTOMATIC);
>  	}
>  

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150925/d0631f40/attachment.sig>