GNOME Bugzilla – Bug 703476
tests/actor-offscreen-redirect: Fix race condition
Last modified: 2013-07-02 22:24:18 UTC
We've been observing this test failing very frequently when the machine is under load; presumably the timeout is firing before the stage has had a chance to do the initial draw. Close this race by simply waiting for the initial paint synchronously.
Created attachment 248246 [details] [review] tests/actor-offscreen-redirect: Fix race condition
Created attachment 248256 [details] [review] conform: Use a repaint function Timeouts and idles are subject to the whims of the load of the machine running the tests, as we found out with the new installed tests and OSTree-based VM running the conformance test suite continuously. We should be able to use a repaint function and a blocking loop that either is terminated because we hit g_assert(), or because a flag gets toggled once we know that the Stage has been at least painted once. The currently enabled tests using clutter_stage_read_pixels() have been updated to this approach.
Review of attachment 248256 [details] [review]: This patch looks reasonable at a high level to me, but I don't know the tests that well. The actor-offscreen-redirect diff is somewhat obscured by the de-indentation, but if switching to clutter_threads_add_repaint_func_full(_POST_PAINT) is enough to fix this, then it looks good to me. Just one question: ::: tests/conform/texture-fbo.c @@ -227,3 @@ - /* We force continuous redrawing of the stage, since we need to skip - * the first few frames, and we wont be doing anything else that - * will trigger redrawing. */ Does this comment no longer apply? We're doing a continuous redraw queue in the other actor-layout.c test.
I think the actor-layout test should also stop queuing redraws: we can get away with just one, since we're blocking until it happens. with the timeout, the continuous redraw was added to work around races, and because I actually copied the code from previously existing tests without actually questioning the validity of the approach.
actually, I just pushed a commit that removes the continuous queue_redraw() from actor-layout.c as well — and the installed tests seem to be working. thanks!