Merge lp:~alan-griffiths/mir/fix-1335747 into lp:mir

[Alan Griffiths]

tests: synchronize use of counters correctly

tests: synchronize use of counters correctly

[Alan Griffiths (alan-griffiths)]

To give an idea of the infrequence and scale of the problem here's some output from my investigation:

DEBUG run=188, i=938
DEBUG run=1306, i=662
DEBUG run=5562, i=1444
DEBUG run=12271, i=5
DEBUG run=12604, i=1810
DEBUG run=14656, i=2012
DEBUG run=23680, i=4
DEBUG run=31931, i=474
DEBUG run=35833, i=978
DEBUG run=39250, i=1176
DEBUG run=44260, i=1547
DEBUG run=47337, i=1214
DEBUG run=51397, i=1146
DEBUG run=58659, i=76
DEBUG run=58691, i=8700
DEBUG run=60875, i=1
DEBUG run=70940, i=2093
DEBUG run=75398, i=1137
DEBUG run=80812, i=4
DEBUG run=81348, i=7
DEBUG run=82227, i=94
DEBUG run=83219, i=1424
DEBUG run=90168, i=2145
DEBUG run=90484, i=908
DEBUG run=101722, i=224
DEBUG run=105501, i=1901

[Cemil Azizoglu (cemil-azizoglu)]

It'd be better if we synchronized properly. 10000 seems safe but who knows what would happen under a different/heavy load.

review: Disapprove

[Alan Griffiths (alan-griffiths)]

> It'd be better if we synchronized properly. 10000 seems safe but who knows
> what would happen under a different/heavy load.

"Disapprove" seems harsh - wasn't "Needs Fixing" enough?

Anyway, fixed.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1733
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2031/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/758
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/764
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/757
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/552
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/552/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/550
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/550/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2292
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2292/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1919
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9031

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2031/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

lgtm

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1734
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2036/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/764
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/770
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/763
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/557/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/555
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/555/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2305
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2305/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1923
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9040

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2036/rebuild

review: Needs Fixing (continuous-integration)

[Cemil Azizoglu (cemil-azizoglu)]

Looks good.

review: Approve

[Cemil Azizoglu (cemil-azizoglu)]

fails due to https://bugs.launchpad.net/mir/+bug/1336178. Reapproving.

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'tests/integration-tests/test_surfaceloop.cpp'
--- tests/integration-tests/test_surfaceloop.cpp	2014-06-12 13:23:29 +0000
+++ tests/integration-tests/test_surfaceloop.cpp	2014-06-30 16:11:37 +0000
@@ -28,7 +28,6 @@
 #include "mir_test_framework/basic_client_server_fixture.h"
 
 #include <thread>
-#include <atomic>
 #include <mutex>
 #include <condition_variable>
 #include <gmock/gmock.h>
@@ -153,17 +152,21 @@
 
         CountingStubBuffer()
         {
-            int created = buffers_created.load();
-            while (!buffers_created.compare_exchange_weak(created, created + 1)) std::this_thread::yield();
+            std::lock_guard<std::mutex> lock{buffers_mutex};
+            ++buffers_created;
+            buffers_cv.notify_one();
         }
         ~CountingStubBuffer()
         {
-            int destroyed = buffers_destroyed.load();
-            while (!buffers_destroyed.compare_exchange_weak(destroyed, destroyed + 1)) std::this_thread::yield();
+            std::lock_guard<std::mutex> lock{buffers_mutex};
+            ++buffers_destroyed;
+            buffers_cv.notify_one();
         }
 
-        static std::atomic<int> buffers_created;
-        static std::atomic<int> buffers_destroyed;
+        static std::mutex buffers_mutex;
+        static std::condition_variable buffers_cv;
+        static int buffers_created;
+        static int buffers_destroyed;
     };
 
     class StubGraphicBufferAllocator : public mtd::StubBufferAllocator
@@ -204,8 +207,10 @@
     std::shared_ptr<mg::Platform> platform;
 };
 
-std::atomic<int> BufferCounterConfig::CountingStubBuffer::buffers_created;
-std::atomic<int> BufferCounterConfig::CountingStubBuffer::buffers_destroyed;
+std::mutex BufferCounterConfig::CountingStubBuffer::buffers_mutex;
+std::condition_variable BufferCounterConfig::CountingStubBuffer::buffers_cv;
+int BufferCounterConfig::CountingStubBuffer::buffers_created;
+int BufferCounterConfig::CountingStubBuffer::buffers_destroyed;
 }
 
 struct SurfaceLoop : mtf::BasicClientServerFixture<BufferCounterConfig>
@@ -226,8 +231,15 @@
     {
         mtf::BasicClientServerFixture<BufferCounterConfig>::TearDown();
 
-        EXPECT_EQ(BufferCounterConfig::CountingStubBuffer::buffers_created.load(),
-                  BufferCounterConfig::CountingStubBuffer::buffers_destroyed.load());
+        using  Counter = BufferCounterConfig::CountingStubBuffer;
+
+        std::chrono::seconds const long_enough{4};
+        auto const all_buffers_destroyed =
+            []{ return Counter::buffers_created == Counter::buffers_destroyed; };
+        std::unique_lock<std::mutex> lock{Counter::buffers_mutex};
+
+        EXPECT_TRUE(
+            Counter::buffers_cv.wait_for(lock, long_enough, all_buffers_destroyed));
     }
 };