Merge lp:~andreas-pokorny/mir/dispatchable-action-queue-and-fixes into lp:mir

[Andreas Pokorny]

Adds ActionQueue, a simple dispatchable utility

Additionally fixes a missing mir common symbol

This is another split-out from the input platform work. This includes a fix for mircommons symbols.map lacking the remove_watch symbols. A minor move and a ActionQueue which is used inside the new input platform handling code and inside stub input platform.

ActionQueue is a trivial std::function queue that implements the Dispatchable interface.

[lp:~andreas-pokorny/mir/dispatchable-action-queue-and-fixes]

2378. By Andreas Pokorny on 2015-03-10

merged lp:mir

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2378
http://jenkins.qa.ubuntu.com/job/mir-ci/3183/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1587
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1586
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1541
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1180
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1180/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1541
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1541/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4557
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/18715

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

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

suggest-fixing:
165 + {
could save some brackets for the conditional

100 + if (pipe(pipefds) < 0)
should be pipe2(pipefds, O_CLOEXEC))... also if it was O_CLOEXEC | O_NONBLOCK, we could call the write() function without the lock dance in enqueue()

147 + return FdEvent::readable|FdEvent::error;
126 + if (events&FdEvent::error)
spacing

review: Needs Fixing

[Chris Halse Rogers (raof)]

I think we've got enough usages of pipe2 now that we should pull mir_test/pipe.h into a utility class :).

Having said that, we really should be using an eventfd for these cases where we're just signalling. (I should go back and fix up my ones, too).

In addition to the nits picked up by Kevin:

46 + virtual FdEvents relevant_events() const override;

You've got a stray “virtual” there.

147 + return FdEvent::readable|FdEvent::error;

You don't need to specify that you handle FdEvent::error, although it's harmless.

review: Needs Fixing

[lp:~andreas-pokorny/mir/dispatchable-action-queue-and-fixes]

2379. By Andreas Pokorny on 2015-03-11

review findings addressed

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2379
http://jenkins.qa.ubuntu.com/job/mir-ci/3190/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1598
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1597
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1552
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1187
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1187/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1552
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1552/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4568
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/18748

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

review: Approve (continuous-integration)

[lp:~andreas-pokorny/mir/dispatchable-action-queue-and-fixes]

2380. By Andreas Pokorny on 2015-03-11

removed mock_dispatchable as a separate file, not needed

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2380
http://jenkins.qa.ubuntu.com/job/mir-ci/3194/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1605
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1604
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/1559
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1191
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1191/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1559
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/1559/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4575
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/18757

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

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

lgtm

review: Approve

[Chris Halse Rogers (raof)]

LGTM

review: Approve

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

[Alberto Aguirre (albaguirre)]

LGTM

review: Approve

[Alan Griffiths (alan-griffiths)]

OK

review: Approve

=== added file 'include/common/mir/dispatch/action_queue.h'
--- include/common/mir/dispatch/action_queue.h	1970-01-01 00:00:00 +0000
+++ include/common/mir/dispatch/action_queue.h	2015-03-11 12:29:17 +0000
@@ -0,0 +1,53 @@
+/*
+ * Copyright © 2015 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Andreas Pokorny <[e-mail]>
+ */
+
+#ifndef MIR_DISPATCH_ACTION_QUEUE_H_
+#define MIR_DISPATCH_ACTION_QUEUE_H_
+
+#include "mir/fd.h"
+#include "mir/dispatch/dispatchable.h"
+
+#include <list>
+#include <mutex>
+
+namespace mir
+{
+namespace dispatch
+{
+
+class ActionQueue : public Dispatchable
+{
+public:
+    ActionQueue();
+    Fd watch_fd() const override;
+
+    void enqueue(std::function<void()> const& action);
+
+    bool dispatch(FdEvents events) override;
+    FdEvents relevant_events() const override;
+private:
+    void consume();
+    void wake();
+    mir::Fd event_fd;
+    std::mutex list_lock;
+    std::list<std::function<void()>> actions;
+};
+}
+}
+
+#endif // MIR_DISPATCH_ACTION_QUEUE_H_

=== modified file 'src/common/dispatch/CMakeLists.txt'
--- src/common/dispatch/CMakeLists.txt	2015-02-18 05:27:28 +0000
+++ src/common/dispatch/CMakeLists.txt	2015-03-11 12:29:17 +0000
@@ -16,6 +16,7 @@
 
 list(
   APPEND MIR_COMMON_SOURCES
+  ${CMAKE_CURRENT_SOURCE_DIR}/action_queue.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/simple_dispatch_thread.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/multiplexing_dispatchable.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/utils.cpp

=== added file 'src/common/dispatch/action_queue.cpp'
--- src/common/dispatch/action_queue.cpp	1970-01-01 00:00:00 +0000
+++ src/common/dispatch/action_queue.cpp	2015-03-11 12:29:17 +0000
@@ -0,0 +1,87 @@
+/*
+ * Copyright © 2015 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Andreas Pokorny <[e-mail]>
+ */
+
+#include "mir/dispatch/action_queue.h"
+
+#include <boost/throw_exception.hpp>
+#include <sys/eventfd.h>
+
+mir::dispatch::ActionQueue::ActionQueue()
+    : event_fd{eventfd(0, EFD_CLOEXEC|EFD_NONBLOCK)}
+{
+    if (event_fd < 0)
+        BOOST_THROW_EXCEPTION((std::system_error{errno,
+                                                 std::system_category(),
+                                                 "Failed to create event fd for action queue"}));
+}
+
+mir::Fd mir::dispatch::ActionQueue::watch_fd() const
+{
+    return event_fd;
+}
+
+void mir::dispatch::ActionQueue::enqueue(std::function<void()> const& action)
+{
+    std::unique_lock<std::mutex> lock(list_lock);
+    actions.push_back(action);
+    wake();
+}
+
+bool mir::dispatch::ActionQueue::dispatch(FdEvents events)
+{
+    if (events&FdEvent::error)
+        return false;
+
+    std::list<std::function<void()>> actions_to_process;
+
+    {
+        std::unique_lock<std::mutex> lock(list_lock);
+        consume();
+        std::swap(actions_to_process, actions);
+    }
+
+    while(!actions_to_process.empty())
+    {
+        actions_to_process.front()();
+        actions_to_process.pop_front();
+    }
+    return true;
+}
+
+mir::dispatch::FdEvents mir::dispatch::ActionQueue::relevant_events() const
+{
+    return FdEvent::readable;
+}
+
+void mir::dispatch::ActionQueue::consume()
+{
+    uint64_t num_actions;
+    if (read(event_fd, &num_actions, sizeof num_actions) != sizeof num_actions)
+        BOOST_THROW_EXCEPTION((std::system_error{errno,
+                                                 std::system_category(),
+                                                 "Failed to consume action queue notification"}));
+}
+
+void mir::dispatch::ActionQueue::wake()
+{
+    uint64_t one_more{1};
+    if (write(event_fd, &one_more, sizeof one_more) != sizeof one_more)
+        BOOST_THROW_EXCEPTION((std::system_error{errno,
+                                                 std::system_category(),
+                                                 "Failed to wake action queue"}));
+}

=== modified file 'src/common/symbols.map'
--- src/common/symbols.map	2015-03-05 05:47:28 +0000
+++ src/common/symbols.map	2015-03-11 12:29:17 +0000
@@ -235,10 +235,21 @@
       mir::dispatch::MultiplexingDispatchable::dispatch*;
       mir::dispatch::MultiplexingDispatchable::relevant_events*;
       mir::dispatch::MultiplexingDispatchable::add_watch*;
+      mir::dispatch::MultiplexingDispatchable::remove_watch*;
 
       typeinfo?for?mir::dispatch::MultiplexingDispatchable;
       vtable?for?mir::dispatch::MultiplexingDispatchable;
 
+      mir::dispatch::ActionQueue::ActionQueue*;
+      mir::dispatch::ActionQueue::?ActionQueue*;
+      mir::dispatch::ActionQueue::watch_fd*;
+      mir::dispatch::ActionQueue::dispatch*;
+      mir::dispatch::ActionQueue::relevant_events*;
+      mir::dispatch::ActionQueue::enqueue*;
+
+      typeinfo?for?mir::dispatch::ActionQueue;
+      vtable?for?mir::dispatch::ActionQueue;
+
       mir::detail::RefCountedLibrary::operator*;
       mir::detail::RefCountedLibrary::?RefCountedLibrary*;
       mir::detail::RefCountedLibrary::RefCountedLibrary*;

=== modified file 'tests/unit-tests/dispatch/CMakeLists.txt'
--- tests/unit-tests/dispatch/CMakeLists.txt	2015-02-18 05:27:28 +0000
+++ tests/unit-tests/dispatch/CMakeLists.txt	2015-03-11 12:29:17 +0000
@@ -1,4 +1,5 @@
 list(APPEND UNIT_TEST_SOURCES
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_action_queue.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_simple_dispatch_thread.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_multiplexing_dispatchable.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_dispatch_utils.cpp

=== added file 'tests/unit-tests/dispatch/test_action_queue.cpp'
--- tests/unit-tests/dispatch/test_action_queue.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/dispatch/test_action_queue.cpp	2015-03-11 12:29:17 +0000
@@ -0,0 +1,74 @@
+/*
+ * Copyright © 2015 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Andreasd Pokorny <[e-mail]>
+ */
+
+#include "mir/dispatch/action_queue.h"
+
+#include "mir_test/fd_utils.h"
+
+#include <gtest/gtest.h>
+
+namespace mt = mir::test;
+namespace md = mir::dispatch;
+using namespace ::testing;
+
+TEST(ActionQueue, watch_fd_becomes_readable_on_enqueue)
+{
+    md::ActionQueue queue;
+
+    ASSERT_FALSE(mt::fd_is_readable(queue.watch_fd()));
+    queue.enqueue([]{});
+    ASSERT_TRUE(mt::fd_is_readable(queue.watch_fd()));
+}
+
+TEST(ActionQueue, executes_action_on_dispatch)
+{
+    md::ActionQueue queue;
+
+    auto action_exeuted = false;
+
+    queue.enqueue([&](){action_exeuted = true;});
+    ASSERT_FALSE(action_exeuted);
+    queue.dispatch(md::FdEvent::readable);
+    ASSERT_TRUE(action_exeuted);
+}
+
+TEST(ActionQueue, calls_nothing_on_error)
+{
+    md::ActionQueue queue;
+
+    auto action_exeuted = false;
+
+    queue.enqueue([&](){action_exeuted = true;});
+    queue.dispatch(md::FdEvent::error);
+    ASSERT_FALSE(action_exeuted);
+}
+
+TEST(ActionQueue, executes_action_only_once)
+{
+    md::ActionQueue queue;
+
+    auto count_executed = 0;
+
+    queue.enqueue([&](){++count_executed;});
+    queue.dispatch(md::FdEvent::readable);
+    EXPECT_THROW(queue.dispatch(md::FdEvent::readable),std::system_error);
+    EXPECT_THROW(queue.dispatch(md::FdEvent::readable),std::system_error);
+    EXPECT_THAT(count_executed, Eq(1));
+}
+
+