Oh, sorry, I wasn't sure how to reproduce the other case. (Did you see my last email? I don't know if I should write a new client, or if I can test it with existing ones.)<div>I have some time, I'm just not familiar with the code :)</div> <div> <div class="gmail_quote">On Sun, Jul 22, 2012 at 8:45 AM, Kristian Høgsberg <<a href="mailto:hoegsberg@gmail.com" target="_blank">hoegsberg@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="HOEnZb"><div class="h5">On Tue, Jul 10, 2012 at 02:35:10PM -0400, Kristian Høgsberg wrote: > On Tue, Jul 10, 2012 at 09:28:11AM -0700, Dima Ryazanov wrote: > > I see the following use-after-free when a client disconnects: > > > > ==21822== Invalid write of size 8 > > ==21822== at 0x54A8F74: wl_list_remove (wayland-util.c:51) > > ==21822== by 0x409C87: destroy_frame_callback (compositor.c:1362) > > ==21822== by 0x56B44E7: destroy_resource (wayland-server.c:427) > > ==21822== by 0x54A9557: for_each_helper (wayland-util.c:268) > > ==21822== by 0x54A9593: wl_map_for_each (wayland-util.c:274) > > ==21822== by 0x56B4640: wl_client_destroy (wayland-server.c:474) > > ==21822== by 0x56B3E52: wl_client_connection_data (wayland-server.c:231) > > ==21822== by 0x56B7BD9: wl_event_source_fd_dispatch (event-loop.c:79) > > ==21822== by 0x56B85AE: wl_event_loop_dispatch (event-loop.c:410) > > ==21822== by 0x56B5C05: wl_display_run (wayland-server.c:1090) > > ==21822== by 0x40ECBE: main (compositor.c:3394) > > ==21822== Address 0xc793d98 is 616 bytes inside a block of size 680 free'd > > ==21822== at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > > ==21822== by 0x407DAB: destroy_surface (compositor.c:689) > > ==21822== by 0x56B44E7: destroy_resource (wayland-server.c:427) > > ==21822== by 0x54A9557: for_each_helper (wayland-util.c:268) > > ==21822== by 0x54A9593: wl_map_for_each (wayland-util.c:274) > > ==21822== by 0x56B4640: wl_client_destroy (wayland-server.c:474) > > ==21822== by 0x56B3E52: wl_client_connection_data (wayland-server.c:231) > > ==21822== by 0x56B7BD9: wl_event_source_fd_dispatch (event-loop.c:79) > > ==21822== by 0x56B85AE: wl_event_loop_dispatch (event-loop.c:410) > > ==21822== by 0x56B5C05: wl_display_run (wayland-server.c:1090) > > ==21822== by 0x40ECBE: main (compositor.c:3394) > > > > This seems to fix it for me, though I've no idea if it's actually correct. > > It's actually an elegant fix for the case you hit (client disconnect), > since the callback objects on the list will be destroyed by the > resource cleanup, so all we need to to is take the list head out to > prevent the callback objects from accessing that freed memory as > they're destroyed. > > We have a different case to consider though. When a surface is > explicitly destroyed by calling surface.destroy(), we still need to > destroy pending callbacks, and that case the resource system wont do > it for us. So we need to iterate through the list and free them > manually. </div></div>I was hoping you'd have time to implement the above, but I've pushed 1e51fecdf5ade44c77de325679f1e010d8ba4273 now, which should fix the problem and handle the regular destroy case. Kristian <div class="HOEnZb"><div class="h5"> > > Kristian > > > --- > > src/compositor.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/src/compositor.c b/src/compositor.c > > index 678e0bd..9fe561a 100644 > > --- a/src/compositor.c > > +++ b/src/compositor.c > > @@ -686,6 +686,8 @@ destroy_surface(struct wl_resource *resource) > > if (!region_is_undefined(&surface->input)) > > pixman_region32_fini(&surface->input); > > > > + wl_list_remove(&surface->frame_callback_list); > > + > > free(surface); > > } > > > > -- > > 1.7.9.5 > > > > _______________________________________________ > > wayland-devel mailing list > > <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> > > <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </div></div></blockquote></div> </div>