Mir

Merge lp://staging/~andreas-pokorny/mir/wait-for-input-config-before-emitting-input-state into lp://staging/mir

  1. wait-for-input-config-before-emitting-input-state
  2. Merge into development-branch
Proposed by Andreas Pokorny
Status: Merged
Merged at revision: 4112
Proposed branch: lp://staging/~andreas-pokorny/mir/wait-for-input-config-before-emitting-input-state
Merge into: lp://staging/mir
Diff against target: 118 lines (+43/-29)
2 files modified
src/server/graphics/nested/input_platform.cpp (+41/-29)
src/server/graphics/nested/input_platform.h (+2/-0)
To merge this branch: bzr merge lp://staging/~andreas-pokorny/mir/wait-for-input-config-before-emitting-input-state
Reviewer Review Type Date Requested Status
Alexandros Frantzis (community) Needs Information
Mir CI Bot continuous-integration Approve
Review via email: mp+319328@code.staging.launchpad.net

Commit message

Wait for the arrival of the input config message before attempting to propagate input device state events

The former informs the nested server about the available devices, the latter is supposed to synchronize the host server input state with the focused client (here the nested server).

When launching the nested server there is a potential race between the initial input device state event and the input config. This problem is most visible when vt switching. Since the per-surface input fds have been removed, and all events are sent to the connection this race is harder to notice. Still the two messages may be sent by different threads in the server. This should be the last cause for the two prominent CI failures mentioned.

Description of the change

This race was found while working on the issue with stuck alt keys.

There are two messages involved. One indicates which input devices are available in the system. The one is treated as an Event the InputDeviceStateEvent, which provides the current input status like cursor position, pointer buttons, modifier state and pressed keys.

In the past a reordering of that was easy to trigger because it involved separate fds, now it requires a specific scheduling on the server, which happens frequently when trying to simulate pause/resume behavior.

Maybe the attached bugs have already been fixed or are just more rare due to other changes:
* removal of the per-surface fds
* updated surface input dispatcher logic in mir::input::SurfaceInputDispatcher::for_each)

Nonetheless the change here is related to the two linked problems.

To post a comment you must log in.
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4069
https://mir-jenkins.ubuntu.com/job/mir-ci/3114/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4177
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4264
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4254
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4254
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4254
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4204
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4204/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4204
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4204/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4204
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4204/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4204
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4204/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4204
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4204/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4204
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4204/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3114/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

The root cause of this issue is that the server-client information exchange does not enforce the required order. It would be preferable if we fixed the out of order issue in the server, rather than implementing what is essentially a workaround in the client. Is this feasible?

Also, is there the possibility of this race occurring after we already have some input devices (i.e. a devices is added after startup and the state event reaches the client before the input-config event)? If so, the workaround is not enough, since it is only applied when devices.empty() == true.

review: Needs Information
Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

> The root cause of this issue is that the server-client information exchange
> does not enforce the required order. It would be preferable if we fixed the
> out of order issue in the server, rather than implementing what is essentially
> a workaround in the client. Is this feasible?

The MP here seemed like the easy fix to get the follow up parts working.

> Also, is there the possibility of this race occurring after we already have
> some input devices (i.e. a devices is added after startup and the state event
> reaches the client before the input-config event)? If so, the workaround is
> not enough, since it is only applied when devices.empty() == true.

Sounds right..

I will try to change the way the input device changes are handled in the server... currently through an observer in the main thread .. vs the input thread..

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

So should this be work in progress now?

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches