Mir

Intermittent deadlock when swithcing to session with custom display configuration while closing other session

Bug #1340669 reported by Alexandros Frantzis on 2014-07-11

This bug affects 1 person

	Status	Importance	Assigned to	Milestone
Mir	Fix Released	High	Alexandros Frantzis	Mir 0.5.0
0.4	Won't Fix	Undecided	Unassigned
mir (Ubuntu)	Fix Released	High	Unassigned

Bug Description

If when closing a session we change focus to another session that has a custom display configuration, we may hit a deadlock.

This can been seen occasionally as a hang when logging out of an X session under XMir. It currently only affects XMir because it is the only production client that applies custom display configurations.

Here is how it happens:

[IPC thread]

~SessionMediator
SessionManager::close_session()
SessionManager::set_focus_to_locked(next session)
BroadcastingSessionEventSink::handle_focus_change()
MediatingDisplayChanger handles the focus change event and enqueues a display reconfiguration action into AsioMainLoop

From this point we have two interesting execution threads:

[IPC thread continues]

~ApplicationSession
...
mir::scene::Observers::surface_removed() *** Acquires READ lock on surface observer [1]
...
AsioMainLoop::unregister_fd_handler *** Enqueues an action into AsioMainLoop and waits for it to complete

[MainLoop thread]

Executes display reconfiguration action:
MultiThreadedCompositor::stop()
mir::scene::Observers::remove() ** Acquires WRITE lock on observer [2]

If the IPC thread calls the surface_removed() observer before the display reconfiguration action is executed, then the MainLoop thread at [2] ends up waiting for a WRITE lock, but can't get it because a READ lock has been acquired at [1]. At the same time, the IPC thread is blocked waiting for unregister_fd_handler() to return, but it doesn't return because the unregistration action has been enqueued after the display reconfiguration handler which is now blocked at [2].

Tags:

Related branches

lp:~afrantzis/mir/fix-1340669-deadlock

Merged into lp:mir at revision 1768

PS Jenkins bot (community): Approve (continuous-integration) on 2014-07-11

Alan Griffiths: Approve on 2014-07-11

lp:ubuntu/utopic-proposed/mir

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-07-11:

backtrace of deadlock Edit (36.5 KiB, text/plain)

Cemil Azizoglu (cemil-azizoglu) on 2014-07-11

tags:

added: rtm14

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-11:

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

Changed in mir:
status:	New → Fix Committed

Daniel van Vugt (vanvugt) on 2014-07-14

Changed in mir:
milestone:	none → 0.5.0

Daniel van Vugt (vanvugt) on 2014-07-16

Changed in mir (Ubuntu):
importance:	Undecided → High
status:	New → Triaged

Revision history for this message

Launchpad Janitor (janitor) wrote on 2014-07-17:

This bug was fixed in the package mir - 0.5.0+14.10.20140717-0ubuntu1

---------------
mir (0.5.0+14.10.20140717-0ubuntu1) utopic; urgency=medium

  [ Daniel van Vugt ]
  * New upstream release 0.5.0 (https://launchpad.net/mir/+milestone/0.5.0)
    - mirclient ABI unchanged at 8. Clients do not need rebuilding.
    - mirserver ABI bumped to 23. Servers need rebuilding, but probably don't
      need modification:
      . DefaultServerConfiguration/Cursor API: Cursor interfaces changed, most
        notably CursorImages moved from ::mir::graphics to ::mir::input.
      . DefaultServerConfiguration: New "prompt" API.
      . DefaultServerConfiguration: "clock" member is now static.
      . SessionAuthorizer: New functions.
      . ServerConfiguration: New function added: the_prompt_connector().
    - Enhancements:
      . Add AddressSanitizer cmake build type.
      . frontend, client API, tests: add support for prompt session
        permissions and for client detecting errors.
      . server: Ensure our emergency cleanup handling infrastructure is
        signal-safe.
      . Implement and enable an xcursor based image loader for cursors.
      . Fix warnings raised by the new g++-4.9.
      . shared, scene: Introduce a generic listener collection.
      . MirMotionEvent: Define a struct typedef to allow for
        pointer_coordinates to be used individually.
    - Bugs fixed:
      . Nexus 10 leaks during overlay operations (LP: #1331769)
      . MultiThreadedCompositor deadlocks (LP: #1335311)
      . Intermittent test failure in ClientSurfaceEvents can client query
        orientation (LP: #1335741)
      . Intermittent test failure in ClientSurfaceEvents/OrientationEvents
        (LP: #1335752)
      . Intermittent memory error in ClientSurfaceEvents on
        orientation query (LP: #1335819)
      . mir_unit_tests.EventDistributorTest.* SEGFAULT (LP: #1338902)
      . [regression] Device locks randomly on welcome screen (LP: #1339700)
      . Intermittent deadlock when switching to session with custom display
        config & closing other session (LP: #1340669)
      . Mir cursor has no hotspot setting, assumes (0, 0) (LP: #1189775)
      . clang built mir_unit_tests.ProtobufSocketCommunicatorFD crashes
        intermittently (LP: #1300653)
      . g++-4.9 binary incompatibilities with libraries built with g++-4.8
        (LP: #1329089)
      . [test regression] SurfaceLoop fails sporadically on deleting surfaces
        for a disconnecting client (LP: #1335747)
      . Intermittent test failure ServerShutdown when clients are blocked
        (LP: #1335873)
      . [regression] mir_demo_client_multiwin is displayed with obviously
        wrong colours (LP: #1339471)
      . Partially onscreen surfaces not occluded when covered by another
        surface (LP: #1340078)
      . SurfaceConfigurator::attribute_set always say "unfocused" for focus
        property changes (LP: #1336548)
-- Ubuntu daily release <email address hidden> Thu, 17 Jul 2014 07:58:53 +0000

This bug was fixed in the package mir - 0.5.0+14.10.20140717-0ubuntu1

---------------
mir (0.5.0+14.10.20140717-0ubuntu1) utopic; urgency=medium

[ Daniel van Vugt ]
  * New upstream release 0.5.0 (https://launchpad.net/mir/+milestone/0.5.0)
    - mirclient ABI unchanged at 8. Clients do not need rebuilding.
    - mirserver ABI bumped to 23. Servers need rebuilding, but probably don't
      need modification:
      . DefaultServerConfiguration/Cursor API: Cursor interfaces changed, most
        notably CursorImages moved from ::mir::graphics to ::mir::input.
      . DefaultServerConfiguration: New "prompt" API.
      . DefaultServerConfiguration: "clock" member is now static.
      . SessionAuthorizer: New functions.
      . ServerConfiguration: New function added: the_prompt_connector().
    - Enhancements:
      . Add AddressSanitizer cmake build type.
      . frontend, client API, tests: add support for prompt session
        permissions and for client detecting errors.
      . server: Ensure our emergency cleanup handling infrastructure is
        signal-safe.
      . Implement and enable an xcursor based image loader for cursors.
      . Fix warnings raised by the new g++-4.9.
      . shared, scene: Introduce a generic listener collection.
      . MirMotionEvent: Define a struct typedef to allow for
        pointer_coordinates to be used individually.
    - Bugs fixed:
      . Nexus 10 leaks during overlay operations (LP: #1331769)
      . MultiThreadedCompositor deadlocks (LP: #1335311)
      . Intermittent test failure in ClientSurfaceEvents can client query
        orientation (LP: #1335741)
      . Intermittent test failure in ClientSurfaceEvents/OrientationEvents
        (LP: #1335752)
      . Intermittent memory error in ClientSurfaceEvents on
        orientation query (LP: #1335819)
      . mir_unit_tests.EventDistributorTest.* SEGFAULT (LP: #1338902)
      . [regression] Device locks randomly on welcome screen (LP: #1339700)
      . Intermittent deadlock when switching to session with custom display
        config & closing other session (LP: #1340669)
      . Mir cursor has no hotspot setting, assumes (0, 0) (LP: #1189775)
      . clang built mir_unit_tests.ProtobufSocketCommunicatorFD crashes
        intermittently (LP: #1300653)
      . g++-4.9 binary incompatibilities with libraries built with g++-4.8
        (LP: #1329089)
      . [test regression] SurfaceLoop fails sporadically on deleting surfaces
        for a disconnecting client (LP: #1335747)
      . Intermittent test failure ServerShutdown when clients are blocked
        (LP: #1335873)
      . [regression] mir_demo_client_multiwin is displayed with obviously
        wrong colours (LP: #1339471)
      . Partially onscreen surfaces not occluded when covered by another
        surface (LP: #1340078)
      . SurfaceConfigurator::attribute_set always say "unfocused" for focus
        property changes (LP: #1336548)
 -- Ubuntu daily release <ps-jenkins@lists.canonical.com>   Thu, 17 Jul 2014 07:58:53 +0000