Mir

Merge lp:~kdub/mir/fix-1584784 into lp:mir

Proposed by Kevin DuBois on 2016-05-31
Status: Merged
Approved by: Kevin DuBois on 2016-06-04
Approved revision: 3537
Merged at revision: 3530
Proposed branch: lp:~kdub/mir/fix-1584784
Merge into: lp:mir
Diff against target: 511 lines (+191/-48)
8 files modified
src/client/buffer_stream.cpp (+20/-12)
src/client/buffer_vault.cpp (+42/-7)
src/client/buffer_vault.h (+11/-3)
src/client/mir_connection.h (+1/-1)
tests/acceptance-tests/test_client_surface_swap_buffers.cpp (+73/-1)
tests/integration-tests/test_buffer_scheduling.cpp (+1/-1)
tests/unit-tests/client/test_buffer_vault.cpp (+23/-23)
tests/unit-tests/client/test_client_buffer_stream.cpp (+20/-0)
To merge this branch: bzr merge lp:~kdub/mir/fix-1584784
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Approve on 2016-06-04
Cemil Azizoglu (community) Approve on 2016-06-02
Chris Halse Rogers 2016-05-31 Approve on 2016-06-01
Review via email: mp+296106@code.launchpad.net

Commit message

client: make sure that users of mir_buffer_stream_swap_buffers() never get blocked awaiting incoming buffers. The code this MP is fixing could wait on buffers if the producer produced quickly. (eg, sw Xmir with a small window, common in pocket desktop on frieza)

The signalling had to shift from the glue-code in bufferstream to the BufferVault, as that is the object that knows if there are buffers available. As mentioned in prior MPs, one of the things to be done once the old stuff is removed is to condense the glue-code in mir_client_buffer_stream.cpp and BufferVault (its currently useful for switching between NBUFFERS=0 / 3)

fixes: LP: #1584784

Description of the change

client: make sure that users of mir_buffer_stream_swap_buffers() never get blocked awaiting incoming buffers. The code this MP is fixing could wait on buffers if the producer produced quickly. (eg, sw Xmir with a small window, common in pocket desktop on frieza).

The signalling had to shift from the glue-code in bufferstream to the BufferVault, as that is the object that knows if there are buffers available. As mentioned in prior MPs, one of the things to be done once the old stuff is removed is to condense the glue-code in mir_client_buffer_stream.cpp and BufferVault (its currently useful for switching between NBUFFERS=0 / 3)

fixes: LP: #1584784

Note: The approved version of this fix will be ported to 0.23. I've ported the initial revision over to the 0.23 branch already to get the ball rolling on the silo rebuild.

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

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

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

review: Needs Fixing (continuous-integration)
Kevin DuBois (kdub) wrote :

other failures need addressing though.

Mir CI Bot (mir-ci-bot) wrote :

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

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

review: Needs Fixing (continuous-integration)
Mir CI Bot (mir-ci-bot) wrote :

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

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

review: Needs Fixing (continuous-integration)
Chris Halse Rogers (raof) wrote :

Looks sensible.

review: Approve
Daniel van Vugt (vanvugt) wrote :

Ideally std::map<int, Owner> should be a typedef, only defined once.

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3530
https://mir-jenkins.ubuntu.com/job/mir-ci/1063/
Executed test runs:

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

review: Needs Fixing (continuous-integration)
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3534
https://mir-jenkins.ubuntu.com/job/mir-ci/1064/
Executed test runs:

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

review: Needs Fixing (continuous-integration)
Mir CI Bot (mir-ci-bot) wrote :

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

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

review: Needs Fixing (continuous-integration)
Mir CI Bot (mir-ci-bot) wrote :

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

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

review: Approve (continuous-integration)
Kevin DuBois (kdub) wrote :

Changed the locking a bit from the "approve" from Chris (idea is still the same), but should probably get another review before landing.

Daniel van Vugt (vanvugt) wrote :

Oh will this also fix the "no new buffers" crash? (bug 1506358)

Kevin DuBois (kdub) wrote :

re: 1506358, probably not, most of the code change is in the new path, and seems that crash came through the old path. I'll comment in the bug too.

re: rev 3537, corrects a problem seen in porting this to the 0.23 series (and which is averted in the 0.24 series by other commits that are larger and don't need a full port to 0.23)

Mir CI Bot (mir-ci-bot) wrote :

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

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

review: Approve (continuous-integration)
Cemil Azizoglu (cemil-azizoglu) wrote :

This works for me.

review: Approve
Kevin DuBois (kdub) wrote :

Cancelled the autolanding, still working on testing the recent revs with full stack.

Mir CI Bot (mir-ci-bot) wrote :

FAILED: Autolanding.
Approved revid is not set in launchpad. This is most likely a launchpad issue and re-approve should fix it. There is also a chance (although a very small one) this is a permission problem of the ps-jenkins bot.
https://mir-jenkins.ubuntu.com/job/mir-autolanding/297/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1193
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/320/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1243
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1234
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1234
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1203
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1203/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1203
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1203/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/1203
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1203/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/1203
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1203/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1203
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1203/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/297/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1193
    FAILURE: https://mir-jenkins.ubuntu.com/job/generic-land-mp/320/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/321/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1243
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1234
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1234
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1203
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1203/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1203
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1203/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/1203
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1203/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/1203
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1203/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1203
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1203/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Kevin DuBois (kdub) wrote :

> Cancelled the autolanding, still working on testing the recent revs with full
> stack.

Exposed lp: #1588929, which after a good deal of investigation seems to be a pre-existing bug (and doesnt look related to this fix), during full stack testing, so MP is good to autoland.

Mir CI Bot (mir-ci-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/client/buffer_stream.cpp'
2--- src/client/buffer_stream.cpp 2016-06-02 05:33:50 +0000
3+++ src/client/buffer_stream.cpp 2016-06-02 13:02:31 +0000
4@@ -181,6 +181,14 @@
5 }
6 if (next_buffer_wait_handle.is_pending())
7 next_buffer_wait_handle.result_received();
8+
9+ while (!incoming_buffers.empty())
10+ {
11+ auto b = incoming_buffers.front();
12+ for (auto i = 0; i < b.fd_size(); i++)
13+ close(b.fd(i));
14+ incoming_buffers.pop();
15+ }
16 }
17
18 void set_size(geom::Size) override
19@@ -299,6 +307,8 @@
20 geom::Size size, MirPixelFormat format, int usage,
21 unsigned int initial_nbuffers) :
22 vault(factory, mirbuffer_factory, requests, surface_map, size, format, usage, initial_nbuffers),
23+ current(nullptr),
24+ future(vault.withdraw()),
25 size_(size)
26 {
27 }
28@@ -310,9 +320,9 @@
29 void advance_current_buffer(std::unique_lock<std::mutex>& lk)
30 {
31 lk.unlock();
32- auto buffer = vault.withdraw().get();
33+ auto c = future.get();
34 lk.lock();
35- current = buffer;
36+ current = c;
37 }
38
39 std::shared_ptr<mir::client::ClientBuffer> current_buffer() override
40@@ -336,18 +346,16 @@
41 std::unique_lock<std::mutex> lk(mutex);
42 if (!current)
43 advance_current_buffer(lk);
44+ auto c = current;
45+ current = nullptr;
46 lk.unlock();
47
48- vault.deposit(current);
49-
50- next_buffer_wait_handle.expect_result();
51- vault.wire_transfer_outbound(current);
52- next_buffer_wait_handle.result_received();
53-
54+ vault.deposit(c);
55+ auto wh = vault.wire_transfer_outbound(c, done);
56+ auto f = vault.withdraw();
57 lk.lock();
58- advance_current_buffer(lk);
59- done();
60- return &next_buffer_wait_handle;
61+ future = std::move(f);
62+ return wh;
63 }
64
65 void set_size(geom::Size size) override
66@@ -398,7 +406,7 @@
67 mcl::BufferVault vault;
68 std::mutex mutable mutex;
69 std::shared_ptr<mcl::Buffer> current{nullptr};
70- MirWaitHandle next_buffer_wait_handle;
71+ mir::client::NoTLSFuture<std::shared_ptr<mcl::Buffer>> future;
72 MirWaitHandle scale_wait_handle;
73 int current_swap_interval = 1;
74 geom::Size size_;
75
76=== modified file 'src/client/buffer_vault.cpp'
77--- src/client/buffer_vault.cpp 2016-06-02 05:33:50 +0000
78+++ src/client/buffer_vault.cpp 2016-06-02 13:02:31 +0000
79@@ -120,12 +120,9 @@
80 BOOST_THROW_EXCEPTION(std::logic_error("no buffer in map"));
81 }
82
83-mcl::NoTLSFuture<std::shared_ptr<mcl::Buffer>> mcl::BufferVault::withdraw()
84+
85+mcl::BufferVault::BufferMap::iterator mcl::BufferVault::available_buffer()
86 {
87- std::unique_lock<std::mutex> lk(mutex);
88- if (disconnected_)
89- BOOST_THROW_EXCEPTION(std::logic_error("server_disconnected"));
90- mcl::NoTLSPromise<std::shared_ptr<mcl::Buffer>> promise;
91 auto it = std::find_if(buffers.begin(), buffers.end(),
92 [this](std::pair<int, Owner> const& entry) {
93 return ((entry.second == Owner::Self) &&
94@@ -136,7 +133,16 @@
95 [this](std::pair<int, Owner> const& entry) {
96 return ((entry.second == Owner::Self) &&
97 (checked_buffer_from_map(entry.first)->size() == size)); });
98+ return it;
99+}
100
101+mcl::NoTLSFuture<std::shared_ptr<mcl::Buffer>> mcl::BufferVault::withdraw()
102+{
103+ std::unique_lock<std::mutex> lk(mutex);
104+ if (disconnected_)
105+ BOOST_THROW_EXCEPTION(std::logic_error("server_disconnected"));
106+ mcl::NoTLSPromise<std::shared_ptr<mcl::Buffer>> promise;
107+ auto it = available_buffer();
108 auto future = promise.get_future();
109 if (it != buffers.end())
110 {
111@@ -170,7 +176,8 @@
112 checked_buffer_from_map(it->first)->increment_age();
113 }
114
115-void mcl::BufferVault::wire_transfer_outbound(std::shared_ptr<mcl::Buffer> const& buffer)
116+MirWaitHandle* mcl::BufferVault::wire_transfer_outbound(
117+ std::shared_ptr<mcl::Buffer> const& buffer, std::function<void()> const& done)
118 {
119 std::unique_lock<std::mutex> lk(mutex);
120 auto it = buffers.find(buffer->rpc_id());
121@@ -181,6 +188,20 @@
122
123 buffer->submitted();
124 server_requests->submit_buffer(*buffer);
125+
126+ lk.lock();
127+ if (disconnected_)
128+ BOOST_THROW_EXCEPTION(std::logic_error("server_disconnected"));
129+ if (buffers.end() != available_buffer())
130+ {
131+ done();
132+ }
133+ else
134+ {
135+ next_buffer_wait_handle.expect_result();
136+ deferred_cb = done;
137+ }
138+ return &next_buffer_wait_handle;
139 }
140
141 void mcl::BufferVault::wire_transfer_inbound(int buffer_id)
142@@ -228,13 +249,27 @@
143 promises.front().set_value(buffer);
144 promises.pop_front();
145 }
146+
147+ trigger_callback(std::move(lk));
148 }
149
150 void mcl::BufferVault::disconnected()
151 {
152- std::lock_guard<std::mutex> lk(mutex);
153+ std::unique_lock<std::mutex> lk(mutex);
154 disconnected_ = true;
155 promises.clear();
156+ trigger_callback(std::move(lk));
157+}
158+
159+void mcl::BufferVault::trigger_callback(std::unique_lock<std::mutex> lk)
160+{
161+ if (auto cb = deferred_cb)
162+ {
163+ deferred_cb = {};
164+ lk.unlock();
165+ cb();
166+ next_buffer_wait_handle.result_received();
167+ }
168 }
169
170 void mcl::BufferVault::set_scale(float scale)
171
172=== modified file 'src/client/buffer_vault.h'
173--- src/client/buffer_vault.h 2016-06-02 05:33:50 +0000
174+++ src/client/buffer_vault.h 2016-06-02 13:02:31 +0000
175@@ -22,6 +22,7 @@
176 #include "mir/geometry/size.h"
177 #include "mir_toolkit/common.h"
178 #include "mir_toolkit/mir_native_buffer.h"
179+#include "mir_wait_handle.h"
180 #include <memory>
181 #include "no_tls_future-inl.h"
182 #include <deque>
183@@ -67,7 +68,8 @@
184 NoTLSFuture<std::shared_ptr<Buffer>> withdraw();
185 void deposit(std::shared_ptr<Buffer> const& buffer);
186 void wire_transfer_inbound(int buffer_id);
187- void wire_transfer_outbound(std::shared_ptr<Buffer> const& buffer);
188+ MirWaitHandle* wire_transfer_outbound(
189+ std::shared_ptr<Buffer> const& buffer, std::function<void()> const&);
190 void set_size(geometry::Size);
191 void disconnected();
192 void set_scale(float scale);
193@@ -75,6 +77,11 @@
194 void decrease_buffer_count();
195
196 private:
197+ enum class Owner;
198+ typedef std::map<int, Owner> BufferMap;
199+ BufferMap::iterator available_buffer();
200+ void trigger_callback(std::unique_lock<std::mutex> lk);
201+
202 void alloc_buffer(geometry::Size size, MirPixelFormat format, int usage);
203 void free_buffer(int free_id);
204 void realloc_buffer(int free_id, geometry::Size size, MirPixelFormat format, int usage);
205@@ -89,9 +96,8 @@
206 MirPixelFormat const format;
207 int const usage;
208
209- enum class Owner;
210 std::mutex mutex;
211- std::map<int, Owner> buffers;
212+ BufferMap buffers;
213 std::deque<NoTLSPromise<std::shared_ptr<Buffer>>> promises;
214 geometry::Size size;
215 bool disconnected_;
216@@ -99,6 +105,8 @@
217 size_t needed_buffer_count;
218 size_t const initial_buffer_count;
219 int last_received_id = 0;
220+ MirWaitHandle next_buffer_wait_handle;
221+ std::function<void()> deferred_cb;
222 };
223 }
224 }
225
226=== modified file 'src/client/mir_connection.h'
227--- src/client/mir_connection.h 2016-06-02 05:33:50 +0000
228+++ src/client/mir_connection.h 2016-06-02 13:02:31 +0000
229@@ -260,6 +260,7 @@
230
231 mutable std::mutex mutex; // Protects all members of *this (except release_wait_handles)
232
233+ std::shared_ptr<mir::client::ClientPlatform> platform;
234 std::shared_ptr<mir::client::ConnectionSurfaceMap> surface_map;
235 std::shared_ptr<mir::client::AsyncBufferFactory> buffer_factory;
236 std::shared_ptr<mir::client::rpc::MirBasicRpcChannel> const channel;
237@@ -280,7 +281,6 @@
238 int surface_error_id{-1};
239
240 std::shared_ptr<mir::client::ClientPlatformFactory> const client_platform_factory;
241- std::shared_ptr<mir::client::ClientPlatform> platform;
242 std::shared_ptr<mir::client::ClientBufferFactory> client_buffer_factory;
243 std::shared_ptr<EGLNativeDisplayType> native_display;
244
245
246=== modified file 'tests/acceptance-tests/test_client_surface_swap_buffers.cpp'
247--- tests/acceptance-tests/test_client_surface_swap_buffers.cpp 2016-01-29 08:18:22 +0000
248+++ tests/acceptance-tests/test_client_surface_swap_buffers.cpp 2016-06-02 13:02:31 +0000
249@@ -21,13 +21,17 @@
250 #include "mir_test_framework/connected_client_with_a_surface.h"
251 #include "mir/test/doubles/null_display_buffer_compositor_factory.h"
252 #include "mir/test/signal.h"
253+#include "mir/compositor/scene_element.h"
254+#include "mir/graphics/renderable.h"
255+#include "mir/graphics/cursor.h"
256
257 #include <gtest/gtest.h>
258
259 namespace mtf = mir_test_framework;
260 namespace mt = mir::test;
261 namespace mtd = mir::test::doubles;
262-
263+namespace mc = mir::compositor;
264+namespace mg = mir::graphics;
265 using namespace testing;
266
267 namespace
268@@ -50,6 +54,7 @@
269 ConnectedClientWithASurface::SetUp();
270 }
271 };
272+
273 }
274
275 TEST_F(SurfaceSwapBuffers, does_not_block_when_surface_is_not_composited)
276@@ -67,3 +72,70 @@
277 ASSERT_TRUE(buffers_swapped.wait_for(std::chrono::seconds{20}));
278 }
279 }
280+
281+namespace
282+{
283+struct BufferCollectingCompositorFactory : mc::DisplayBufferCompositorFactory
284+{
285+public:
286+ auto create_compositor_for(mg::DisplayBuffer&)
287+ -> std::unique_ptr<mc::DisplayBufferCompositor> override
288+ {
289+ struct CollectingDisplayBufferCompositor : mc::DisplayBufferCompositor
290+ {
291+ void composite(mc::SceneElementSequence&& seq)
292+ {
293+ for (auto& s : seq)
294+ buffers.insert(s->renderable()->buffer());
295+ }
296+ std::set<std::shared_ptr<mg::Buffer>> buffers;
297+ };
298+
299+ return std::make_unique<CollectingDisplayBufferCompositor>();
300+ }
301+};
302+
303+struct SwapBuffersDoesntBlockOnSubmission : mtf::ConnectedClientWithASurface
304+{
305+ unsigned int figure_out_nbuffers()
306+ {
307+ auto default_nbuffers = 3u;
308+ if (auto server_nbuffers_switch = getenv("MIR_SERVER_NBUFFERS"))
309+ {
310+ if (server_nbuffers_switch && atoi(server_nbuffers_switch) > 0)
311+ return atoi(server_nbuffers_switch);
312+ else
313+ {
314+ auto client_nbuffers_switch = getenv("MIR_CLIENT_NBUFFERS");
315+ if (client_nbuffers_switch)
316+ return atoi(client_nbuffers_switch);
317+ }
318+ }
319+ return default_nbuffers;
320+ }
321+ void SetUp() override
322+ {
323+ server.override_the_display_buffer_compositor_factory([]
324+ {
325+ return std::make_shared<BufferCollectingCompositorFactory>();
326+ });
327+
328+ ConnectedClientWithASurface::SetUp();
329+ server.the_cursor()->hide();
330+ }
331+
332+ void TearDown() override
333+ {
334+ ConnectedClientWithASurface::TearDown();
335+ }
336+
337+ unsigned int nbuffers = figure_out_nbuffers();
338+};
339+}
340+
341+//LP: #1584784
342+TEST_F(SwapBuffersDoesntBlockOnSubmission, can_swap_nbuffers_times_without_blocking)
343+{
344+ for (auto i = 0u; i != nbuffers; ++i)
345+ mir_buffer_stream_swap_buffers(mir_surface_get_buffer_stream(surface), nullptr, nullptr);
346+}
347
348=== modified file 'tests/integration-tests/test_buffer_scheduling.cpp'
349--- tests/integration-tests/test_buffer_scheduling.cpp 2016-06-02 05:33:50 +0000
350+++ tests/integration-tests/test_buffer_scheduling.cpp 2016-06-02 13:02:31 +0000
351@@ -465,7 +465,7 @@
352 {
353 auto buffer = vault.withdraw().get();
354 vault.deposit(buffer);
355- vault.wire_transfer_outbound(buffer);
356+ vault.wire_transfer_outbound(buffer, []{});
357 last_size_ = buffer->size();
358 entries.emplace_back(BufferEntry{mg::BufferID{(unsigned int)ipc->last_transferred_to_server()}, age, Access::unblocked});
359 available--;
360
361=== modified file 'tests/unit-tests/client/test_buffer_vault.cpp'
362--- tests/unit-tests/client/test_buffer_vault.cpp 2016-06-02 05:33:50 +0000
363+++ tests/unit-tests/client/test_buffer_vault.cpp 2016-06-02 13:02:31 +0000
364@@ -208,14 +208,14 @@
365 auto buffer = vault.withdraw().get();
366 EXPECT_CALL(mock_requests, submit_buffer(Ref(*buffer)));
367 vault.deposit(buffer);
368- vault.wire_transfer_outbound(buffer);
369+ vault.wire_transfer_outbound(buffer, []{});
370 }
371
372 TEST_F(StartedBufferVault, cant_transfer_if_not_in_acct)
373 {
374 auto buffer = vault.withdraw().get();
375 EXPECT_THROW({
376- vault.wire_transfer_outbound(buffer);
377+ vault.wire_transfer_outbound(buffer, []{});
378 }, std::logic_error);
379 }
380
381@@ -235,7 +235,7 @@
382 {
383 auto buffer = vault.withdraw().get();
384 vault.deposit(buffer);
385- vault.wire_transfer_outbound(buffer);
386+ vault.wire_transfer_outbound(buffer, []{});
387 EXPECT_THROW({
388 vault.deposit(buffer);
389 }, std::logic_error);
390@@ -247,7 +247,7 @@
391 vault->wire_transfer_inbound(package.buffer_id());
392 auto buffer = vault->withdraw().get();
393 vault->deposit(buffer);
394- vault->wire_transfer_outbound(buffer);
395+ vault->wire_transfer_outbound(buffer, []{});
396
397 vault->wire_transfer_inbound(package.buffer_id());
398 auto buffer2 = vault->withdraw().get();
399@@ -321,7 +321,7 @@
400
401 auto buffer = vault->withdraw().get();
402 vault->deposit(buffer);
403- vault->wire_transfer_outbound(buffer);
404+ vault->wire_transfer_outbound(buffer, []{});
405 }
406
407 TEST_F(StartedBufferVault, reallocates_incoming_buffers_of_incorrect_size_with_immediate_response)
408@@ -530,19 +530,19 @@
409 vault.wire_transfer_inbound(requested_buffer.buffer_id());
410 auto b = vault.withdraw().get();
411 vault.deposit(b);
412- vault.wire_transfer_outbound(b);
413-
414- b = vault.withdraw().get();
415- vault.deposit(b);
416- vault.wire_transfer_outbound(b);
417-
418- b = vault.withdraw().get();
419- vault.deposit(b);
420- vault.wire_transfer_outbound(b);
421-
422- b = vault.withdraw().get();
423- vault.deposit(b);
424- vault.wire_transfer_outbound(b);
425+ vault.wire_transfer_outbound(b, []{});
426+
427+ b = vault.withdraw().get();
428+ vault.deposit(b);
429+ vault.wire_transfer_outbound(b, []{});
430+
431+ b = vault.withdraw().get();
432+ vault.deposit(b);
433+ vault.wire_transfer_outbound(b, []{});
434+
435+ b = vault.withdraw().get();
436+ vault.deposit(b);
437+ vault.wire_transfer_outbound(b, []{});
438
439 vault.decrease_buffer_count();
440 vault.wire_transfer_inbound(package.buffer_id());
441@@ -555,12 +555,12 @@
442 {
443 auto buffer = vault.withdraw().get();
444 vault.deposit(buffer);
445- vault.wire_transfer_outbound(buffer);
446+ vault.wire_transfer_outbound(buffer, []{});
447 auto first_id = buffer->rpc_id();
448
449 buffer = vault.withdraw().get();
450 vault.deposit(buffer);
451- vault.wire_transfer_outbound(buffer);
452+ vault.wire_transfer_outbound(buffer, []{});
453 auto second_id = buffer->rpc_id();
454
455 vault.wire_transfer_inbound(second_id);
456@@ -593,7 +593,7 @@
457 Mock::VerifyAndClearExpectations(&mock_requests);
458
459 vault.deposit(buffer);
460- vault.wire_transfer_outbound(buffer);
461+ vault.wire_transfer_outbound(buffer, []{});
462
463 EXPECT_CALL(mock_requests, free_buffer(_))
464 .Times(1);
465@@ -610,7 +610,7 @@
466 vault.set_size(new_size);
467 Mock::VerifyAndClearExpectations(&mock_requests);
468
469- vault.wire_transfer_outbound(buffer);
470+ vault.wire_transfer_outbound(buffer, []{});
471 EXPECT_CALL(mock_requests, free_buffer(_))
472 .Times(1);
473 vault.wire_transfer_inbound(buffer->rpc_id());
474@@ -642,7 +642,7 @@
475 auto buffer = vault.withdraw().get();
476 vault.deposit(buffer);
477 buffer->received();
478- vault.wire_transfer_outbound(buffer);
479+ vault.wire_transfer_outbound(buffer, []{});
480 vault.wire_transfer_inbound(buffer->rpc_id());
481 }
482 }
483
484=== modified file 'tests/unit-tests/client/test_client_buffer_stream.cpp'
485--- tests/unit-tests/client/test_client_buffer_stream.cpp 2016-06-02 05:33:50 +0000
486+++ tests/unit-tests/client/test_client_buffer_stream.cpp 2016-06-02 13:02:31 +0000
487@@ -789,4 +789,24 @@
488 EXPECT_THAT(stream.get_current_buffer_id(), Eq(10));
489 }
490
491+//LP: #1584784
492+TEST_P(ClientBufferStream, can_cycle_through_available_buffers_without_waiting)
493+{
494+ ON_CALL(mock_protobuf_server, submit_buffer(_,_,_))
495+ .WillByDefault(mtd::RunProtobufClosure());
496+ mcl::BufferStream bs{
497+ nullptr, wait_handle, mock_protobuf_server,
498+ std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
499+ map, factory,
500+ response, perf_report, "", size, nbuffers};
501+ service_requests_for(bs, mock_protobuf_server.alloc_count);
502+
503+ auto count = 0;
504+ for(auto i = 0u; i < mock_protobuf_server.alloc_count; i++)
505+ {
506+ bs.get_current_buffer();
507+ bs.next_buffer([&count]{ count++;});
508+ }
509+}
510+
511 INSTANTIATE_TEST_CASE_P(BufferSemanticsMode, ClientBufferStream, Bool());

Subscribers

People subscribed via source and target branches