Mir

Merge lp:~vanvugt/mir/fix-1653658 into lp:mir

Proposed by Daniel van Vugt
Status: Superseded
Proposed branch: lp:~vanvugt/mir/fix-1653658
Merge into: lp:mir
Diff against target: 312 lines (+110/-11)
11 files modified
src/client/mir_connection.cpp (+8/-3)
src/client/rpc/mir_basic_rpc_channel.cpp (+23/-8)
src/client/rpc/mir_basic_rpc_channel.h (+8/-0)
src/client/rpc/mir_protobuf_rpc_channel.cpp (+24/-0)
src/client/rpc/mir_protobuf_rpc_channel.h (+3/-0)
tests/acceptance-tests/test_client_library.cpp (+14/-0)
tests/include/mir/test/doubles/stub_connection_configuration.h (+6/-0)
tests/unit-tests/client/test_client_mir_surface.cpp (+6/-0)
tests/unit-tests/client/test_mir_connection.cpp (+9/-0)
tests/unit-tests/client/test_mir_render_surface.cpp (+3/-0)
tests/unit-tests/platforms/android/client/test_android_client_platform.cpp (+6/-0)
To merge this branch: bzr merge lp:~vanvugt/mir/fix-1653658
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Approve
Daniel van Vugt Abstain
Chris Halse Rogers Approve
Review via email: mp+314055@code.launchpad.net

Commit message

Fix race between MirConnection's destructor and its RPC callbacks.
(LP: #1653658)

This ensures RPC callbacks never occur (so never touch the MirConnection) after destruction of the MirConnection has commenced.

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

FAILED: Continuous integration, rev:3904
https://mir-jenkins.ubuntu.com/job/mir-ci/2514/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3274/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3341
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3333
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3333
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3333
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3303
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3303/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/3303
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3303/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3303
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3303/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3303/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3303/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/3303
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3303/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/3303
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3303/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3905
https://mir-jenkins.ubuntu.com/job/mir-ci/2525/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3287/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3354
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3346
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3346
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3346
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3316
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3316/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/3316
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3316/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3316
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3316/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3316/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3316/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/3316
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3316/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/3316
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3316/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Weird. I just ran those tests on my own krillin and they pass just fine.

Then again those above crashes on krillin are not new. I had seen them several times before this branch ever existed.

Revision history for this message
Chris Halse Rogers (raof) wrote :

Looks good.

Nonblocking nits:
We should fix it so that MirConnection::channel is always valid. If some tests don't provide a valid channel, we should fix the tests.

It's a bit weird that complete_response searches through pending_calls twice, but I can see why you've done it that way. Doing it better probably requires deeper changes.

review: Approve
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3906
https://mir-jenkins.ubuntu.com/job/mir-ci/2527/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3290/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3357
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3349
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3349
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3349
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3319
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3319/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/3319
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3319/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3319/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3319/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3319/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/3319
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3319/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/3319
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3319/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Hmm the only unexpected failure is still krillin:

05:52:44 [2017-01-05 05:52:44.153385] server_example_test_client.cpp: Terminating client
05:52:44 [2017-01-05 05:52:44.153647] mirserver: Stopping
05:52:44 I: [FAILED] mir_demo_client_animated_cursor
05:52:44 [timestamp] End : mir_demo_client_animated_cursor 2017-01-05T05:52:44+0000

But it works on my krillin :/

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3907
https://mir-jenkins.ubuntu.com/job/mir-ci/2528/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3292/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3359
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3351
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3351
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3351
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3321
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3321/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/3321
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3321/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3321
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3321/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3321/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3321/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/3321
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3321/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3321/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Points for consistency. All tests pass on krillin but some are leaving behind core files, or something, which makes it a failure...

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

OK, two issues found so far:
  (1) My krillin can't generate core files even with ulimit set. Had to also set /proc/sys/kernel/core_pattern (see bug 1654479)
  (2) Bug 1654478 might be related.

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3908
https://mir-jenkins.ubuntu.com/job/mir-ci/2536/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3301/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3368
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3360
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3360
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3360
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3330/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3330
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3330/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3330
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3330/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3330/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3330/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/3330
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3330/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/3330
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3330/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Finally. I think the clang amd64 run is showing the bug here.

Revision history for this message
Daniel van Vugt (vanvugt) :
review: Needs Fixing
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3909
https://mir-jenkins.ubuntu.com/job/mir-ci/2538/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3303/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3370
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3362
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3362
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3362
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3332/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3332
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3332/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3332
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3332/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3332/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3332/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/3332
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3332/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/3332
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3332/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) :
review: Abstain
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3911
https://mir-jenkins.ubuntu.com/job/mir-ci/2539/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/3304
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3371
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3363
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3363
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3363
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3333
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3333/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/3333
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3333/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3333
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3333/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/3333
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3333/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/3333
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3333/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/3333
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3333/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp 2017-01-04 14:53:35 +0000
+++ src/client/mir_connection.cpp 2017-01-06 05:34:08 +0000
@@ -309,9 +309,11 @@
309309
310MirConnection::~MirConnection() noexcept310MirConnection::~MirConnection() noexcept
311{311{
312 // We don't die while if are pending callbacks (as they touch this).312 if (channel) // some tests don't have one
313 // But, if after 500ms we don't get a call, assume it won't happen.313 {
314 connect_wait_handle.wait_for_pending(std::chrono::milliseconds(500));314 channel->discard_future_calls();
315 channel->wait_for_outstanding_calls();
316 }
315317
316 std::lock_guard<decltype(mutex)> lock(mutex);318 std::lock_guard<decltype(mutex)> lock(mutex);
317 surface_map.reset();319 surface_map.reset();
@@ -643,6 +645,9 @@
643 server.disconnect(ignored.get(), ignored.get(),645 server.disconnect(ignored.get(), ignored.get(),
644 google::protobuf::NewCallback(this, &MirConnection::done_disconnect));646 google::protobuf::NewCallback(this, &MirConnection::done_disconnect));
645647
648 if (channel)
649 channel->discard_future_calls();
650
646 return &disconnect_wait_handle;651 return &disconnect_wait_handle;
647}652}
648653
649654
=== modified file 'src/client/rpc/mir_basic_rpc_channel.cpp'
--- src/client/rpc/mir_basic_rpc_channel.cpp 2016-07-18 07:38:38 +0000
+++ src/client/rpc/mir_basic_rpc_channel.cpp 2017-01-06 05:34:08 +0000
@@ -83,9 +83,10 @@
83 auto call = pending_calls.find(result.id());83 auto call = pending_calls.find(result.id());
84 if (call != pending_calls.end())84 if (call != pending_calls.end())
85 {85 {
86 completion = call->second;86 completion = std::move(call->second);
87 pending_calls.erase(call);87 pending_calls.erase(call);
88 }88 }
89 ++running_callbacks;
89 }90 }
9091
91 if (!completion.complete)92 if (!completion.complete)
@@ -97,29 +98,43 @@
97 rpc_report->complete_response(result);98 rpc_report->complete_response(result);
98 completion.complete->Run();99 completion.complete->Run();
99 }100 }
101
102 {
103 std::unique_lock<std::mutex> lock(mutex);
104 --running_callbacks;
105 pending_calls_shrank.notify_all();
106 }
100}107}
101108
102void mclrd::PendingCallCache::force_completion()109void mclrd::PendingCallCache::force_completion()
103{110{
104 std::unique_lock<std::mutex> lock(mutex);111 std::unique_lock<std::mutex> lock(mutex);
105 for (auto& call : pending_calls)112 ++running_callbacks;
113 while (!pending_calls.empty())
106 {114 {
107 auto& completion = call.second;115 auto i = pending_calls.begin();
116 auto completion = std::move(i->second);
117 pending_calls.erase(i);
118 lock.unlock();
108 completion.complete->Run();119 completion.complete->Run();
120 lock.lock();
109 }121 }
110122 --running_callbacks;
111 pending_calls.erase(pending_calls.begin(), pending_calls.end());123 pending_calls_shrank.notify_all();
112}124}
113125
114
115
116bool mclrd::PendingCallCache::empty() const126bool mclrd::PendingCallCache::empty() const
117{127{
118 std::unique_lock<std::mutex> lock(mutex);128 std::unique_lock<std::mutex> lock(mutex);
119 return pending_calls.empty();129 return pending_calls.empty();
120}130}
121131
122132void mclrd::PendingCallCache::wait_till_complete() const
133{
134 std::unique_lock<std::mutex> lock(mutex);
135 while (running_callbacks || !pending_calls.empty())
136 pending_calls_shrank.wait(lock);
137}
123138
124mclr::MirBasicRpcChannel::MirBasicRpcChannel() :139mclr::MirBasicRpcChannel::MirBasicRpcChannel() :
125 next_message_id(0),140 next_message_id(0),
126141
=== modified file 'src/client/rpc/mir_basic_rpc_channel.h'
--- src/client/rpc/mir_basic_rpc_channel.h 2016-01-29 08:18:22 +0000
+++ src/client/rpc/mir_basic_rpc_channel.h 2017-01-06 05:34:08 +0000
@@ -24,6 +24,7 @@
24#include <mutex>24#include <mutex>
25#include <atomic>25#include <atomic>
26#include <vector>26#include <vector>
27#include <condition_variable>
2728
28namespace google29namespace google
29{30{
@@ -77,6 +78,8 @@
7778
78 bool empty() const;79 bool empty() const;
7980
81 void wait_till_complete() const;
82
80private:83private:
8184
82 struct PendingCall85 struct PendingCall
@@ -94,6 +97,8 @@
94 };97 };
9598
96 std::mutex mutable mutex;99 std::mutex mutable mutex;
100 std::condition_variable mutable pending_calls_shrank;
101 int running_callbacks = 0;
97 std::map<int, PendingCall> pending_calls;102 std::map<int, PendingCall> pending_calls;
98 std::shared_ptr<RpcReport> const rpc_report;103 std::shared_ptr<RpcReport> const rpc_report;
99};104};
@@ -110,6 +115,9 @@
110 google::protobuf::MessageLite* response,115 google::protobuf::MessageLite* response,
111 google::protobuf::Closure* complete) = 0;116 google::protobuf::Closure* complete) = 0;
112117
118 virtual void discard_future_calls() = 0;
119 virtual void wait_for_outstanding_calls() = 0;
120
113protected:121protected:
114 MirBasicRpcChannel();122 MirBasicRpcChannel();
115 mir::protobuf::wire::Invocation invocation_for(123 mir::protobuf::wire::Invocation invocation_for(
116124
=== modified file 'src/client/rpc/mir_protobuf_rpc_channel.cpp'
--- src/client/rpc/mir_protobuf_rpc_channel.cpp 2016-12-23 07:58:44 +0000
+++ src/client/rpc/mir_protobuf_rpc_channel.cpp 2017-01-06 05:34:08 +0000
@@ -197,6 +197,20 @@
197 google::protobuf::MessageLite* response,197 google::protobuf::MessageLite* response,
198 google::protobuf::Closure* complete)198 google::protobuf::Closure* complete)
199{199{
200 if (discard)
201 {
202 /*
203 * Until recently we had no explicit plan for what to do in this case.
204 * Callbacks would race with destruction of the MirConnection and either
205 * succeed, deadlock, crash or corrupt memory. However the one apparent
206 * intent in the old plan was that we close all closures so that the
207 * user doesn't leak any memory. So do that...
208 */
209 if (complete)
210 complete->Run();
211 return;
212 }
213
200 // Only send message when details saved for handling response214 // Only send message when details saved for handling response
201 std::vector<mir::Fd> fds;215 std::vector<mir::Fd> fds;
202 if (parameters->GetTypeName() == "mir.protobuf.BufferRequest")216 if (parameters->GetTypeName() == "mir.protobuf.BufferRequest")
@@ -228,6 +242,16 @@
228 send_message(invocation, invocation, fds);242 send_message(invocation, invocation, fds);
229}243}
230244
245void mclr::MirProtobufRpcChannel::discard_future_calls()
246{
247 discard = true;
248}
249
250void mclr::MirProtobufRpcChannel::wait_for_outstanding_calls()
251{
252 pending_calls.wait_till_complete();
253}
254
231void mclr::MirProtobufRpcChannel::send_message(255void mclr::MirProtobufRpcChannel::send_message(
232 mir::protobuf::wire::Invocation const& body,256 mir::protobuf::wire::Invocation const& body,
233 mir::protobuf::wire::Invocation const& invocation,257 mir::protobuf::wire::Invocation const& invocation,
234258
=== modified file 'src/client/rpc/mir_protobuf_rpc_channel.h'
--- src/client/rpc/mir_protobuf_rpc_channel.h 2016-12-13 06:00:32 +0000
+++ src/client/rpc/mir_protobuf_rpc_channel.h 2017-01-06 05:34:08 +0000
@@ -98,10 +98,13 @@
98 google::protobuf::MessageLite const* parameters,98 google::protobuf::MessageLite const* parameters,
99 google::protobuf::MessageLite* response,99 google::protobuf::MessageLite* response,
100 google::protobuf::Closure* complete) override;100 google::protobuf::Closure* complete) override;
101 void discard_future_calls() override;
102 void wait_for_outstanding_calls() override;
101103
102private:104private:
103 std::shared_ptr<RpcReport> const rpc_report;105 std::shared_ptr<RpcReport> const rpc_report;
104 detail::PendingCallCache pending_calls;106 detail::PendingCallCache pending_calls;
107 std::atomic_bool discard{false};
105108
106 static constexpr size_t size_of_header = 2;109 static constexpr size_t size_of_header = 2;
107 detail::SendBuffer header_bytes;110 detail::SendBuffer header_bytes;
108111
=== modified file 'tests/acceptance-tests/test_client_library.cpp'
--- tests/acceptance-tests/test_client_library.cpp 2016-12-17 00:52:45 +0000
+++ tests/acceptance-tests/test_client_library.cpp 2017-01-06 05:34:08 +0000
@@ -246,6 +246,20 @@
246 mir_connection_release(connection);246 mir_connection_release(connection);
247}247}
248248
249TEST_F(ClientLibrary, shutdown_race_is_resolved_safely)
250{ // An attempt at a regression test for race LP: #1653658
251 connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
252 auto const spec = mir_connection_create_spec_for_normal_surface(
253 connection, 123, 456, mir_pixel_format_abgr_8888);
254 surface = mir_surface_create_sync(spec);
255 mir_surface_spec_release(spec);
256
257 EXPECT_THAT(surface, IsValid());
258
259 mir_surface_release(surface, [](MirSurface*, void*){ sleep(1); }, NULL);
260 mir_connection_release(connection);
261}
262
249TEST_F(ClientLibrary, can_set_surface_state)263TEST_F(ClientLibrary, can_set_surface_state)
250{264{
251 connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);265 connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
252266
=== modified file 'tests/include/mir/test/doubles/stub_connection_configuration.h'
--- tests/include/mir/test/doubles/stub_connection_configuration.h 2016-12-23 05:15:02 +0000
+++ tests/include/mir/test/doubles/stub_connection_configuration.h 2017-01-06 05:34:08 +0000
@@ -42,6 +42,12 @@
42 channel_call_count++;42 channel_call_count++;
43 c->Run();43 c->Run();
44 }44 }
45 void discard_future_calls() override
46 {
47 }
48 void wait_for_outstanding_calls() override
49 {
50 }
45 mir::Fd watch_fd() const51 mir::Fd watch_fd() const
46 {52 {
47 int fd[2];53 int fd[2];
4854
=== modified file 'tests/unit-tests/client/test_client_mir_surface.cpp'
--- tests/unit-tests/client/test_client_mir_surface.cpp 2016-12-23 07:58:44 +0000
+++ tests/unit-tests/client/test_client_mir_surface.cpp 2017-01-06 05:34:08 +0000
@@ -261,6 +261,12 @@
261 {261 {
262 delete closure;262 delete closure;
263 }263 }
264 void discard_future_calls() override
265 {
266 }
267 void wait_for_outstanding_calls() override
268 {
269 }
264};270};
265271
266void null_connected_callback(MirConnection* /*connection*/, void * /*client_context*/)272void null_connected_callback(MirConnection* /*connection*/, void * /*client_context*/)
267273
=== modified file 'tests/unit-tests/client/test_mir_connection.cpp'
--- tests/unit-tests/client/test_mir_connection.cpp 2016-12-23 07:58:44 +0000
+++ tests/unit-tests/client/test_mir_connection.cpp 2017-01-06 05:34:08 +0000
@@ -156,6 +156,9 @@
156 complete->Run();156 complete->Run();
157 }157 }
158158
159 MOCK_METHOD0(discard_future_calls, void());
160 MOCK_METHOD0(wait_for_outstanding_calls, void());
161
159 MOCK_METHOD2(on_buffer_stream_create, void(mp::BufferStream&, google::protobuf::Closure* complete));162 MOCK_METHOD2(on_buffer_stream_create, void(mp::BufferStream&, google::protobuf::Closure* complete));
160 MOCK_METHOD2(connect, void(mp::ConnectParameters const*,mp::Connection*));163 MOCK_METHOD2(connect, void(mp::ConnectParameters const*,mp::Connection*));
161 MOCK_METHOD1(configure_display_sent, void(mp::DisplayConfiguration const*));164 MOCK_METHOD1(configure_display_sent, void(mp::DisplayConfiguration const*));
@@ -716,6 +719,12 @@
716 {719 {
717 delete closure;720 delete closure;
718 }721 }
722 void discard_future_calls() override
723 {
724 }
725 void wait_for_outstanding_calls() override
726 {
727 }
719 };728 };
720 TestConnectionConfiguration conf{729 TestConnectionConfiguration conf{
721 mock_platform, std::make_shared<NiceMock<FakeRpcChannel>>(), mock_buffer_allocator };730 mock_platform, std::make_shared<NiceMock<FakeRpcChannel>>(), mock_buffer_allocator };
722731
=== modified file 'tests/unit-tests/client/test_mir_render_surface.cpp'
--- tests/unit-tests/client/test_mir_render_surface.cpp 2016-12-13 06:00:32 +0000
+++ tests/unit-tests/client/test_mir_render_surface.cpp 2017-01-06 05:34:08 +0000
@@ -87,6 +87,9 @@
87 complete->Run();87 complete->Run();
88 }88 }
8989
90 MOCK_METHOD0(discard_future_calls, void());
91 MOCK_METHOD0(wait_for_outstanding_calls, void());
92
90 MOCK_METHOD2(on_buffer_stream_create, void(mp::BufferStream&, google::protobuf::Closure* complete));93 MOCK_METHOD2(on_buffer_stream_create, void(mp::BufferStream&, google::protobuf::Closure* complete));
9194
92 MOCK_CONST_METHOD0(watch_fd, mir::Fd());95 MOCK_CONST_METHOD0(watch_fd, mir::Fd());
9396
=== modified file 'tests/unit-tests/platforms/android/client/test_android_client_platform.cpp'
--- tests/unit-tests/platforms/android/client/test_android_client_platform.cpp 2016-12-23 07:58:44 +0000
+++ tests/unit-tests/platforms/android/client/test_android_client_platform.cpp 2017-01-06 05:34:08 +0000
@@ -204,6 +204,12 @@
204 channel_call_count++;204 channel_call_count++;
205 c->Run();205 c->Run();
206 }206 }
207 void discard_future_calls() override
208 {
209 }
210 void wait_for_outstanding_calls() override
211 {
212 }
207 mir::Fd watch_fd() const213 mir::Fd watch_fd() const
208 {214 {
209 int fd[2];215 int fd[2];

Subscribers

People subscribed via source and target branches