Mir

[regression] [BufferQueue] double-buffered client freezes as no buffer is returned on compositor_release.

Bug #1319765 reported by Daniel van Vugt
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mir
Fix Released
Medium
Daniel van Vugt
mir (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

Double-buffered clients can freeze in some cases, indefinitely waiting for a callback that never comes (because no new compositor frames are scheduled).

Test case:
  1. Force double-buffering in the server by changing 3 to 2 in buffer_stream_factory.cpp.
  2. Start your modified server using: mir_demo_server_shell
  3. Run: mir_demo_client_egltriangle
  4. Rotate the screen (Ctrl+Alt+Left/Right/Down)

Expected: Screen rotates and client keeps rendering.
Observed: Screen rotates but client is frozen until you drag or resize it.

Tags: regression

Related branches

summary: - [regression] [BufferQueue] client freezes as no buffer is returned after
+ [regression] [BufferQueue] client freezes as no buffer is returned on
compositor_release.
Revision history for this message
Alexandros Frantzis (afrantzis) wrote : Re: [regression] [BufferQueue] client freezes as no buffer is returned on compositor_release.

> if (current_compositor_buffer != buffer.get() && nbuffers > 1)
> release(buffer.get(), std::move(lock));
> // else ... what happens to buffer? It's not in any queue any more, is it?

If the buffer is not released (and assuming nbuffers >1), then this means that the buffer is stored in "current_compositor_buffer" and therefore is not lost. The current_compositor_buffer is released to the client when in compositor_acquire() we decide that it's time to give out a newer buffer (i.e. !should_use_current_buffer).

So, everything seems to be in order here. Of course, there may be a sequence of actions that is problematic and we haven't yet discovered, but we need more information to investigate further.

Changed in mir:
status: New → Incomplete
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The problem then, essentially is that there is no subsequent compositor_acquire() call for the client callback to originate from. Because no new frames are scheduled while the (only) client is already frozen waiting for a buffer. I can confirm that it un-freezes and comes back to life if you force a new frame (compositor_acquire) in the demo-shell: resize/move/opacity the surface.

I'll need to review this in detail again...

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Dropped importance to medium as the bug only occurs if you have switched to double buffering (which is presently never at all, in the current Mir code). It is a regression though because this used to work.

summary: - [regression] [BufferQueue] client freezes as no buffer is returned on
- compositor_release.
+ [regression] [BufferQueue] double-buffered client freezes as no buffer
+ is returned on compositor_release.
Changed in mir:
importance: Critical → Medium
status: Incomplete → Triaged
description: updated
Changed in mir:
assignee: nobody → Daniel van Vugt (vanvugt)
status: Triaged → In Progress
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

Fix committed into lp:mir/devel at revision None, scheduled for release in mir, milestone Unknown

Changed in mir:
status: In Progress → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (7.2 KiB)

This bug was fixed in the package mir - 0.2.0+14.10.20140605-0ubuntu1

---------------
mir (0.2.0+14.10.20140605-0ubuntu1) utopic; urgency=medium

  [ Daniel van Vugt ]
  * New upstream release 0.2.0 (https://launchpad.net/mir/+milestone/0.2.0)
    - mirclient ABI unchanged, still at 7. Clients do not need rebuilding.
    - mirserver ABI bumped to 20. Shells need rebuilding.
      . Cursor::set_image() parameters changed.
      . Display::the_cursor() renamed to Display::create_hardware_cursor()
      . Platform::create_display() requires a new parameter; gl_program_factory
      . Renderable::buffer() no longer accepts any parameter at all. Multi-
        monitor frame sync is guaranteed in other ways now.
      . Scene::generate_renderable_list() renamed to renderable_list_for(id)
        where id is an opaque compositor ID of your choosing.
      . Scene::set_change_callback() replaced by the more generic:
        add_observer() and remove_observer() functions.
      . Added default implementation for SceneObserver.
      . SessionCreator renamed to ConnectionCreator.
      . ConnectedSessions renamed to Connections.
      . ProtobufSessionCreator renamed to ProtobufConnectionCreator.
      . SessionAuthorizer: pid_t parameters replaced by SessionCredentials.
      . Massive architectural changes to Input-everything.
      . Surface no longer implements Renderable, but emits one via
        compositor_snapshot().
      . Pass the full renderable list to Renderer::render().
      . Graceful handling of exceptions thrown from server threads.
      . Clarify size position interfaces in the Surface classes.
      . Plumbing for trusted sessions.
      . Allow posting and managing custom main-loop actions.
      . Timer extension.
      . Identify client process when client connects to Mir not when socket
        connects.
      . Use the ServerActionQueue for display config.
      . Recomposition signal moved to the MultiThreadedCompositor.
      . Make timer service replaceable.
      . Clarify assumptions about how many buffers a client can fill without
        blocking.
      . Introduce EmergencyCleanup interface.
    - Demo shell enhancements:
      . You can now change graphics modes on the fly using Ctrl+Alt+(-/=).
        To reset to the preferred mode use Ctrl+Alt+0.
      . The above mode changing as well as existing screen rotation keys
        (Ctrl+Alt+arrows) are now per-display; only applied to the monitor
        the mouse pointer is on.
      . New shell controls documented.
    - A new testing category, performance test, was introduced. It currently
      runs glmark2-es2 and compares the result to a minimum threshold.
    - MIR_VERSION_MINOR is tied to MIRSERVER_ABI in the sense that a change
      in the former now requires dependent projects that a rebuild is
      necessary.
    - SwitchingBundle was replaced by BufferQueue.
    - Expand credentials to include uid/gid for session authorizer.
    - Bypass control is now Mesa-specific and tied to the command line options.
      So the environment variable MIR_BYPASS has changed to MIR_SERVER_BYPASS.
    - Ongoing architectural changes in the compositor/renderer logic to
   ...

Read more...

Changed in mir (Ubuntu):
status: New → Fix Released
Changed in mir:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.