Merge lp:~afrantzis/mir/fix-1354446 into lp:mir

[Alexandros Frantzis]

tests: Fix race in InputTestingServerConfiguration

We need to wait for the 'input_injection_thread' variable to be
assigned to before starting. Otherwise we may end up calling
on_exit() before assignment and try to join an unjoinable thread.

tests: Fix race in InputTestingServerConfiguration

We need to wait for the 'input_injection_thread' variable to be
assigned to before starting. Otherwise we may end up calling
on_exit() before assignment and try to join an unjoinable thread.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1832
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2359/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1271
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1277
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1260
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/881
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/881/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/882
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/882/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/195
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/195/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2358
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11230

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

LGTM

review: Approve

[Kevin DuBois (kdub)]

review: Approve

[Daniel van Vugt (vanvugt)]

Should we be concerned that on_exit is being called before on_start? Because that appears to be the only way for the bug to occur.

review: Needs Information

[Daniel van Vugt (vanvugt)]

Providing both functions are executed from the same thread then you could just say:

void mtf::InputTestingServerConfiguration::on_start()
{
    if (exited) return;
    input_injection_thread =
        std::thread(std::mem_fn(&mtf::InputTestingServerConfiguration::inject_input), this);
}

void mtf::InputTestingServerConfiguration::on_exit()
{
    if (input_injection_thread.joinable())
        input_injection_thread.join();
    exited = true;
}

[Alexandros Frantzis (afrantzis)]

> Should we be concerned that on_exit is being called before on_start? Because\
> Providing both functions are executed from the same thread then you could just say

on_exit() is called from a different thread during the call to on_start() because the the input injection thread may run first and cause the test to finish before we have left on_start().

[Daniel van Vugt (vanvugt)]

Bugger. That's somewhat unexpected but does convince me then this is the right fix.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/971/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1291
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1297
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1279/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/355
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/355/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/355
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/355/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/215/console

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/973/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1301
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1307
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1289/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/357
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/357/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/357
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/357/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/225/console

review: Needs Fixing (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Building remotely on cyclops-node07 in workspace /home/ubuntu/jenkins/workspace/mir-mediumtests-builder-utopic-armhf
java.io.IOException: Failed to mkdirs: /home/ubuntu/jenkins/workspace/mir-mediumtests-builder-utopic-armhf

It seems cyclops-node07 is stuck in a bad state.

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/976/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1310
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1316
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1298/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/360
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/360/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/360
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/360/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/235
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/235/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2389/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11406

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'tests/mir_test_framework/input_testing_server_options.cpp'
--- tests/mir_test_framework/input_testing_server_options.cpp	2014-08-06 01:40:20 +0000
+++ tests/mir_test_framework/input_testing_server_options.cpp	2014-08-11 08:51:07 +0000
@@ -27,6 +27,7 @@
 
 #include "mir_test/fake_event_hub.h"
 #include "mir_test/fake_event_hub_input_configuration.h"
+#include "mir_test/wait_condition.h"
 
 #include <boost/throw_exception.hpp>
 
@@ -34,6 +35,7 @@
 #include <stdexcept>
 
 namespace mtf = mir_test_framework;
+namespace mt = mir::test;
 
 namespace mf = mir::frontend;
 namespace mg = mir::graphics;
@@ -55,7 +57,21 @@
 
 void mtf::InputTestingServerConfiguration::on_start()
 {
-    input_injection_thread = std::thread(std::mem_fn(&mtf::InputTestingServerConfiguration::inject_input), this);
+    auto const start_input_injection = std::make_shared<mt::WaitCondition>();
+
+    input_injection_thread = std::thread{
+        [this, start_input_injection]
+        {
+            // We need to wait for the 'input_injection_thread' variable to be
+            // assigned to before starting. Otherwise we may end up calling
+            // on_exit() before assignment and try to join an unjoinable thread.
+            start_input_injection->wait_for_at_most_seconds(3);
+            if (!start_input_injection->woken())
+                BOOST_THROW_EXCEPTION(std::runtime_error("Input injection thread start signal timed out"));
+            inject_input();
+        }};
+
+    start_input_injection->wake_up_everyone();
 }
 
 void mtf::InputTestingServerConfiguration::on_exit()