Mir

Merge lp://staging/~albaguirre/mir/fix-1481034 into lp://staging/mir

  1. fix-1481034
  2. Merge into development-branch
Proposed by Alberto Aguirre
Status: Merged
Approved by: Alberto Aguirre
Approved revision: no longer in the source branch.
Merged at revision: 2808
Proposed branch: lp://staging/~albaguirre/mir/fix-1481034
Merge into: lp://staging/mir
Diff against target: 44 lines (+9/-9)
1 file modified
src/server/input/default_input_manager.cpp (+9/-9)
To merge this branch: bzr merge lp://staging/~albaguirre/mir/fix-1481034
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alan Griffiths Approve
Review via email: mp+266781@code.staging.launchpad.net

Commit message

input: Avoid throwing exception while unwinding input thread

The input thread exception handler and the the enqueued action can race to set a std::promise during start.
Have the promise be owned and set by the starting action only instead.

Description of the change

input: Avoid throwing exception while unwinding input thread

The input thread exception handler and the the enqueued action can race to set a std::promise during start.
Have the promise be owned and set by the starting action only instead.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2806
http://jenkins.qa.ubuntu.com/job/mir-ci/4462/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3387
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/242
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/914
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3335
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/611
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/611/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3335
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3335/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6126
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22288

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4462/rebuild

review: Approve (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

Hah! I happen to have also run into and fixed this in the glib-mainloop-deadlock branch.

I think http://bazaar.launchpad.net/~raof/mir/fix-deadlock-in-glib-alarm/revision/2802 is a cleaner fix. There's no reason for the exception handler sent to ThreadedDispatcher to touch the started_promise at all, as ~promise() sets the exception iff a value hasn't been set and we're already going to mir::terminate_with_current_exception().

Revision history for this message
Alberto Aguirre (albaguirre) wrote :

@Chris, yeah I'll cherry pick your fix in here. Thanks!

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

24 +
25 + /* Emulate move-semantics for started_promise.
26 + *
27 + * We need the starting-lambda to own started_promise so that it is guaranteed that
28 + * started_future gets signalled; either by ->set_value in the success path or
29 + * by the destruction of started_promise generating a broken_promise exception.
30 + */
31 + started_promise.reset();

Couldn't we just do a move capture in the lambda expression?

   queue->enqueue([this, promise = std::move(started_promise)]()

review: Needs Information
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2807
http://jenkins.qa.ubuntu.com/job/mir-ci/4470/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3397
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/252
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/924
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3345
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/619
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/619/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3345
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3345/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6134
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22313

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4470/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

> Couldn't we just do a move capture in the lambda expression?
>
> queue->enqueue([this, promise = std::move(started_promise)]()

Yep.

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

LGTM

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2808
http://jenkins.qa.ubuntu.com/job/mir-ci/4472/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3399
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/254
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/926
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3347
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/621
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/621/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3347
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3347/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6136
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22317

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4472/rebuild

review: Approve (continuous-integration)

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