GNOME Bugzilla – Bug 738686
Fix problems resulting in left-over queued frames
Last modified: 2014-12-11 16:57:50 UTC
I observed in some testing that we were spending a bunch of time traversing the list of queued frames as if the list of frames was growing without bound; putting in some debugging prints I found a number of problems.
Created attachment 288750 [details] [review] Fix problems resulting in left-over queued frames * Use -1 rather than 0 as a flag for pending queue entries; 0 is a valid frame_counter value from Cogl. * Consistently handle the fact we can have more than one pending entry. It's app misbehavior to submit a new frame before _NET_WM_FRAME_DRAWN is received; but we accept such frame messages, so we can't just leak them. * If we remove send_frame_message_timer, assign the current frame counter to pending entries. * To try to avoid regressing on this, when sending _NET_WM_FRAME_TIMINGS messages, if we have stale messages, or messages with no frame drawn time, warn and remove them from the queue rather than just accumulating. * Improve commenting.
Created attachment 288751 [details] [review] MetaWindowActor: don't overwrite send_frame_messages_timer If the app finished multiple frames before we sent _NET_WM_FRAME_DRAWN, we could add the send_frame_messages_timer multiple times. In the rare case that the app immediately closed the window, the older timeout could potentially then run on the freed actor.
Review of attachment 288750 [details] [review]: Looks good modulo typo. Re second bullet ... which application actually does that? The messages are handled by gtk so we probably should fix gtk as well. ::: src/compositor/meta-window-actor.c @@ +119,3 @@ typedef struct _FrameData FrameData; +/* Each time the applicatio updates the sync request counter to a new even value applicatio_n_
Review of attachment 288751 [details] [review]: Oh good catch. That might / should fix https://bugzilla.redhat.com/show_bug.cgi?id=1005299
(In reply to comment #3) > Review of attachment 288750 [details] [review]: > > Looks good modulo typo. > > Re second bullet ... which application actually does that? The messages are > handled by gtk so we probably should fix gtk as well. This part of the problem was noticed by inspection and trying to make the code paths consistent. What does it mean that there's a frame in the queue with frame_counter==-1 ? OK - that indicates the app incremented the frame sync counter to an even value and we haven't yet drawn a frame for it. Can this happen more than once in the normal case? yes. In the send_frame_messages_timeout case? yes. So we need to make sure that in both cases we handle that correctly. Because I don't think GTK+ does this, it doesn't seem immediately obvious to me that it's causing the crashes from the Red Hat bug, but it's certainly conceivably that there might be some edge case where if a window is mapped and unmapped or something it could occur.
*** Bug 734171 has been marked as a duplicate of this bug. ***
Attachment 288750 [details] pushed as 9745f9f - Fix problems resulting in left-over queued frames Attachment 288751 [details] pushed as 5e84c8f - MetaWindowActor: don't overwrite send_frame_messages_timer
*** Bug 741379 has been marked as a duplicate of this bug. ***
Any chance to get a patch release for this? I am getting this crash quite often
(In reply to comment #9) > Any chance to get a patch release for this? I am getting this crash quite often The patches are in the stable (3.14) branch as well so should be in the next 3.14.x release.