On 25/09/15 08:19 AM, Hardening wrote: > Le 25/09/2015 11:31, Pekka Paalanen a écrit : >> On Thu, 24 Sep 2015 23:40:26 +0200 >> David FORT <[email protected]> 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? > > > Hello Pekka, > you're right I've not elaborated much. > > So the general problem was that wl_seat doesn't have a release request, > so we can't track the usage of a wl_seat by wayland clients. And as we > can't track the usage, we can't release it safely or we take the risk > that clients could address a released object. In the current situation > we can't safely release a seat object.
Should we move forward with your patch to add seat release protocol first? Does that change this implementation at all? > So in my patch I'm mallocating a seat (not having it static with the RDP > context), an I took the parts of weston_seat_release() that only do > things internally to weston (so that I release as much as I can). > > > Perhaps I'm not cleaning up correctly because with my patch applied, if I: > * open a terminal; > * connect with 2 RDP clients (so 2 seats); > * take the focus in the terminal; > * stop weston with a Ctrl-C > > I get the following valgrind traceback during weston's shutdown, related > to the input method: > ^C[14:35:47.687] caught signal 2 > ==21909== Invalid read of size 8 > ==21909== at 0x4173B9: unbind_input_method (text-backend.c:834) > ==21909== by 0x4E3D37B: destroy_resource (wayland-server.c:537) > ==21909== by 0x4E42255: for_each_helper.isra.0 (wayland-util.c:359) > ==21909== by 0x4E4279E: wl_map_for_each (wayland-util.c:365) > ==21909== by 0x4E3DF57: wl_client_destroy (wayland-server.c:675) > ==21909== by 0x418401: text_backend_destroy (text-backend.c:1046) > ==21909== by 0x93542F2: shell_destroy (shell.c:6473) > ==21909== by 0x4109C6: weston_compositor_destroy > (wayland-server-core.h:264) > ==21909== by 0x40897D: main (main.c:829) > ==21909== Address 0x9b9e6f0 is 112 bytes inside a block of size 120 free'd > ==21909== at 0x4C2BDEC: free (in > /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==21909== by 0x6A468AE: rdp_peer_context_free (wayland-server-core.h:264) > ==21909== by 0x6D520DA: freerdp_peer_context_free (peer.c:738) > ==21909== by 0x6A469B7: rdp_client_activity (compositor-rdp.c:674) > ==21909== by 0x4E3FE51: wl_event_loop_dispatch (event-loop.c:422) > ==21909== by 0x4E3E6B4: wl_display_run (wayland-server.c:1004) > ==21909== by 0x408D35: main (main.c:818) > ==21909== > > > Anyway without the patch, valgrind complains during seat releasing (for > example when a RDP peer disconnects). Sigh. There's a lot of multi-seat related problems in the text-backend/input-method bits. I've got a bunch of patches I need to sort through and post. :/ I don't think you've added this problem, I think it's already there (and I may have fixed it already...) _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
