QtMir

Merge lp://staging/~andreas-pokorny/qtmir/keep-mir-types-alive into lp://staging/qtmir

  1. keep-mir-types-alive
  2. Merge into trunk
Proposed by Andreas Pokorny
Status: Needs review
Proposed branch: lp://staging/~andreas-pokorny/qtmir/keep-mir-types-alive
Merge into: lp://staging/qtmir
Diff against target: 82 lines (+22/-14)
1 file modified
src/platforms/mirserver/mirserverhooks.cpp (+22/-14)
To merge this branch: bzr merge lp://staging/~andreas-pokorny/qtmir/keep-mir-types-alive
Reviewer Review Type Date Requested Status
Alan Griffiths Disapprove
Unity8 CI Bot (community) continuous-integration Approve
Review via email: mp+321427@code.staging.launchpad.net

Commit message

Store the relevant mir server parts as shared_ptr and reset before shutdown

This adjustment is required for mir 0.27 and newer.

Description of the change

This is needed for mir-0.27 or mir-1.0 and is currently part of the dnd integration silo.

Backstory: To resolve a race between main thread to session communication and input thread to window/session communication, the internal InputDeviceObserver are executed inside the input thread, but to keep thread usage intact for 3rd party users a wrapper was added that would ensure that all external InputDeviceObservers are executed in the main thread just like before. Since there are not external users inside mir server and since there is only a weak_ptr in qtmir, the wrapper is not kept alive. So no InputDeviceHub for qtmir.

This change will also work with mir-0.26

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

PASSED: Continuous integration, rev:626
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/644/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4774
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4802
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4625
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4625/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4625
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4625/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4625
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4625/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4625
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4625/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4625
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4625/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4625
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4625/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/644/rebuild

review: Approve (continuous-integration)
Reply
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I don't understand the explanation.

Why do *all* these objects need preserving after Mir is done with them but need releasing before Mir releases them.

review: Needs Information
Reply
Revision history for this message
Gerry Boland (gerboland) wrote :

I expect this branch would be a better solution for this issue:
https://code.launchpad.net/~gerboland/qtmir/small-hooks-change/+merge/321743
Andreas - can you have a look?

Reply
Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

> I expect this branch would be a better solution for this issue:
> https://code.launchpad.net/~gerboland/qtmir/small-hooks-change/+merge/321743
> Andreas - can you have a look?

This MP will show the same problem because nothing will keep the input device hub alive..

Reply
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Objects that need to be kept alive are typically owned in libmirserver not by downstream projects.

It seems to me that the right fix is in Mir. Mir should own the ExternalInputDeviceHub by attaching it to mir::DisplayServer::Private, or something that this owns.

review: Disapprove
Reply
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> Objects that need to be kept alive are typically owned in libmirserver not by
> downstream projects.
>
> It seems to me that the right fix is in Mir. Mir should own the
> ExternalInputDeviceHub by attaching it to mir::DisplayServer::Private, or
> something that this owns.

For example: lp:~alan-griffiths/mir/preserve-ExternalInputDeviceHub/+merge/321845

Reply

Unmerged revisions

626. By Andreas Pokorny

Store the relevant mir server parts as shared_ptr and reset before shutdown

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