GNOME Bugzilla – Bug 754158
wayland: Don't broadcast selection owner changes
Last modified: 2015-09-02 03:29:35 UTC
I discovered this while debugging something looking like a memory leak in GNOME Terminal. What seemed to be leaking was GDK_SELECTION_NOTIFY events created in gdkselection-wayland.c but I have yet to find out why they actually leaked, or if something other funky was going on. What I discovered though is that O(n^2) "GDK_OWNER_CHANGE" events were emitted for n open terminal windows, which seemed odd. This patch fixes that case by not broadcasting the change, but only sending it once per time it happens. I'm not sure whether this actually fixes the issues I have been seeing (I have logs where ~59000 GDK_SELECTION_NOTIFY events are queued up but only ~27000 being emitted before more are being queued which is extremely odd by itself). The extreme GDK_SELECTION_NOTIFY queueing I've been seeing is very hard to reproduce, and happens once or less per day of quite heavy use. I have no idea if this patch affects this since but I guess I will find out in due time.
Created attachment 310075 [details] [review] wayland: Don't broadcast selection owner changes When receiving a selection or when a drag icon enter a window, it was targeted at a specific window. Lets emit the GDK_OWNER_CHANGE event only for this window, instead of broadcasting. Broadcasting has some nasty side effects. For example, if there was n GdkWindows, and one would for every "owner-change" signal handler receive n signals about the owner being changed. An example of where this went a bit out of hand was gnome-terminal, which added one listener per terminal window. This meant that if one had m number of terminal windows, each time any one would loose or gain keyboard focus, O(m^2) owner-change events would be emitted.
Review of attachment 310075 [details] [review]: This looks right to me. Looking at the GTK+ side, the code handling owner-change events goes window -> display -> clipboard, and then emits an owner-change signal on the clipboard. No need to do the same dance for each toplevel
Attachment 310075 [details] pushed as d682aed - wayland: Don't broadcast selection owner changes