Comment 9 for bug 1234609

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Some more information about this issue that may lead to a more informed decision about how to solve it:

The core of the problem is that unity-mir reacts asynchronously to events from the SessionListener. MirSurfaceManager connects to the SessionListener::sessionCreatedSurface() signal using Qt::ConnectionType type = Qt::AutoConnection (the default value) [1] which means that if the emitter and the receiver are in different threads, the event is dispatched through the main loop at a later time. Changing this to Qt::BlockedQueuedConnection, which forces for the signal emission mechanism to wait until the event is handled, fixes the problem.

A tangential problem is that the Mir code doesn't currently notify listeners about destroyed surfaces when the session is closed, it only notifies about the session closing. Unity-mir depends on getting surface destruction events to release the mir surface resources it holds (shell::Surface). Adding surface destruction events to shell::ApplicationSession::~ApplicationSession() [2] solves the problem, but conflicts with using Qt::BlockedQueuedConnection as noted above, because in that case the emitter and the receiver are the same thread, and Qt::BlockedQueuedConnection deadlocks.

We have (at least) two options:

(a) Share shell::Surface ownership with the shell, as is done in https://code.launchpad.net/~robertcarr/mir/hold-surface-alive/+merge/189400

(b) Change [1] to use Qt::BlockedQueuedConnection, and change unity-mir to automatically release surface resources when receiving the session stopping signal, without needing explicit surface destruction events.

If option (a) doesn't have any unintended side effects, it would be my preference since it's conceptually cleaner/simpler, and has fewer edge cases.

[1] https://bazaar.launchpad.net/~mir-team/unity-mir/trunk/view/99/src/modules/Unity/Application/mirsurfacemanager.cpp#L64

[2] This is the same problem and fix as in https://bazaar.launchpad.net/~robertcarr/mir/hold-surface-alive/revision/1066