Mir

Merge lp:~andreas-pokorny/mir/split-main-loop-and-fix-races into lp:mir

split-main-loop-and-fix-races
Merge into development-branch

Proposed by Andreas Pokorny on 2014-05-22

Status:	Work in progress
Proposed branch:	lp:~andreas-pokorny/mir/split-main-loop-and-fix-races
Merge into:	lp:mir
Diff against target:	3020 lines (+1675/-617) 31 files modified include/platform/mir/graphics/event_handler_register.h (+3/-0) include/server/mir/default_server_configuration.h (+5/-0) include/server/mir/loop.h (+37/-0) include/server/mir/main_loop.h (+1/-2) include/server/mir/server_configuration.h (+2/-0) include/server/mir/time/timer_loop.h (+37/-0) include/test/mir_test_doubles/mock_event_handler_register.h (+53/-0) include/test/mir_test_doubles/mock_loop.h (+44/-0) include/test/mir_test_doubles/mock_server_action_queue.h (+44/-0) src/platform/graphics/mesa/display.cpp (+1/-0) src/server/CMakeLists.txt (+3/-0) src/server/asio_main_loop.cpp (+88/-307) src/server/asio_main_loop.h (+21/-27) src/server/asio_server_action_queue.cpp (+91/-0) src/server/asio_server_action_queue.h (+58/-0) src/server/asio_timer_service.cpp (+260/-0) src/server/asio_timer_service.h (+58/-0) src/server/compositor/default_configuration.cpp (+1/-1) src/server/default_server_configuration.cpp (+47/-2) src/server/display_server.cpp (+14/-1) src/server/synchronous_server_action.cpp (+50/-0) src/server/synchronous_server_action.h (+48/-0) tests/acceptance-tests/test_display_configuration.cpp (+1/-0) tests/integration-tests/test_display_server_main_loop_events.cpp (+44/-0) tests/unit-tests/CMakeLists.txt (+3/-0) tests/unit-tests/graphics/mesa/test_display.cpp (+5/-17) tests/unit-tests/graphics/mesa/test_linux_virtual_terminal.cpp (+2/-16) tests/unit-tests/test_asio_main_loop.cpp (+45/-244) tests/unit-tests/test_asio_server_action_queue.cpp (+219/-0) tests/unit-tests/test_asio_timer_service.cpp (+312/-0) tests/unit-tests/test_synchronous_server_action.cpp (+78/-0)
To merge this branch:	bzr merge lp:~andreas-pokorny/mir/split-main-loop-and-fix-races
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Alberto Aguirre (community)			Needs Fixing on 2014-06-23
Alan Griffiths			Needs Fixing on 2014-06-18
PS Jenkins bot (community)	continuous-integration		Approve on 2014-06-17
Alexandros Frantzis (community)		2014-05-22	Needs Fixing on 2014-06-16
Robert Carr (community)			Approve on 2014-06-16
Andreas Pokorny (community)			Approve on 2014-05-28
Review via email: mp+220571@code.launchpad.net

Commit message

Add the ability to unregister fd callbacks from AsioMainLoop
Moves Timer handling out of MainLoop into a separate TimerService

To identify previously registered callbacks the caller has to submit a void pointer. The supplied fd set does need to match the full registered set, only those supplied will be removed.

Reconfiguration operations are now executed sequentially together with the action handlers in both the TimerService and MainLoop.

Description of the change

Adds a method to unregister some of the file descriptors from a FDhandler in mir::AsioMainLoop. This is needed by the upcoming InputSender to unregister from InputChannels of removed surfaces.

The FDHandler is associated with a void const* on registry and when removing a file descriptor again from the main loop.

Since Asio copies handlers, cancelling operations does not really stop it
from execution. This can cause update issues in AlarmImpl::update_timer
that lead to alarms not being triggered again. The solution applied here
is sequential execution of all handlers inside the ServerActionQueues and reconfigurations of any kind are executed using the utility class SentinelAction which blocks until the action completes. This utility either executes inside the queue or on the callers thread, depending on the current thread id.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-22:

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

review: Approve (continuous-integration)

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-05-22:

What's the use case for this?

166 + remove_if(begin(stream_descriptors),

Do we also want to erase the items?

203 + end(stream_descriptors) != find_if(begin(stream_descriptors),
204 + end(stream_descriptors),
205 + [s](stream_descriptor_ptr const& item)
206 + {return item.get() == s;} ))

Doesn't this code need to be protected from concurrent access to stream descriptors? For example, handling an fd while concurrently reducing the fd set?

Also the code would be clearer if this piece of code was placed in a function.

Needs Information/Needs Fixing

review: Needs Fixing

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-05-22:

> What's the use case for this?

The input sender, I am just preparing, observes those fds for incoming input ACK messages, which contain Conumed/NotConsumed flags. Those fds are closed and opened together with surfaces.

> 166 + remove_if(begin(stream_descriptors),
>
> Do we also want to erase the items?

I thought that would happen here. The destruction of the unique_ptr instances should delete the instance.

>
> 203 + end(stream_descriptors) != find_if(begin(stream_descriptors),
> 204 + end(stream_descriptors),
> 205 + [s](stream_descriptor_ptr const& item)
> 206 + {return item.get() == s;} ))
>
> Doesn't this code need to be protected from concurrent access to stream
> descriptors? For example, handling an fd while concurrently reducing the fd
> set?

Yes. Thanks! Just fixed.

> Also the code would be clearer if this piece of code was placed in a function.
>
> Needs Information/Needs Fixing

ACK

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-05-22:

> > 166 + remove_if(begin(stream_descriptors),
> >
> > Do we also want to erase the items?
>
> I thought that would happen here. The destruction of the unique_ptr instances should delete the
> instance.

I was referring to the erase-remove idiom: std::remove and remove_if just move the items to the end of the data structure (invalidating them in the process), so we also need to erase them like:

v.erase(std::remove_if(...), v.end());

(Unfortunately the name 'remove' is not very descriptive of what the function is actually doing.)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-22:

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

review: Approve (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-05-22:

> I was referring to the erase-remove idiom: std::remove and remove_if just move
> the items to the end of the data structure (invalidating them in the process),
> so we also need to erase them like:
>
> v.erase(std::remove_if(...), v.end());
>
> (Unfortunately the name 'remove' is not very descriptive of what the function
> is actually doing.)

Maybe not descriptive but it was not thoughtful to assume that an algorithm could make that kind of changes to the container. Now that I fixed it I have to deal with the issue that asio might still execute a destroyed callback that refers to a recently destroyed Handler... fixing.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-22:

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

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-23:

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

review: Approve (continuous-integration)

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-05-23:

Is the current plan to use the server main loop, or will we have a different object?

I wonder if using the main loop mechanism to handle the FDs for input is appropriate. Do we need total serialization of the messages? I could imagine using a more focused class for this purpose (but I am not familiar with the details).

This is not a NACK, just a call for information.

review: Needs Information

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-05-23:

> Is the current plan to use the server main loop, or will we have a different
> object?
>
> I wonder if using the main loop mechanism to handle the FDs for input is
> appropriate. Do we need total serialization of the messages? I could imagine
> using a more focused class for this purpose (but I am not familiar with the
> details).

It is not strictly necessary to handles those responses in a specific or dedicated thread. Handling user input sequentially is . Up to now the android based input dispatcher forwards user input notifications into another thread for dispatch. This thread was also used to receive those responses.

In the InputSender implementation I assumed the main loop to be a natural pick as I did not assume a lot of load caused by those notifications and assumed that creating another thread for that use case is not necessary. A lot of assumptions... so to resolve that, how about:

class EventLoop : public graphics::EventHandlerRegister, public time::Timer, public ServerActionQueue
{
public:
virtual void run() = 0;
virtual void stop() = 0;
}; // or WorkLoop or just Loop ..

class MainLoop : EventLoop
{
};

class AsioMainLoop : MainLoop
{
...
};

That way the decision which thread should handle that could be done inside the DefaultServerConfiguration. i.e. through std::shared_ptr<EventLoop> the_sender_loop(), std::shared_ptr<EventLoop> the_dispatcher_loop(), ... and components could 'indicate' the necessity of strictly sequential execution, by requiring the type MainLoop.

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-05-27:

Now you can have a look at lp:~andreas-pokorny/mir/input-sender ~> lines 600ff

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-05-28:

There are races inside reconfiguration options like timer reschedule and unregistering the fd handler (the code around 270 + races for "still_registered")..

review: Needs Fixing

lp:~andreas-pokorny/mir/split-main-loop-and-fix-races updated on 2014-05-28

1649. By Andreas Pokorny on 2014-05-28: cleaning up empty lines and removing debug code

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-28:

FAILED: Continuous integration, rev:1648
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1752/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/367
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/368/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/365/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/271
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/271/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/270/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1023/console

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

review: Needs Fixing (continuous-integration)

lp:~andreas-pokorny/mir/split-main-loop-and-fix-races updated on 2014-05-28

1650. By Andreas Pokorny on 2014-05-28: fix missing update of alarm state to pending and missing timer_thread update
1651. By Andreas Pokorny on 2014-05-28: make AsioTimerService::stop synchronous

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-05-28:

The code is slightly simpler than the intermediate combined solution, but has grown in a way that replacing asio with a different solution looks like a good idea.

review: Approve

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-05-28:

+ in a later step (for main loop and timer fd)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-28:

FAILED: Continuous integration, rev:1650
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1756/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/372
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/373
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/370/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/275
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/275/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/274
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/274/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1033/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-28:

FAILED: Continuous integration, rev:1651
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1761/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/379
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/380
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/377/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/280
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/280/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/279
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/279/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1047
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1047/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1605/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/7858

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Alexandros Frantzis (afrantzis) on 2014-05-29:

lp:~andreas-pokorny/mir/split-main-loop-and-fix-races updated on 2014-05-30

1652. By Andreas Pokorny on 2014-05-30

Renamed SentinelAction to SynchronousServerAction

and all remaining references of main loop in timer service tests got renamed to timer service.

1653. By Andreas Pokorny on 2014-05-30

renamed test again

Revision history for this message

Andreas Pokorny (andreas-pokorny) on 2014-05-30:

lp:~andreas-pokorny/mir/split-main-loop-and-fix-races updated on 2014-05-30

1654. By Andreas Pokorny on 2014-05-30: Getting rid of InternalState in FDHandler

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-30:

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

review: Approve (continuous-integration)

lp:~andreas-pokorny/mir/split-main-loop-and-fix-races updated on 2014-05-30

1655. By Andreas Pokorny on 2014-05-30: remove action queue again
1656. By Andreas Pokorny on 2014-05-30: switching from mutex to atomic<State>

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-30:

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

review: Approve (continuous-integration)

Revision history for this message

Alexandros Frantzis (afrantzis) on 2014-05-30:

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-30:

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

review: Approve (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) on 2014-05-31:

lp:~andreas-pokorny/mir/split-main-loop-and-fix-races updated on 2014-05-31

1657. By Andreas Pokorny on 2014-05-31: Explain synchronizatio aspects of main loop and fix c++ style guide issues

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-31:

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

review: Approve (continuous-integration)

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-06-02:

First pass basically looks ok...

Not sure about handling the finished signals in main loop...while they aren't high bandwidth they are timing sensitive and my intuition is a thread will provide better average latency. Of course this is all just speculation without benchmarking of some realistic input scenarios.

The timeouts in the unit test of a few hundred milliseconds may need bumping, such timeouts frequently fail under QEMU on Jenkins.
oh

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-06-02:

Not sure where "oh" came from at the end there ;)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-06-03:

Why do we have one MP coupling two changes?

1. "Add the ability to unregister fd callbacks from AsioMainLoop" and
2. "Moves Timer handling out of MainLoop into a separate TimerService"

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-06-03:

We've used the base "mir" namespace for the utilities that help bring up the display server implemented by components in nested namespaces. I.e. it contains display_server, run_mir and default_configuration and a couple of files to support this.

I don't think src/server is the right place for asio_timer_service or synchronous_server_action. (Nor, necessarily, asio_main_loop but that's pre-existing.) Whether these belong in "time" (or a new "scheduling" namespace) is less clear to me.

review: Needs Fixing

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-06-03:

> Why do we have one MP coupling two changes?
>
> 1. "Add the ability to unregister fd callbacks from AsioMainLoop" and
> 2. "Moves Timer handling out of MainLoop into a separate TimerService"

It started with just 1. and when giving the rationale for 1, doing 2. was suggested, for keepig those things out of the main loop.

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-06-06:

833 + boost::optional<std::thread::id> main_loop_thread;
1486 + boost::optional<std::thread::id> queue_thread_id,

No need for boost::optional, since std::thread::id() constructs a thread id guaranteed to not represent any valid thread (it will compare unequal to any valid thread ids).

1511 + if (done)
1512 + return;

Unprotected access to 'done'.

1540 +#ifndef MIR_SENTINEL_ACTION_H
1541 +#define MIR_SENTINEL_ACTION_H

Wrong guard name (also missing '_' at the end of the name).

2240 + std::atomic<int> p1_handler_never_triggers{-1};
2241 + std::atomic<int> p2_handler_executes{-1};

No need for atomic, since the handlers are guaranteed not to run after run() returns.

2850 +TEST(SynchronousServerActionTest, enqueues_action_if_thread_differs)

It would be better to use an expectation on mtd::MockServerActionQueue::enqueue(), since
currently we don't check that the queue is actually used. For example, if we mess up the code and call action() directly instead of enqueuing this test will still succeed.

mtd::MockServerActionQueue mock_queue;
EXPECT_CALL(mock_queue, enqueue(_,_))
.WillOnce(...invoke arg2...);

review: Needs Fixing

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-06-10:

> 833 + boost::optional<std::thread::id> main_loop_thread;
> 1486 + boost::optional<std::thread::id> queue_thread_id,
>
> No need for boost::optional, since std::thread::id() constructs a thread id
> guaranteed to not represent any valid thread (it will compare unequal to any
> valid thread ids).

I use those three states to differ between - queue is not running, queue is running on a different thread, queue is running on the same thread.

> 1511 + if (done)
> 1512 + return;
>
> Unprotected access to 'done'.
>
> 1540 +#ifndef MIR_SENTINEL_ACTION_H
> 1541 +#define MIR_SENTINEL_ACTION_H
>
> Wrong guard name (also missing '_' at the end of the name).
>
> 2240 + std::atomic<int> p1_handler_never_triggers{-1};
> 2241 + std::atomic<int> p2_handler_executes{-1};
>
> No need for atomic, since the handlers are guaranteed not to run after run()
> returns.

ack fixing.

> 2850 +TEST(SynchronousServerActionTest, enqueues_action_if_thread_differs)
>
> It would be better to use an expectation on
> mtd::MockServerActionQueue::enqueue(), since
> currently we don't check that the queue is actually used. For example, if we
> mess up the code and call action() directly instead of enqueuing this test
> will still succeed.
>
> mtd::MockServerActionQueue mock_queue;
> EXPECT_CALL(mock_queue, enqueue(_,_))
> .WillOnce(...invoke arg2...);

See above, this is tested by the first unit test.

lp:~andreas-pokorny/mir/split-main-loop-and-fix-races updated on 2014-06-10

1658. By Andreas Pokorny on 2014-06-10: merged devel, and addressed findings
1659. By Andreas Pokorny on 2014-06-10: fixes strange indent

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-10:

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

review: Approve (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-06-10:

fixed the weird indent

> We've used the base "mir" namespace for the utilities that help bring up the
> display server implemented by components in nested namespaces. I.e. it
> contains display_server, run_mir and default_configuration and a couple of
> files to support this.
>
> I don't think src/server is the right place for asio_timer_service or
> synchronous_server_action. (Nor, necessarily, asio_main_loop but that's pre-
> existing.) Whether these belong in "time" (or a new "scheduling" namespace) is
> less clear to me.

I proposed one solution (with this branch as prerequisite) here: https://code.launchpad.net/~andreas-pokorny/mir/move-loops-to-scheduler-namespace/+merge/222616

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-06-10:

111 +class TimerService : public Timer
112 +{
113 +public:
114 + virtual void run() = 0;
115 + virtual void stop() = 0;
116 +};

I don't find "TimerService" a particularly helpful name. Saying "TimerService *is a* Timer" seems wrong.

Suggestions: "GlobalTimer", "MainTimer" or "SystemTimer"?

OTOH looking at mt::Timer ... that could be plausibly renamed "AlarmService".

Anyway, something about the naming needs fixing. ;)

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-10:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-06-10:

> 111 +class TimerService : public Timer
> 112 +{
> 113 +public:
> 114 + virtual void run() = 0;
> 115 + virtual void stop() = 0;
> 116 +};
>
> I don't find "TimerService" a particularly helpful name. Saying "TimerService
> *is a* Timer" seems wrong.
> Suggestions: "GlobalTimer", "MainTimer" or "SystemTimer"?

That part of the interface is all about starting (run) and stopping.. like in MainLoop so:

AlarmLoop?

> OTOH looking at mt::Timer ... that could be plausibly renamed "AlarmService".
>
> Anyway, something about the naming needs fixing. ;)

Agreed Timer is a bad pick for something that provides Alarms (aka Timers).
AlarmService sounds good ..

lp:~andreas-pokorny/mir/split-main-loop-and-fix-races updated on 2014-06-10

1660. By Andreas Pokorny on 2014-06-10: restore fix for alarm unit tests

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-10:

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

review: Approve (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-06-12:

> [...]
>
> Anyway, something about the naming needs fixing.

I tried to address that in the MP that also moves the classes into a better namespace: https://code.launchpad.net/~andreas-pokorny/mir/move-loops-to-scheduler-namespace/+merge/222616

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-06-13:

1061 + // of boost::asio only allowing static methods inside the taits type.

s/taits/traits/

~~~~

1372 + return std::make_shared<mc::TimeoutFrameDroppingPolicyFactory>(the_timer_service(),

TimeoutFrameDroppingPolicyFactory requires a shared_ptr<Timer>, not a shared_ptr<TimerService> - this reinforces my feeling that these interfaces are wrong. It also requires an extra header here that shouldn't be needed. Vis:

1363 +#include "mir/time/timer_service.h"

review: Needs Fixing

lp:~andreas-pokorny/mir/split-main-loop-and-fix-races updated on 2014-06-15

1661. By Andreas Pokorny on 2014-06-13: typo
1662. By Andreas Pokorny on 2014-06-15: Removed mir::timer::TimerService the run/stop part of the interface is now represented by mir::Loop

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-15:

FAILED: Continuous integration, rev:1662
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1900/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/567/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/573/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/566/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/421/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/418/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1609/console

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

review: Needs Fixing (continuous-integration)

lp:~andreas-pokorny/mir/split-main-loop-and-fix-races updated on 2014-06-16

1663. By Andreas Pokorny on 2014-06-16: merged lp:mir/development-branch
1664. By Andreas Pokorny on 2014-06-16: merged lp:mir/development-branch

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-16:

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

review: Approve (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-06-16:

> 1372 + return
> std::make_shared<mc::TimeoutFrameDroppingPolicyFactory>(the_timer_service(),
>
> TimeoutFrameDroppingPolicyFactory requires a shared_ptr<Timer>, not a
> shared_ptr<TimerService> - this reinforces my feeling that these interfaces
> are wrong. It also requires an extra header here that shouldn't be needed.
> Vis:
>
> 1363 +#include "mir/time/timer_service.h"

I think we already had that before, but with MainLoop. I think it was wrong to extend Timer with start/stop and use it as "the_timer_service()". Those two parts are now separate - I only need a combined interface for CachedPtr. I think I could have avoided that with a new interface for CachedPtr to accept a "compatible" make function - assuming that every operator() call of CachedPtr has identical behaviour.

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-06-16:

> assuming that every operator() call of CachedPtr
> has identical behaviour.

Responding to just this part of the comment (not the MP): it is hard to ensure this as users of Mir can override these calls.

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-06-16:

41 + virtual std::shared_ptr<mir::time::Timer> the_timer();

"mir::" is not needed.

~~~~

79 +namespace mir
80 +{
81 +class Loop

I think "Loop" is an uninformative name (what is it?) in the wrong namespace (what role it has in setting up the server)?

Should it be time::Scheduler or todo::Scheduler?

review: Needs Fixing

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-06-16:

> > assuming that every operator() call of CachedPtr
> > has identical behaviour.
>
> Responding to just this part of the comment (not the MP): it is hard to ensure
> this as users of Mir can override these calls.

Hm I thought I would be safe from that kind of abuse by making the CachedPtr private - but it still felt like a hack so I did not do it.

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-06-16:

> 41 + virtual std::shared_ptr<mir::time::Timer> the_timer();
>
> "mir::" is not needed.
>
> ~~~~
>
> 79 +namespace mir
> 80 +{
> 81 +class Loop
>
> I think "Loop" is an uninformative name (what is it?) in the wrong namespace
> (what role it has in setting up the server)?

A blocking but stoppable thing? I thought within that domain (to be moved to scheduler namespace) it would make at least some artificial sense. I.e a horrible implementation of Loop::run could be a simple loop: while(!done) { execute_pending_timers }

> Should it be time::Scheduler or todo::Scheduler?

Scheduler sounds ok too.

It is in the wrong namespace still, because I thought I should to the moving in a separate MP.

lp:~andreas-pokorny/mir/split-main-loop-and-fix-races updated on 2014-06-16

1665. By Andreas Pokorny on 2014-06-16: unnecessary namespace

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-06-16:

> > 833 + boost::optional<std::thread::id> main_loop_thread;
> > 1486 + boost::optional<std::thread::id> queue_thread_id,
> >
> > No need for boost::optional, since std::thread::id() constructs a thread id
> > guaranteed to not represent any valid thread (it will compare unequal to any
> > valid thread ids).
>
> I use those three states to differ between - queue is not running,
> queue is running on a different thread, queue is running on the same thread.

In the current code at least, I don't see a need to differentiate states (1) and (3):

if (current_thread == queue_thread) //state 2
{
...
}
else // no queue_thread or current_thread != queue_thread i.e. state 1 && state 3
{
...
}

Anyway, not a blocker.

+ virtual std::shared_ptr<Loop> the_timer_loop();
...and other timer related stuff

How about something like?

TimerService
{
  void run() = 0;
  void stop() = 0;
  std::shared_ptr<Timer> create_timer();
};

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-16:

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

review: Approve (continuous-integration)

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-06-16:

Mostly happy I think.

Not sure I see the point in the mir::Loop interface? Yes the main loop and the timer service both have run/stop methods, but which component really benefits from the loop interface?

review: Approve

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-06-16:

1959 -class MockEventRegister : public mg::EventHandlerRegister
1960 +class MockVirtualTerminal : public mgm::VirtualTerminal

This (new) class is not needed (we have mtd::MockVirtualTerminal).

> 2850 +TEST(SynchronousServerActionTest, enqueues_action_if_thread_differs)
>
> > It would be better to use an expectation on
> > mtd::MockServerActionQueue::enqueue(), since
> > currently we don't check that the queue is actually used. For example, if we
> > mess up the code and call action() directly instead of enqueuing this test
> > will still succeed.
>
> See above, this is tested by the first unit test.

I don't see how this is covered by the first test. All other tests check that enqueue is never called. If we mess up the code in the SynchronousServerAction constructor to always do:

done = true;
action();

all tests will still pass. Regardless of the details, the bottom line is that this test is called "enqueues_action_if_thread_differs", but it currently really is "executes_action_if_thread_differs".

review: Needs Fixing

lp:~andreas-pokorny/mir/split-main-loop-and-fix-races updated on 2014-06-16

1666. By Andreas Pokorny on 2014-06-16: review findings

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-17:

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

review: Approve (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-06-18:

> Mostly happy I think.
>
> Not sure I see the point in the mir::Loop interface? Yes the main loop and the
> timer service both have run/stop methods, but which component really benefits
> from the loop interface?

Mmmm.

Adding an interface whose purpose is unclear to the wrong namespace is enough to make me unhappy with the MP.

review: Needs Fixing

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-06-20:

OK I started looking at the dependent MP and decided that the comment really needs to be addressed here. It seems that the "scheduler" namespace proposed there is is going to be about dispatching events.

That implies that the Loop interface is about an EventLoop?

It makes more sense to start/stop "the event loop" than, as proposed here, to start and stop "the loop". ("Loop" is meaninglessly generic.)

A lot of the problem understanding this stack of MPs comes down to misleading or ambiguous names.

Maybe the name "scheduler" in the next MP is also setting wrong expectations. Only some of those events are schedule based - others are based on fds and interrupts. Perhaps "dispatcher" would be better.

But if both TimerLoop and MainLoop are dispatching events (albeit from different sources) I am still unsure that this is a right approach.

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-06-23:

Loop as an interface doesn't seem necessary.

It seems the timer and the "thing" in which the timer runs are coupled together so it doesn't make sense to expose "the_timer" and "the_timer_service" as the interfaces do not express the coupling between them.

Instead TimerService and Timer interfaces should fold into an AlarmService interface, expose it in the config as the_alarm_service() :

namespace mir
{
namespace time
{
class AlarmService
{
  void run() = 0;
  void stop() = 0;
  virtual std::unique_ptr<Alarm> notify_in(std::chrono::milliseconds delay,
      std::function<void()> callback) = 0;
  virtual std::unique_ptr<Alarm> notify_at(Timestamp time_point,
      std::function<void()> callback) = 0;
  virtual std::unique_ptr<Alarm> create_alarm(std::function<void()> callback) = 0;
};

Then a mir user that would like to replace the alarm service can use any mechanism it wants - threads, timer fds, mainloops...etc..

review: Needs Fixing

Unmerged revisions

1666. By Andreas Pokorny on 2014-06-16: review findings
1665. By Andreas Pokorny on 2014-06-16: unnecessary namespace
1664. By Andreas Pokorny on 2014-06-16: merged lp:mir/development-branch
1663. By Andreas Pokorny on 2014-06-16: merged lp:mir/development-branch
1662. By Andreas Pokorny on 2014-06-15: Removed mir::timer::TimerService the run/stop part of the interface is now represented by mir::Loop
1661. By Andreas Pokorny on 2014-06-13: typo
1660. By Andreas Pokorny on 2014-06-10: restore fix for alarm unit tests
1659. By Andreas Pokorny on 2014-06-10: fixes strange indent
1658. By Andreas Pokorny on 2014-06-10: merged devel, and addressed findings
1657. By Andreas Pokorny on 2014-05-31: Explain synchronizatio aspects of main loop and fix c++ style guide issues

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Andreas Pokorny

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'include/platform/mir/graphics/event_handler_register.h'
 --- include/platform/mir/graphics/event_handler_register.h	2013-08-28 03:41:48 +0000
 +++ include/platform/mir/graphics/event_handler_register.h	2014-06-16 22:41:10 +0000
@@ -37,8 +37,11 @@
      virtual void register_fd_handler(
          std::initializer_list<int> fds,
++        void const* owner,
          std::function<void(int)> const& handler) = 0;
++    virtual void unregister_fd_handler(void const* owner) = 0;
++
  protected:
      EventHandlerRegister() = default;
      virtual ~EventHandlerRegister() = default;
 === modified file 'include/server/mir/default_server_configuration.h'
 --- include/server/mir/default_server_configuration.h	2014-06-12 15:35:08 +0000
 +++ include/server/mir/default_server_configuration.h	2014-06-16 22:41:10 +0000
@@ -73,6 +73,8 @@
  namespace time
+ {
  class Clock;
++class Timer;
++class TimerLoop;
+ }
  namespace scene
+ {
@@ -159,6 +161,7 @@
      virtual std::shared_ptr<compositor::Compositor> the_compositor();
      virtual std::shared_ptr<input::InputManager>    the_input_manager();
      virtual std::shared_ptr<MainLoop>               the_main_loop();
++    virtual std::shared_ptr<Loop>                   the_timer_loop();
      virtual std::shared_ptr<ServerStatusListener>   the_server_status_listener();
      virtual std::shared_ptr<DisplayChanger>         the_display_changer();
      virtual std::shared_ptr<graphics::Platform>     the_graphics_platform();
@@ -279,6 +282,7 @@
      virtual std::shared_ptr<time::Clock> the_clock();
      virtual std::shared_ptr<ServerActionQueue> the_server_action_queue();
++    virtual std::shared_ptr<time::Timer> the_timer();
  protected:
      std::shared_ptr<options::Option> the_options() const;
@@ -363,6 +367,7 @@
      CachedPtr<graphics::DisplayReport> display_report;
      CachedPtr<time::Clock> clock;
      CachedPtr<MainLoop> main_loop;
++    CachedPtr<time::TimerLoop> timer_loop;
      CachedPtr<ServerStatusListener> server_status_listener;
      CachedPtr<graphics::DisplayConfigurationPolicy> display_configuration_policy;
      CachedPtr<graphics::nested::HostConnection> host_connection;
 === added file 'include/server/mir/loop.h'
 --- include/server/mir/loop.h	1970-01-01 00:00:00 +0000
 +++ include/server/mir/loop.h	2014-06-16 22:41:10 +0000
@@ -0,0 +1,37 @@
++/*
++ * Copyright © 2014 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: Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#ifndef MIR_LOOP_H_
++#define MIR_LOOP_H_
++
++namespace mir
++{
++class Loop
++{
++public:
++    Loop() = default;
++    virtual ~Loop() = default;
++    virtual void run() = 0;
++    virtual void stop() = 0;
++protected:
++    Loop(Loop const&) = delete;
++    Loop& operator=(Loop const&) = delete;
++};
++}
++
++#endif
 === modified file 'include/server/mir/main_loop.h'
 --- include/server/mir/main_loop.h	2014-06-03 11:04:15 +0000
 +++ include/server/mir/main_loop.h	2014-06-16 22:41:10 +0000
@@ -20,13 +20,12 @@
  #define MIR_MAIN_LOOP_H_
  #include "mir/graphics/event_handler_register.h"
--#include "mir/time/timer.h"
  #include "mir/server_action_queue.h"
  namespace mir
+ {
--class MainLoop : public graphics::EventHandlerRegister, public time::Timer,
++class MainLoop : public graphics::EventHandlerRegister,
                   public ServerActionQueue
+ {
  public:
 === modified file 'include/server/mir/server_configuration.h'
 --- include/server/mir/server_configuration.h	2014-06-02 17:07:02 +0000
 +++ include/server/mir/server_configuration.h	2014-06-16 22:41:10 +0000
@@ -50,6 +50,7 @@
+ }
  class MainLoop;
++class Loop;
  class ServerStatusListener;
  class DisplayChanger;
  class EmergencyCleanup;
@@ -65,6 +66,7 @@
      virtual std::shared_ptr<input::InputManager> the_input_manager() = 0;
      virtual std::shared_ptr<input::InputDispatcher> the_input_dispatcher() = 0;
      virtual std::shared_ptr<MainLoop> the_main_loop() = 0;
++    virtual std::shared_ptr<Loop> the_timer_loop() = 0;
      virtual std::shared_ptr<ServerStatusListener> the_server_status_listener() = 0;
      virtual std::shared_ptr<DisplayChanger> the_display_changer() = 0;
      virtual std::shared_ptr<graphics::Platform>  the_graphics_platform() = 0;
 === added file 'include/server/mir/time/timer_loop.h'
 --- include/server/mir/time/timer_loop.h	1970-01-01 00:00:00 +0000
 +++ include/server/mir/time/timer_loop.h	2014-06-16 22:41:10 +0000
@@ -0,0 +1,37 @@
++/*
++ * Copyright © 2014 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: Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#ifndef MIR_TIME_TIMER_LOOP_H_
++#define MIR_TIME_TIMER_LOOP_H_
++
++#include "mir/time/timer.h"
++#include "mir/loop.h"
++
++namespace mir
++{
++namespace time
++{
++
++class TimerLoop : public Timer, public Loop
++{
++};
++
++}
++}
++
++#endif
 === added file 'include/test/mir_test_doubles/mock_event_handler_register.h'
 --- include/test/mir_test_doubles/mock_event_handler_register.h	1970-01-01 00:00:00 +0000
 +++ include/test/mir_test_doubles/mock_event_handler_register.h	2014-06-16 22:41:10 +0000
@@ -0,0 +1,53 @@
++/*
++ * Copyright © 2014 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: Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#ifndef MOCK_EVENT_HANDLER_REGISTER_H_
++#define MOCK_EVENT_HANDLER_REGISTER_H_
++
++#include "mir/graphics/event_handler_register.h"
++
++#include <gmock/gmock.h>
++
++namespace mir
++{
++namespace test
++{
++namespace doubles
++{
++
++class MockEventHandlerRegister : public graphics::EventHandlerRegister
++{
++public:
++    ~MockEventHandlerRegister() noexcept {}
++
++    MOCK_METHOD2(register_signal_handler,
++                 void(std::initializer_list<int>,
++                      std::function<void(int)> const&));
++
++    MOCK_METHOD3(register_fd_handler,
++                 void(std::initializer_list<int>, void const*,
++                      std::function<void(int)> const&));
++
++    MOCK_METHOD1(unregister_fd_handler, void(void const*));
++};
++
++}
++}
++}
++
++#endif
 === added file 'include/test/mir_test_doubles/mock_loop.h'
 --- include/test/mir_test_doubles/mock_loop.h	1970-01-01 00:00:00 +0000
 +++ include/test/mir_test_doubles/mock_loop.h	2014-06-16 22:41:10 +0000
@@ -0,0 +1,44 @@
++/*
++ * Copyright © 2014 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: Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#ifndef MIR_MOCK_LOOP_H_
++#define MIR_MOCK_LOOP_H_
++
++#include "mir/loop.h"
++
++#include "mir_test/gmock_fixes.h"
++
++namespace mir
++{
++namespace test
++{
++namespace doubles
++{
++
++class MockLoop : public mir::Loop
++{
++public:
++    MOCK_METHOD0(run, void());
++    MOCK_METHOD0(stop, void());
++};
++
++}
++}
++}
++
++#endif
 === added file 'include/test/mir_test_doubles/mock_server_action_queue.h'
 --- include/test/mir_test_doubles/mock_server_action_queue.h	1970-01-01 00:00:00 +0000
 +++ include/test/mir_test_doubles/mock_server_action_queue.h	2014-06-16 22:41:10 +0000
@@ -0,0 +1,44 @@
++/*
++ * Copyright © 2014 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: Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#ifndef MIR_MOCK_SERVER_ACTION_QUEUE_H_
++#define MIR_MOCK_SERVER_ACTION_QUEUE_H_
++
++#include "mir/server_action_queue.h"
++#include <gmock/gmock.h>
++
++namespace mir
++{
++namespace test
++{
++namespace doubles
++{
++
++class MockServerActionQueue : public mir::ServerActionQueue
++{
++public:
++    MOCK_METHOD2(enqueue, void (void const* /*owner*/, mir::ServerAction const& /*action*/));
++    MOCK_METHOD1(pause_processing_for, void(void const* /*owner*/));
++    MOCK_METHOD1(resume_processing_for, void(void const* /*owner*/));
++};
++
++}
++}
++}
++
++#endif
 === modified file 'src/platform/graphics/mesa/display.cpp'
 --- src/platform/graphics/mesa/display.cpp	2014-06-10 14:40:23 +0000
 +++ src/platform/graphics/mesa/display.cpp	2014-06-16 22:41:10 +0000
@@ -233,6 +233,7 @@
+ {
      handlers.register_fd_handler(
          {monitor.fd()},
++        this,
          [conf_change_handler, this](int)
+         {
              monitor.process_events([conf_change_handler]
 === modified file 'src/server/CMakeLists.txt'
 --- src/server/CMakeLists.txt	2014-06-06 10:03:54 +0000
 +++ src/server/CMakeLists.txt	2014-06-16 22:41:10 +0000
@@ -30,6 +30,9 @@
    display_server.cpp
    default_server_configuration.cpp
    asio_main_loop.cpp
++  asio_server_action_queue.cpp
++  asio_timer_service.cpp
++  synchronous_server_action.cpp
+ )
  set(MIRSERVER_LINKAGE SHARED)
 === modified file 'src/server/asio_main_loop.cpp'
 --- src/server/asio_main_loop.cpp	2014-06-12 15:35:08 +0000
 +++ src/server/asio_main_loop.cpp	2014-06-16 22:41:10 +0000
@@ -16,87 +16,15 @@
   * Authored by: Alexandros Frantzis <alexandros.frantzis@canonical.com>
   */
--#include "mir/asio_main_loop.h"
++#include "asio_main_loop.h"
--#include "boost/date_time/posix_time/conversion.hpp"
++#include "synchronous_server_action.h"
  #include <cassert>
  #include <mutex>
  #include <condition_variable>
--namespace
--{
--struct MirClockTimerTraits
--{
--    // TODO the clock used by the main loop is a global setting, this is a restriction
--    // of boost::asio only allowing static methods inside the taits type.
--    struct TimerServiceClockStorage
--    {
--    public:
--        void set_clock(std::shared_ptr<mir::time::Clock const> const& clock)
--        {
--            std::lock_guard<std::mutex> lock(timer_service_mutex);
--            auto stored_clock = timer_service_clock.lock();
--            if (stored_clock && stored_clock != clock)
--                BOOST_THROW_EXCEPTION(std::logic_error("A clock is already in use as time source for mir::AsioMainLoop"));
--            timer_service_clock = clock;
--        }
--        mir::time::Timestamp now()
--        {
--            std::lock_guard<std::mutex> lock(timer_service_mutex);
--            auto clock = timer_service_clock.lock();
--            if (!clock)
--                BOOST_THROW_EXCEPTION(std::logic_error("No clock available to create time stamp"));
--            return clock->sample();
--        }
--    private:
--        std::mutex timer_service_mutex;
--        std::weak_ptr<mir::time::Clock const> timer_service_clock;
--    };
--
--    static TimerServiceClockStorage clock_storage;
--
--    static void set_clock(std::shared_ptr<mir::time::Clock const> const& clock)
--    {
--        clock_storage.set_clock(clock);
--    }
--
--    // time_traits interface required by boost::asio::deadline_timer{_service}
--    typedef mir::time::Timestamp time_type;
--    typedef std::chrono::milliseconds duration_type;
--
--
--    static time_type now()
--    {
--        return clock_storage.now();
--    }
--
--    static time_type add(const time_type& t, const duration_type& d)
--    {
--        return t + d;
--    }
--
--    static duration_type subtract(const time_type& t1, const time_type& t2)
--    {
--        return std::chrono::duration_cast<duration_type>(t1 - t2);
--    }
--
--    static bool less_than(const time_type& t1, const time_type& t2)
--    {
--        return t1 < t2;
--    }
--
--    static boost::posix_time::time_duration to_posix_duration(
--        const duration_type& d)
--    {
--        return boost::posix_time::millisec(d.count());
--    }
--};
--
--MirClockTimerTraits::TimerServiceClockStorage MirClockTimerTraits::clock_storage;
--
--typedef boost::asio::basic_deadline_timer<mir::time::Timestamp, MirClockTimerTraits> deadline_timer;
--}
++namespace bap = boost::asio::posix;
  class mir::AsioMainLoop::SignalHandler
+ {
@@ -137,46 +65,64 @@
  class mir::AsioMainLoop::FDHandler
+ {
  public:
++    typedef std::unique_ptr<bap::stream_descriptor> stream_descriptor_ptr;
++
      FDHandler(boost::asio::io_service& io,
                std::initializer_list<int> fds,
++              void const* owner,
                std::function<void(int)> const& handler)
--        : handler{handler}
++        : handler{handler}, owner{owner}
+     {
          for (auto fd : fds)
--        {
--            auto raw = new boost::asio::posix::stream_descriptor{io, fd};
--            auto s = std::unique_ptr<boost::asio::posix::stream_descriptor>(raw);
--            stream_descriptors.push_back(std::move(s));
--        }
--    }
--
--    void async_wait()
--    {
--        for (auto const& s : stream_descriptors)
--        {
--            s->async_read_some(
--                boost::asio::null_buffers(),
--                std::bind(&FDHandler::handle, this,
--                          std::placeholders::_1, std::placeholders::_2, s.get()));
--        }
++            stream_descriptors.emplace_back(new bap::stream_descriptor{io, fd});
++    }
++
++    bool is_owned_by(void const* possible_owner) const
++    {
++        return owner == possible_owner;
++    }
++
++    static void async_wait(std::shared_ptr<FDHandler> const& fd_handler, ServerActionQueue & queue)
++    {
++        for (auto const& s : fd_handler->stream_descriptors)
++            read_some(s.get(), fd_handler, queue);
+     }
  private:
--    void handle(boost::system::error_code err, size_t /*bytes*/,
--                boost::asio::posix::stream_descriptor* s)
++    static void read_some(bap::stream_descriptor* s, std::weak_ptr<FDHandler> const& possible_fd_handler, ServerActionQueue & queue)
+     {
--        if (!err)
--        {
--            handler(s->native_handle());
--            s->async_read_some(
--                boost::asio::null_buffers(),
--                std::bind(&FDHandler::handle, this,
--                          std::placeholders::_1, std::placeholders::_2, s));
--        }
++        s->async_read_some(
++            boost::asio::null_buffers(),
++            [possible_fd_handler,s,&queue](boost::system::error_code err, size_t /*bytes*/)
++            {
++                if (err)
++                    return;
++
++                // The actual execution of the fd handler is moved to the action queue.This allows us to synchronously
++                // unregister FDHandlers even when they are about to be executed. We do this because of the way Asio
++                // copies and executes pending completion handlers.
++                // In worst case during the call to unregister the FDHandler, it may still be executed, but not after
++                // the unregister call returned.
++                queue.enqueue(
++                    s,
++                    [possible_fd_handler, s, &queue]()
++                    {
++                        auto fd_handler = possible_fd_handler.lock();
++                        if (!fd_handler)
++                            return;
++
++                        fd_handler->handler(s->native_handle());
++                        fd_handler.reset();
++
++                        if (possible_fd_handler.lock())
++                            read_some(s, possible_fd_handler, queue);
++                    });
++            });
+     }
--    std::vector<std::unique_ptr<boost::asio::posix::stream_descriptor>> stream_descriptors;
--    std::function<void(int)> handler;
++    std::vector<stream_descriptor_ptr> stream_descriptors;
++    std::function<void(int)> const handler;
++    void const* owner;
  };
  /*
@@ -186,10 +132,9 @@
   * don't have complete type information for SignalHandler and fail
   * to compile.
   */
--mir::AsioMainLoop::AsioMainLoop(std::shared_ptr<time::Clock> const& clock)
--    : work{io}, clock(clock)
++mir::AsioMainLoop::AsioMainLoop()
++    : work{io}, action_queue{io}
+ {
--    MirClockTimerTraits::set_clock(clock);
+ }
  mir::AsioMainLoop::~AsioMainLoop() noexcept(true)
@@ -198,12 +143,14 @@
  void mir::AsioMainLoop::run()
+ {
++    main_loop_thread = std::this_thread::get_id();
      io.run();
+ }
  void mir::AsioMainLoop::stop()
+ {
      io.stop();
++    main_loop_thread.reset();
+ }
  void mir::AsioMainLoop::register_signal_handler(
@@ -222,220 +169,54 @@
  void mir::AsioMainLoop::register_fd_handler(
      std::initializer_list<int> fds,
++    void const* owner,
      std::function<void(int)> const& handler)
+ {
      assert(handler);
--    auto fd_handler = std::unique_ptr<FDHandler>{
--        new FDHandler{io, fds, handler}};
--
--    fd_handler->async_wait();
--
--    fd_handlers.push_back(std::move(fd_handler));
--}
--
--namespace
--{
--class AlarmImpl : public mir::time::Alarm
--{
--public:
--    AlarmImpl(boost::asio::io_service& io,
--              std::chrono::milliseconds delay,
--              std::function<void(void)> callback);
--
--    AlarmImpl(boost::asio::io_service& io,
--              mir::time::Timestamp time_point,
--              std::function<void(void)> callback);
--
--    AlarmImpl(boost::asio::io_service& io,
--              std::function<void(void)> callback);
--
--    ~AlarmImpl() noexcept override;
--
--    bool cancel() override;
--    State state() const override;
--
--    bool reschedule_in(std::chrono::milliseconds delay) override;
--    bool reschedule_for(mir::time::Timestamp time_point) override;
--private:
--    void update_timer();
--    struct InternalState
--    {
--        explicit InternalState(std::function<void(void)> callback)
--            : callback{callback}
--        {
--        }
--
--        mutable std::mutex m;
--        std::function<void(void)> callback;
--        State state;
--    };
--
--    ::deadline_timer timer;
--    std::shared_ptr<InternalState> data;
--};
--
--AlarmImpl::AlarmImpl(boost::asio::io_service& io,
--                     std::chrono::milliseconds delay,
--                     std::function<void ()> callback)
--    : AlarmImpl(io, callback)
--{
--    reschedule_in(delay);
--}
--
--AlarmImpl::AlarmImpl(boost::asio::io_service& io,
--                     mir::time::Timestamp time_point,
--                     std::function<void ()> callback)
--    : AlarmImpl(io, callback)
--{
--    reschedule_for(time_point);
--}
--
--AlarmImpl::AlarmImpl(boost::asio::io_service& io,
--                     std::function<void(void)> callback)
--    : timer{io},
--      data{std::make_shared<InternalState>(callback)}
--{
--    data->state = triggered;
--}
--
--AlarmImpl::~AlarmImpl() noexcept
--{
--    AlarmImpl::cancel();
--}
--
--bool AlarmImpl::cancel()
--{
--    std::lock_guard<decltype(data->m)> lock(data->m);
--    if (data->state == triggered)
--        return false;
--
--    data->state = cancelled;
--    timer.cancel();
--    return true;
--}
--
--mir::time::Alarm::State AlarmImpl::state() const
--{
--    std::lock_guard<decltype(data->m)> lock(data->m);
--
--    return data->state;
--}
--
--bool AlarmImpl::reschedule_in(std::chrono::milliseconds delay)
--{
--    bool cancelling = timer.expires_from_now(delay);
--    update_timer();
--    return cancelling;
--}
--
--bool AlarmImpl::reschedule_for(mir::time::Timestamp time_point)
--{
--    bool cancelling = timer.expires_at(time_point);
--    update_timer();
--    return cancelling;
--}
--
--void AlarmImpl::update_timer()
--{
--    std::lock_guard<decltype(data->m)> lock(data->m);
--    // Awkwardly, we can't stop the async_wait handler from being called
--    // on a destroyed AlarmImpl. This means we need to wedge a shared_ptr
--    // into the async_wait callback.
--    std::weak_ptr<InternalState> possible_data = data;
--    timer.async_wait([possible_data](boost::system::error_code const& ec)
--    {
--        auto data = possible_data.lock();
--        if (!data)
--            return;
--
--        std::unique_lock<decltype(data->m)> lock(data->m);
--        if (!ec && data->state == pending)
--        {
--            data->state = triggered;
--            lock.unlock();
--            data->callback();
--        }
--    });
--    data->state = pending;
--}
--}
--
--std::unique_ptr<mir::time::Alarm> mir::AsioMainLoop::notify_in(std::chrono::milliseconds delay,
--                                                               std::function<void()> callback)
--{
--    return std::unique_ptr<mir::time::Alarm>{new AlarmImpl{io, delay, callback}};
--}
--
--std::unique_ptr<mir::time::Alarm> mir::AsioMainLoop::notify_at(mir::time::Timestamp time_point,
--                                                               std::function<void()> callback)
--{
--    return std::unique_ptr<mir::time::Alarm>{new AlarmImpl{io, time_point, callback}};
--}
--
--std::unique_ptr<mir::time::Alarm> mir::AsioMainLoop::create_alarm(std::function<void()> callback)
--{
--    return std::unique_ptr<mir::time::Alarm>{new AlarmImpl{io, callback}};
++    auto fd_handler = std::make_shared<FDHandler>(io, fds, owner, handler);
++
++    FDHandler::async_wait(fd_handler, action_queue);
++
++    std::lock_guard<std::mutex> lock(fd_handlers_mutex);
++    fd_handlers.push_back(fd_handler);
++}
++
++void mir::AsioMainLoop::unregister_fd_handler(void const* owner)
++{
++    // The SynchronousServerAction makes sure that with the
++    // completion of the method unregister_fd_handler the
++    // handler will no longer be called.
++    // There is a chance for a fd handler callback to happen before
++    // the completion of this method.
++    SynchronousServerAction unregister{
++        action_queue,
++        main_loop_thread,
++        [this,owner]()
++        {
++            std::lock_guard<std::mutex> lock(fd_handlers_mutex);
++            auto end_of_valid = remove_if(
++                begin(fd_handlers),
++                end(fd_handlers),
++                [owner](std::shared_ptr<FDHandler> const& item)
++                {
++                    return item->is_owned_by(owner);
++                });
++            fd_handlers.erase(end_of_valid, end(fd_handlers));
++        }};
+ }
  void mir::AsioMainLoop::enqueue(void const* owner, ServerAction const& action)
+ {
--    {
--        std::lock_guard<std::mutex> lock{server_actions_mutex};
--        server_actions.push_back({owner, action});
--    }
--
--    io.post([this] { process_server_actions(); });
++    action_queue.enqueue(owner, action);
+ }
  void mir::AsioMainLoop::pause_processing_for(void const* owner)
+ {
--    std::lock_guard<std::mutex> lock{server_actions_mutex};
--    do_not_process.insert(owner);
++    action_queue.pause_processing_for(owner);
+ }
  void mir::AsioMainLoop::resume_processing_for(void const* owner)
+ {
--    {
--        std::lock_guard<std::mutex> lock{server_actions_mutex};
--        do_not_process.erase(owner);
--    }
--
--    io.post([this] { process_server_actions(); });
--}
--
--void mir::AsioMainLoop::process_server_actions()
--{
--    std::unique_lock<std::mutex> lock{server_actions_mutex};
--
--    size_t i = 0;
--
--    while (i < server_actions.size())
--    {
--        /*
--         * It's safe to use references to elements, since std::deque<>
--         * guarantees that references remain valid after appends, which is
--         * the only operation that can be performed on server_actions outside
--         * this function (in AsioMainLoop::post()).
--         */
--        auto const& owner = server_actions[i].first;
--        auto const& action = server_actions[i].second;
--
--        if (do_not_process.find(owner) == do_not_process.end())
--        {
--            lock.unlock();
--            action();
--            lock.lock();
--            /*
--             * This erase is always ok, since outside this function
--             * we only append to server_actions, i.e., our index i
--             * is guaranteed to remain valid and correct.
--             */
--            server_actions.erase(server_actions.begin() + i);
--        }
--        else
--        {
--            ++i;
--        }
--    }
++    action_queue.resume_processing_for(owner);
+ }
 === renamed file 'include/server/mir/asio_main_loop.h' => 'src/server/asio_main_loop.h'
 --- include/server/mir/asio_main_loop.h	2014-06-12 15:35:08 +0000
 +++ src/server/asio_main_loop.h	2014-06-16 22:41:10 +0000
@@ -21,8 +21,13 @@
  #include "mir/main_loop.h"
++#include "asio_server_action_queue.h"
++
  #include <boost/asio.hpp>
++#include <boost/optional.hpp>
++
  #include <memory>
++#include <thread>
  #include <vector>
  #include <mutex>
  #include <utility>
@@ -32,52 +37,41 @@
  namespace mir
+ {
--namespace time
--{
--class Clock;
--}
--
  class AsioMainLoop : public MainLoop
+ {
  public:
--    explicit AsioMainLoop(std::shared_ptr<time::Clock> const& clock);
++    AsioMainLoop();
      ~AsioMainLoop() noexcept(true);
--    void run();
--    void stop();
++    void run() override;
++    void stop() override;
      void register_signal_handler(
          std::initializer_list<int> signals,
--        std::function<void(int)> const& handler);
++        std::function<void(int)> const& handler) override;
      void register_fd_handler(
          std::initializer_list<int> fd,
--        std::function<void(int)> const& handler);
--
--    std::unique_ptr<time::Alarm> notify_in(std::chrono::milliseconds delay,
--                                           std::function<void()> callback) override;
--    std::unique_ptr<time::Alarm> notify_at(mir::time::Timestamp time_point,
--                                           std::function<void()> callback) override;
--    std::unique_ptr<time::Alarm> create_alarm(std::function<void()> callback) override;
--
--    void enqueue(void const* owner, ServerAction const& action);
--    void pause_processing_for(void const* owner);
--    void resume_processing_for(void const* owner);
++        void const* owner,
++        std::function<void(int)> const& handler) override;
++
++    void unregister_fd_handler(void const* owner) override;
++
++    void enqueue(void const* owner, ServerAction const& action) override;
++    void pause_processing_for(void const* owner) override;
++    void resume_processing_for(void const* owner) override;
  private:
      class SignalHandler;
      class FDHandler;
--    bool should_process(void const*);
--    void process_server_actions();
      boost::asio::io_service io;
      boost::asio::io_service::work work;
++    boost::optional<std::thread::id> main_loop_thread;
      std::vector<std::unique_ptr<SignalHandler>> signal_handlers;
--    std::vector<std::unique_ptr<FDHandler>> fd_handlers;
--    std::mutex server_actions_mutex;
--    std::deque<std::pair<void const*,ServerAction>> server_actions;
--    std::set<void const*> do_not_process;
--    std::shared_ptr<time::Clock> const clock;
++    std::vector<std::shared_ptr<FDHandler>> fd_handlers;
++    std::mutex fd_handlers_mutex;
++    AsioServerActionQueue action_queue;
  };
+ }
 === added file 'src/server/asio_server_action_queue.cpp'
 --- src/server/asio_server_action_queue.cpp	1970-01-01 00:00:00 +0000
 +++ src/server/asio_server_action_queue.cpp	2014-06-16 22:41:10 +0000
@@ -0,0 +1,91 @@
++/*
++ * Copyright © 2014 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: Alexandros Frantzis <alexandros.frantzis@canonical.com>
++ *              Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#include "asio_server_action_queue.h"
++
++mir::AsioServerActionQueue::AsioServerActionQueue(boost::asio::io_service & service)
++    : io(service)
++{
++}
++
++mir::AsioServerActionQueue::~AsioServerActionQueue() noexcept(true)
++{
++}
++
++void mir::AsioServerActionQueue::enqueue(void const* owner, ServerAction const& action)
++{
++    {
++        std::lock_guard<std::mutex> lock{server_actions_mutex};
++        server_actions.push_back({owner, action});
++    }
++
++    io.post([this] { process_server_actions(); });
++}
++
++void mir::AsioServerActionQueue::pause_processing_for(void const* owner)
++{
++    std::lock_guard<std::mutex> lock{server_actions_mutex};
++    do_not_process.insert(owner);
++}
++
++void mir::AsioServerActionQueue::resume_processing_for(void const* owner)
++{
++    {
++        std::lock_guard<std::mutex> lock{server_actions_mutex};
++        do_not_process.erase(owner);
++    }
++
++    io.post([this] { process_server_actions(); });
++}
++
++void mir::AsioServerActionQueue::process_server_actions()
++{
++    std::unique_lock<std::mutex> lock{server_actions_mutex};
++
++    size_t i = 0;
++
++    while (i < server_actions.size())
++    {
++        /*
++         * It's safe to use references to elements, since std::deque<>
++         * guarantees that references remain valid after appends, which is
++         * the only operation that can be performed on server_actions outside
++         * this function (in AsioServerActionQueue::post()).
++         */
++        auto const& owner = server_actions[i].first;
++        auto const& action = server_actions[i].second;
++
++        if (do_not_process.find(owner) == do_not_process.end())
++        {
++            lock.unlock();
++            action();
++            lock.lock();
++            /*
++             * This erase is always ok, since outside this function
++             * we only append to server_actions, i.e., our index i
++             * is guaranteed to remain valid and correct.
++             */
++            server_actions.erase(server_actions.begin() + i);
++        }
++        else
++        {
++            ++i;
++        }
++    }
++}
 === added file 'src/server/asio_server_action_queue.h'
 --- src/server/asio_server_action_queue.h	1970-01-01 00:00:00 +0000
 +++ src/server/asio_server_action_queue.h	2014-06-16 22:41:10 +0000
@@ -0,0 +1,58 @@
++/*
++ * Copyright © 2014 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: Alexandros Frantzis <alexandros.frantzis@canonical.com>
++ *              Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#ifndef MIR_ASIO_SERVER_ACTION_QUEUE
++#define MIR_ASIO_SERVER_ACTION_QUEUE
++
++#include "mir/server_action_queue.h"
++
++#include <boost/asio.hpp>
++
++#include <mutex>
++#include <condition_variable>
++#include <deque>
++#include <set>
++
++namespace mir
++{
++
++class AsioServerActionQueue : public ServerActionQueue
++{
++public:
++    explicit AsioServerActionQueue(boost::asio::io_service & service);
++    ~AsioServerActionQueue() noexcept(true);
++
++    void enqueue(void const* owner, ServerAction const& action) override;
++    void pause_processing_for(void const* owner) override;
++    void resume_processing_for(void const* owner) override;
++
++private:
++
++    bool should_process(void const*);
++    void process_server_actions();
++
++    boost::asio::io_service & io;
++    std::mutex server_actions_mutex;
++    std::deque<std::pair<void const*,ServerAction>> server_actions;
++    std::set<void const*> do_not_process;
++};
++
++}
++
++#endif
 === added file 'src/server/asio_timer_service.cpp'
 --- src/server/asio_timer_service.cpp	1970-01-01 00:00:00 +0000
 +++ src/server/asio_timer_service.cpp	2014-06-16 22:41:10 +0000
@@ -0,0 +1,260 @@
++/*
++ * Copyright © 2014 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: Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#include "asio_timer_service.h"
++
++#include "boost/date_time/posix_time/conversion.hpp"
++#include <mutex>
++#include <atomic>
++
++namespace
++{
++struct MirClockTimerTraits
++{
++    // TODO the clock used by the main loop is a global setting, this is a restriction
++    // of boost::asio only allowing static methods inside the traits type.
++    struct TimerServiceClockStorage
++    {
++    public:
++        void set_clock(std::shared_ptr<mir::time::Clock const> const& clock)
++        {
++            std::lock_guard<std::mutex> lock(timer_service_mutex);
++            auto stored_clock = timer_service_clock.lock();
++            if (stored_clock && stored_clock != clock)
++                BOOST_THROW_EXCEPTION(std::logic_error("A clock is already in use as time source for mir::AsioTimerService"));
++            timer_service_clock = clock;
++        }
++        mir::time::Timestamp now()
++        {
++            std::lock_guard<std::mutex> lock(timer_service_mutex);
++            auto clock = timer_service_clock.lock();
++            if (!clock)
++                BOOST_THROW_EXCEPTION(std::logic_error("No clock available to create time stamp"));
++            return clock->sample();
++        }
++    private:
++        std::mutex timer_service_mutex;
++        std::weak_ptr<mir::time::Clock const> timer_service_clock;
++    };
++
++    static TimerServiceClockStorage clock_storage;
++
++    static void set_clock(std::shared_ptr<mir::time::Clock const> const& clock)
++    {
++        clock_storage.set_clock(clock);
++    }
++
++    // time_traits interface required by boost::asio::deadline_timer{_service}
++    typedef mir::time::Timestamp time_type;
++    typedef std::chrono::milliseconds duration_type;
++
++
++    static time_type now()
++    {
++        return clock_storage.now();
++    }
++
++    static time_type add(const time_type& t, const duration_type& d)
++    {
++        return t + d;
++    }
++
++    static duration_type subtract(const time_type& t1, const time_type& t2)
++    {
++        return std::chrono::duration_cast<duration_type>(t1 - t2);
++    }
++
++    static bool less_than(const time_type& t1, const time_type& t2)
++    {
++        return t1 < t2;
++    }
++
++    static boost::posix_time::time_duration to_posix_duration(
++        const duration_type& d)
++    {
++        return boost::posix_time::millisec(d.count());
++    }
++};
++
++MirClockTimerTraits::TimerServiceClockStorage MirClockTimerTraits::clock_storage;
++
++typedef boost::asio::basic_deadline_timer<mir::time::Timestamp, MirClockTimerTraits> deadline_timer;
++}
++
++class mir::AsioTimerService::AlarmImpl : public mir::time::Alarm
++{
++public:
++    AlarmImpl(boost::asio::io_service& io,
++              std::chrono::milliseconds delay,
++              std::function<void(void)> callback);
++
++    AlarmImpl(boost::asio::io_service& io,
++              mir::time::Timestamp time_point,
++              std::function<void(void)> callback);
++
++    AlarmImpl(boost::asio::io_service& io,
++              std::function<void(void)> callback);
++
++    ~AlarmImpl() noexcept override;
++
++    // mir::time::Alarm:
++    bool cancel() override;
++    State state() const override;
++
++    bool reschedule_in(std::chrono::milliseconds delay) override;
++    bool reschedule_for(mir::time::Timestamp time_point) override;
++
++private:
++    void update_timer();
++
++    struct InternalState
++    {
++        InternalState(std::function<void(void)> callback)
++            : callback{callback}, state{pending}
++        {
++        }
++
++        std::function<void(void)> callback;
++        std::atomic<State> state;
++    };
++
++    ::deadline_timer timer;
++    std::shared_ptr<InternalState> data;
++};
++
++mir::AsioTimerService::AlarmImpl::AlarmImpl(boost::asio::io_service& io,
++                                            std::chrono::milliseconds delay,
++                                            std::function<void()> callback)
++    : AlarmImpl(io, callback)
++{
++    reschedule_in(delay);
++}
++
++mir::AsioTimerService::AlarmImpl::AlarmImpl(boost::asio::io_service& io,
++                                            mir::time::Timestamp time_point,
++                                            std::function<void()> callback)
++    : AlarmImpl(io, callback)
++{
++    reschedule_for(time_point);
++}
++
++mir::AsioTimerService::AlarmImpl::AlarmImpl(boost::asio::io_service& io,
++                                            std::function<void(void)> callback)
++    : timer{io},
++      data{std::make_shared<InternalState>(callback)}
++{
++    data->state = triggered;
++}
++
++mir::AsioTimerService::AlarmImpl::~AlarmImpl() noexcept
++{
++    AlarmImpl::cancel();
++}
++
++bool mir::AsioTimerService::AlarmImpl::cancel()
++{
++    State expected_state = pending;
++    if (data->state.compare_exchange_strong(expected_state, cancelled))
++    {
++        timer.cancel();
++        return true;
++    }
++    return false;
++}
++
++mir::time::Alarm::State mir::AsioTimerService::AlarmImpl::state() const
++{
++    return data->state;
++}
++
++bool mir::AsioTimerService::AlarmImpl::reschedule_in(std::chrono::milliseconds delay)
++{
++    bool cancelling = timer.expires_from_now(delay);
++    update_timer();
++    return cancelling;
++}
++
++bool mir::AsioTimerService::AlarmImpl::reschedule_for(mir::time::Timestamp time_point)
++{
++    bool cancelling = timer.expires_at(time_point);
++    update_timer();
++    return cancelling;
++}
++
++void mir::AsioTimerService::AlarmImpl::update_timer()
++{
++    auto new_internal_state = std::make_shared<InternalState>(data->callback);
++
++    // Awkwardly, we can't stop the async_wait handler from being called
++    // on a destroyed AlarmImpl. This means we need to wedge a shared_ptr
++    // into the async_wait callback.
++    std::weak_ptr<InternalState> possible_data = new_internal_state;
++    timer.async_wait([possible_data](boost::system::error_code const& ec)
++    {
++        if (ec)
++            return;
++
++        auto data = possible_data.lock();
++        if (!data)
++            return;
++
++        State expected_state = pending;
++        if (data->state.compare_exchange_strong(expected_state, triggered))
++            data->callback();
++    });
++
++    data = new_internal_state;
++}
++
++mir::AsioTimerService::AsioTimerService(std::shared_ptr<time::Clock> const& clock)
++    : work{io}, clock(clock)
++{
++    MirClockTimerTraits::set_clock(clock);
++}
++
++mir::AsioTimerService::~AsioTimerService() noexcept(true)
++{
++}
++
++void mir::AsioTimerService::run()
++{
++    io.run();
++}
++
++void mir::AsioTimerService::stop()
++{
++    io.stop();
++}
++
++std::unique_ptr<mir::time::Alarm> mir::AsioTimerService::notify_in(std::chrono::milliseconds delay,
++                                                                 std::function<void()> callback)
++{
++    return std::unique_ptr<mir::time::Alarm>{new AlarmImpl{io, delay, callback}};
++}
++
++std::unique_ptr<mir::time::Alarm> mir::AsioTimerService::notify_at(mir::time::Timestamp time_point,
++                                                                 std::function<void()> callback)
++{
++    return std::unique_ptr<mir::time::Alarm>{new AlarmImpl{io, time_point, callback}};
++
++}
++
++std::unique_ptr<mir::time::Alarm> mir::AsioTimerService::create_alarm(std::function<void()> callback)
++{
++    return std::unique_ptr<mir::time::Alarm>{new AlarmImpl{io, callback}};
++}
 === added file 'src/server/asio_timer_service.h'
 --- src/server/asio_timer_service.h	1970-01-01 00:00:00 +0000
 +++ src/server/asio_timer_service.h	2014-06-16 22:41:10 +0000
@@ -0,0 +1,58 @@
++/*
++ * Copyright © 2014 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: Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#ifndef MIR_ASIO_TIMER_SERVICE_H_
++#define MIR_ASIO_TIMER_SERVICE_H_
++
++#include "mir/time/timer_loop.h"
++
++#include <boost/asio.hpp>
++#include <boost/optional.hpp>
++
++#include <memory>
++#include <thread>
++#include <mutex>
++
++namespace mir
++{
++
++class AsioTimerService : public time::TimerLoop
++{
++public:
++    explicit AsioTimerService(std::shared_ptr<time::Clock> const& clock);
++    ~AsioTimerService() noexcept(true);
++
++    void run() override;
++    void stop() override;
++
++    std::unique_ptr<time::Alarm> notify_in(std::chrono::milliseconds delay,
++                                           std::function<void()> callback) override;
++    std::unique_ptr<time::Alarm> notify_at(mir::time::Timestamp time_point,
++                                           std::function<void()> callback) override;
++    std::unique_ptr<time::Alarm> create_alarm(std::function<void()> callback) override;
++private:
++    class AlarmImpl;
++    boost::asio::io_service io;
++    boost::asio::io_service::work work;
++
++    std::shared_ptr<time::Clock> const clock;
++};
++
++}
++
++#endif
 === modified file 'src/server/compositor/default_configuration.cpp'
 --- src/server/compositor/default_configuration.cpp	2014-06-09 17:16:32 +0000
 +++ src/server/compositor/default_configuration.cpp	2014-06-16 22:41:10 +0000
@@ -52,7 +52,7 @@
      return frame_dropping_policy_factory(
          [this]()
+         {
--            return std::make_shared<mc::TimeoutFrameDroppingPolicyFactory>(the_main_loop(),
++            return std::make_shared<mc::TimeoutFrameDroppingPolicyFactory>(the_timer(),
                                                                             std::chrono::milliseconds{100});
          });
+ }
 === modified file 'src/server/default_server_configuration.cpp'
 --- src/server/default_server_configuration.cpp	2014-06-11 16:11:32 +0000
 +++ src/server/default_server_configuration.cpp	2014-06-16 22:41:10 +0000
@@ -19,7 +19,6 @@
  #include "mir/default_server_configuration.h"
  #include "mir/options/default_configuration.h"
  #include "mir/abnormal_exit.h"
--#include "mir/asio_main_loop.h"
  #include "mir/default_server_status_listener.h"
  #include "mir/emergency_cleanup.h"
  #include "mir/default_configuration.h"
@@ -39,6 +38,9 @@
  #include "mir/default_configuration.h"
  #include "mir/scene/null_prompt_session_listener.h"
++#include "asio_timer_service.h"
++#include "asio_main_loop.h"
++
  #include <map>
  #include <vector>
  #include <mutex>
@@ -52,6 +54,31 @@
  namespace msh = mir::shell;
  namespace mi = mir::input;
++namespace
++{
++class AsioTimerServiceThread : public mir::AsioTimerService
++{
++public:
++    AsioTimerServiceThread(std::shared_ptr<mir::time::Clock> const& clock)
++        : AsioTimerService(clock)
++    {}
++    void run() override
++    {
++        service_thread = std::thread([this]{mir::AsioTimerService::run();});
++    }
++    void stop() override
++    {
++        if (service_thread.joinable())
++        {
++            mir::AsioTimerService::stop();
++            service_thread.join();
++        }
++    }
++private:
++    std::thread service_thread;
++};
++}
++
  mir::DefaultServerConfiguration::DefaultServerConfiguration(int argc, char const* argv[]) :
          DefaultServerConfiguration(std::make_shared<mo::DefaultConfiguration>(argc, argv))
+ {
@@ -185,7 +212,25 @@
      return main_loop(
          [this]()
+         {
--            return std::make_shared<mir::AsioMainLoop>(the_clock());
++            return std::make_shared<mir::AsioMainLoop>();
++        });
++}
++
++std::shared_ptr<mir::time::Timer> mir::DefaultServerConfiguration::the_timer()
++{
++    return timer_loop(
++        [this]()
++        {
++            return std::make_shared<AsioTimerServiceThread>(the_clock());
++        });
++}
++
++std::shared_ptr<mir::Loop> mir::DefaultServerConfiguration::the_timer_loop()
++{
++    return timer_loop(
++        [this]()
++        {
++            return std::make_shared<AsioTimerServiceThread>(the_clock());
          });
+ }
 === modified file 'src/server/display_server.cpp'
 --- src/server/display_server.cpp	2014-06-03 11:04:15 +0000
 +++ src/server/display_server.cpp	2014-06-16 22:41:10 +0000
@@ -21,6 +21,7 @@
  #include "mir/display_server.h"
  #include "mir/server_configuration.h"
  #include "mir/main_loop.h"
++#include "mir/loop.h"
  #include "mir/server_status_listener.h"
  #include "mir/display_changer.h"
@@ -80,7 +81,8 @@
            input_manager{config.the_input_manager()},
            main_loop{config.the_main_loop()},
            server_status_listener{config.the_server_status_listener()},
--          display_changer{config.the_display_changer()}
++          display_changer{config.the_display_changer()},
++          timer_loop{config.the_timer_loop()}
+     {
          display->register_configuration_change_handler(
              *main_loop,
@@ -96,6 +98,10 @@
+     {
          try
+         {
++            TryButRevertIfUnwinding timers{
++                [this] { timer_loop->stop(); },
++                [this] { timer_loop->run(); }};
++
              TryButRevertIfUnwinding dispatcher{
                  [this] { input_dispatcher->stop(); },
                  [this] { input_dispatcher->start(); }};
@@ -152,6 +158,10 @@
                  [this] { input_dispatcher->start(); },
                  [this] { input_dispatcher->stop(); }};
++            TryButRevertIfUnwinding timers{
++                [this] { timer_loop->run(); },
++                [this] { timer_loop->stop(); }};
++
              compositor->start();
+         }
          catch(std::runtime_error const&)
@@ -184,6 +194,7 @@
      std::shared_ptr<mir::MainLoop> const main_loop;
      std::shared_ptr<mir::ServerStatusListener> const server_status_listener;
      std::shared_ptr<mir::DisplayChanger> const display_changer;
++    std::shared_ptr<mir::Loop> const timer_loop;
  };
  mir::DisplayServer::DisplayServer(ServerConfiguration& config) :
@@ -206,11 +217,13 @@
      p->compositor->start();
      p->input_manager->start();
      p->input_dispatcher->start();
++    p->timer_loop->run();
      p->server_status_listener->started();
      p->main_loop->run();
++    p->timer_loop->stop();
      p->input_dispatcher->stop();
      p->input_manager->stop();
      p->compositor->stop();
 === added file 'src/server/synchronous_server_action.cpp'
 --- src/server/synchronous_server_action.cpp	1970-01-01 00:00:00 +0000
 +++ src/server/synchronous_server_action.cpp	2014-06-16 22:41:10 +0000
@@ -0,0 +1,50 @@
++/*
++ * Copyright © 2014 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: Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#include "synchronous_server_action.h"
++
++mir::SynchronousServerAction::SynchronousServerAction(
++    ServerActionQueue & queue,
++    boost::optional<std::thread::id> queue_thread_id,
++    ServerAction const& action) :
++    done{false}
++{
++    if (queue_thread_id &&
++        *queue_thread_id != std::this_thread::get_id())
++    {
++        queue.enqueue(this,
++                      [this,action]()
++                      {
++                          action();
++                          std::lock_guard<std::mutex> lock(done_mutex);
++                          done = true;
++                          done_condition.notify_one();
++                      });
++    }
++    else
++    {
++        done = true;
++        action();
++    }
++}
++
++mir::SynchronousServerAction::~SynchronousServerAction()
++{
++    std::unique_lock<std::mutex> lock(done_mutex);
++    while(!done) done_condition.wait(lock);
++}
 === added file 'src/server/synchronous_server_action.h'
 --- src/server/synchronous_server_action.h	1970-01-01 00:00:00 +0000
 +++ src/server/synchronous_server_action.h	2014-06-16 22:41:10 +0000
@@ -0,0 +1,48 @@
++/*
++ * Copyright © 2014 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: Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#ifndef MIR_SYNCHRONOUS_SERVER_ACTION_H_
++#define MIR_SYNCHRONOUS_SERVER_ACTION_H_
++
++#include "mir/server_action_queue.h"
++
++#include <boost/optional.hpp>
++
++#include <thread>
++#include <mutex>
++#include <condition_variable>
++
++namespace mir
++{
++
++class SynchronousServerAction
++{
++public:
++    SynchronousServerAction(ServerActionQueue & queue,
++                            boost::optional<std::thread::id> queue_thread_id,
++                            ServerAction const& action);
++    ~SynchronousServerAction();
++private:
++    std::mutex done_mutex;
++    bool done;
++    std::condition_variable done_condition;
++};
++
++}
++
++#endif
 === modified file 'tests/acceptance-tests/test_display_configuration.cpp'
 --- tests/acceptance-tests/test_display_configuration.cpp	2014-06-02 17:07:02 +0000
 +++ tests/acceptance-tests/test_display_configuration.cpp	2014-06-16 22:41:10 +0000
@@ -103,6 +103,7 @@
+     {
          handlers.register_fd_handler(
              {p.read_fd()},
++            this,
              [this, handler](int fd)
+             {
                  char c;
 === modified file 'tests/integration-tests/test_display_server_main_loop_events.cpp'
 --- tests/integration-tests/test_display_server_main_loop_events.cpp	2014-06-03 11:04:15 +0000
 +++ tests/integration-tests/test_display_server_main_loop_events.cpp	2014-06-16 22:41:10 +0000
@@ -30,6 +30,7 @@
  #include "mir_test_doubles/mock_input_manager.h"
  #include "mir_test_doubles/mock_input_dispatcher.h"
  #include "mir_test_doubles/mock_compositor.h"
++#include "mir_test_doubles/mock_loop.h"
  #include "mir_test_doubles/null_display.h"
  #include "mir_test_doubles/mock_server_status_listener.h"
  #include "mir/run_mir.h"
@@ -104,6 +105,7 @@
+     {
          handlers.register_fd_handler(
              {fd},
++            this,
              [this,conf_change_handler](int fd)
+             {
                  char c;
@@ -253,6 +255,14 @@
          return mock_input_dispatcher;
+     }
++    std::shared_ptr<mir::Loop> the_timer_loop() override
++    {
++        if (!mock_timer_loop)
++            mock_timer_loop = std::make_shared<mtd::MockLoop>();
++
++        return mock_timer_loop;
++    }
++
      std::shared_ptr<MockDisplay> the_mock_display()
+     {
          the_display();
@@ -271,6 +281,12 @@
          return mock_connector;
+     }
++    std::shared_ptr<mtd::MockLoop> the_mock_timer_loop()
++    {
++        the_timer_loop();
++        return mock_timer_loop;
++    }
++
      std::shared_ptr<mtd::MockInputManager> the_mock_input_manager()
+     {
          the_input_manager();
@@ -319,6 +335,7 @@
      std::shared_ptr<MockConnector> mock_connector;
      std::shared_ptr<mtd::MockInputManager> mock_input_manager;
      std::shared_ptr<mtd::MockInputDispatcher> mock_input_dispatcher;
++    std::shared_ptr<mtd::MockLoop> mock_timer_loop;
      mt::Pipe p;
      int const pause_signal;
@@ -381,6 +398,7 @@
      auto mock_connector = server_config.the_mock_connector();
      auto mock_input_manager = server_config.the_mock_input_manager();
      auto mock_input_dispatcher = server_config.the_mock_input_dispatcher();
++    auto mock_timer_loop = server_config.the_mock_timer_loop();
+     {
          InSequence s;
@@ -390,8 +408,10 @@
          EXPECT_CALL(*mock_compositor, start()).Times(1);
          EXPECT_CALL(*mock_input_manager, start()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
++        EXPECT_CALL(*mock_timer_loop, run()).Times(1);
          /* Pause */
++        EXPECT_CALL(*mock_timer_loop, stop()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
          EXPECT_CALL(*mock_input_manager, stop()).Times(1);
          EXPECT_CALL(*mock_compositor, stop()).Times(1);
@@ -403,9 +423,11 @@
          EXPECT_CALL(*mock_connector, start()).Times(1);
          EXPECT_CALL(*mock_input_manager, start()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
++        EXPECT_CALL(*mock_timer_loop, run()).Times(1);
          EXPECT_CALL(*mock_compositor, start()).Times(1);
          /* Stop */
++        EXPECT_CALL(*mock_timer_loop, stop()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
          EXPECT_CALL(*mock_input_manager, stop()).Times(1);
          EXPECT_CALL(*mock_compositor, stop()).Times(1);
@@ -437,6 +459,7 @@
      auto mock_connector = server_config.the_mock_connector();
      auto mock_input_manager = server_config.the_mock_input_manager();
      auto mock_input_dispatcher = server_config.the_mock_input_dispatcher();
++    auto mock_timer_loop = server_config.the_mock_timer_loop();
+     {
          InSequence s;
@@ -446,8 +469,10 @@
          EXPECT_CALL(*mock_compositor, start()).Times(1);
          EXPECT_CALL(*mock_input_manager, start()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
++        EXPECT_CALL(*mock_timer_loop, run()).Times(1);
          /* Pause */
++        EXPECT_CALL(*mock_timer_loop, stop()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
          EXPECT_CALL(*mock_input_manager, stop()).Times(1);
          EXPECT_CALL(*mock_compositor, stop()).Times(1);
@@ -455,6 +480,7 @@
          EXPECT_CALL(*mock_display, pause()).Times(1);
          /* Stop */
++        EXPECT_CALL(*mock_timer_loop, stop()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
          EXPECT_CALL(*mock_input_manager, stop()).Times(1);
          EXPECT_CALL(*mock_compositor, stop()).Times(1);
@@ -485,6 +511,7 @@
      auto mock_connector = server_config.the_mock_connector();
      auto mock_input_manager = server_config.the_mock_input_manager();
      auto mock_input_dispatcher = server_config.the_mock_input_dispatcher();
++    auto mock_timer_loop = server_config.the_mock_timer_loop();
+     {
          InSequence s;
@@ -494,8 +521,10 @@
          EXPECT_CALL(*mock_compositor, start()).Times(1);
          EXPECT_CALL(*mock_input_manager, start()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
++        EXPECT_CALL(*mock_timer_loop, run()).Times(1);
          /* Pause failure */
++        EXPECT_CALL(*mock_timer_loop, stop()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
          EXPECT_CALL(*mock_input_manager, stop()).Times(1);
          EXPECT_CALL(*mock_compositor, stop()).Times(1);
@@ -508,8 +537,10 @@
          EXPECT_CALL(*mock_compositor, start()).Times(1);
          EXPECT_CALL(*mock_input_manager, start()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
++        EXPECT_CALL(*mock_timer_loop, run()).Times(1);
          /* Stop */
++        EXPECT_CALL(*mock_timer_loop, stop()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
          EXPECT_CALL(*mock_input_manager, stop()).Times(1);
          EXPECT_CALL(*mock_compositor, stop()).Times(1);
@@ -540,6 +571,7 @@
      auto mock_connector = server_config.the_mock_connector();
      auto mock_input_manager = server_config.the_mock_input_manager();
      auto mock_input_dispatcher = server_config.the_mock_input_dispatcher();
++    auto mock_timer_loop = server_config.the_mock_timer_loop();
+     {
          InSequence s;
@@ -549,6 +581,7 @@
          EXPECT_CALL(*mock_compositor, start()).Times(1);
          EXPECT_CALL(*mock_input_manager, start()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
++        EXPECT_CALL(*mock_timer_loop, run()).Times(1);
          /* Change configuration */
          EXPECT_CALL(*mock_compositor, stop()).Times(1);
@@ -556,6 +589,7 @@
          EXPECT_CALL(*mock_compositor, start()).Times(1);
          /* Stop */
++        EXPECT_CALL(*mock_timer_loop, stop()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
          EXPECT_CALL(*mock_input_manager, stop()).Times(1);
          EXPECT_CALL(*mock_compositor, stop()).Times(1);
@@ -587,6 +621,7 @@
      auto mock_connector = server_config.the_mock_connector();
      auto mock_input_manager = server_config.the_mock_input_manager();
      auto mock_input_dispatcher = server_config.the_mock_input_dispatcher();
++    auto mock_timer_loop = server_config.the_mock_timer_loop();
+     {
          InSequence s;
@@ -596,8 +631,10 @@
          EXPECT_CALL(*mock_compositor, start()).Times(1);
          EXPECT_CALL(*mock_input_manager, start()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
++        EXPECT_CALL(*mock_timer_loop, run()).Times(1);
          /* Pause event */
++        EXPECT_CALL(*mock_timer_loop, stop()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
          EXPECT_CALL(*mock_input_manager, stop()).Times(1);
          EXPECT_CALL(*mock_compositor, stop()).Times(1);
@@ -609,6 +646,7 @@
          EXPECT_CALL(*mock_connector, start()).Times(1);
          EXPECT_CALL(*mock_input_manager, start()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
++        EXPECT_CALL(*mock_timer_loop, run()).Times(1);
          EXPECT_CALL(*mock_compositor, start()).Times(1);
          /* Change configuration (after resuming) */
@@ -617,6 +655,7 @@
          EXPECT_CALL(*mock_compositor, start()).Times(1);
          /* Stop */
++        EXPECT_CALL(*mock_timer_loop, stop()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
          EXPECT_CALL(*mock_input_manager, stop()).Times(1);
          EXPECT_CALL(*mock_compositor, stop()).Times(1);
@@ -652,6 +691,7 @@
      auto mock_input_manager = server_config.the_mock_input_manager();
      auto mock_input_dispatcher = server_config.the_mock_input_dispatcher();
      auto mock_server_status_listener = server_config.the_mock_server_status_listener();
++    auto mock_timer_loop = server_config.the_mock_timer_loop();
+     {
          InSequence s;
@@ -661,9 +701,11 @@
          EXPECT_CALL(*mock_compositor, start()).Times(1);
          EXPECT_CALL(*mock_input_manager, start()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
++        EXPECT_CALL(*mock_timer_loop, run()).Times(1);
          EXPECT_CALL(*mock_server_status_listener, started()).Times(1);
          /* "paused" is emitted after all components have been paused/stopped */
++        EXPECT_CALL(*mock_timer_loop, stop()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
          EXPECT_CALL(*mock_input_manager, stop()).Times(1);
          EXPECT_CALL(*mock_compositor, stop()).Times(1);
@@ -676,10 +718,12 @@
          EXPECT_CALL(*mock_connector, start()).Times(1);
          EXPECT_CALL(*mock_input_manager, start()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, start()).Times(1);
++        EXPECT_CALL(*mock_timer_loop, run()).Times(1);
          EXPECT_CALL(*mock_compositor, start()).Times(1);
          EXPECT_CALL(*mock_server_status_listener, resumed()).Times(1);
          /* Stop */
++        EXPECT_CALL(*mock_timer_loop, stop()).Times(1);
          EXPECT_CALL(*mock_input_dispatcher, stop()).Times(1);
          EXPECT_CALL(*mock_input_manager, stop()).Times(1);
          EXPECT_CALL(*mock_compositor, stop()).Times(1);
 === modified file 'tests/unit-tests/CMakeLists.txt'
 --- tests/unit-tests/CMakeLists.txt	2014-06-04 15:30:21 +0000
 +++ tests/unit-tests/CMakeLists.txt	2014-06-16 22:41:10 +0000
@@ -8,7 +8,10 @@
    UNIT_TEST_SOURCES
    test_gmock_fixes.cpp
++  test_synchronous_server_action.cpp
    test_asio_main_loop.cpp
++  test_asio_server_action_queue.cpp
++  test_asio_timer_service.cpp
    shared_library_test.cpp
    test_raii.cpp
    test_udev_wrapper.cpp
 === modified file 'tests/unit-tests/graphics/mesa/test_display.cpp'
 --- tests/unit-tests/graphics/mesa/test_display.cpp	2014-06-10 14:40:23 +0000
 +++ tests/unit-tests/graphics/mesa/test_display.cpp	2014-06-16 22:41:10 +0000
@@ -23,13 +23,13 @@
  #include "mir/logging/logger.h"
  #include "mir/graphics/display_buffer.h"
  #include "src/server/graphics/default_display_configuration_policy.h"
--#include "mir/time/high_resolution_clock.h"
--#include "mir/asio_main_loop.h"
++#include "src/server/asio_main_loop.h"
  #include "mir_test_doubles/mock_egl.h"
  #include "mir_test_doubles/mock_gl.h"
  #include "src/server/report/null_report_factory.h"
  #include "mir_test_doubles/mock_display_report.h"
++#include "mir_test_doubles/mock_event_handler_register.h"
  #include "mir_test_doubles/null_virtual_terminal.h"
  #include "mir_test_doubles/stub_gl_config.h"
  #include "mir_test_doubles/mock_gl_config.h"
@@ -69,18 +69,6 @@
      ~MockLogger() noexcept(true) {}
  };
--class MockEventRegister : public mg::EventHandlerRegister
--{
--public:
--    MOCK_METHOD2(register_signal_handler,
--                 void(std::initializer_list<int>,
--                 std::function<void(int)> const&));
--    MOCK_METHOD2(register_fd_handler,
--                 void(std::initializer_list<int>,
--                 std::function<void(int)> const&));
--};
--
--
  class MesaDisplayTest : public ::testing::Test
+ {
  public:
@@ -706,9 +694,9 @@
      using namespace testing;
      auto display = create_display(create_platform());
--    MockEventRegister mock_register;
++    mtd::MockEventHandlerRegister mock_register;
--    EXPECT_CALL(mock_register, register_fd_handler(_,_));
++    EXPECT_CALL(mock_register, register_fd_handler(_,_,_));
      display->register_configuration_change_handler(mock_register, []{});
+ }
@@ -719,7 +707,7 @@
      auto display = create_display(create_platform());
--    mir::AsioMainLoop ml{std::make_shared<mir::time::HighResolutionClock>()};
++    mir::AsioMainLoop ml;
      std::condition_variable done;
      int const device_add_count{1};
 === modified file 'tests/unit-tests/graphics/mesa/test_linux_virtual_terminal.cpp'
 --- tests/unit-tests/graphics/mesa/test_linux_virtual_terminal.cpp	2014-06-14 19:52:52 +0000
 +++ tests/unit-tests/graphics/mesa/test_linux_virtual_terminal.cpp	2014-06-16 22:41:10 +0000
@@ -18,10 +18,10 @@
  #include "src/platform/graphics/mesa/linux_virtual_terminal.h"
  #include "src/server/report/null_report_factory.h"
--#include "mir/graphics/event_handler_register.h"
  #include "mir_test/fake_shared.h"
  #include "mir_test_doubles/mock_display_report.h"
++#include "mir_test_doubles/mock_event_handler_register.h"
  #include "mir_test/gmock_fixes.h"
  #include <gtest/gtest.h>
@@ -70,20 +70,6 @@
  // Add a typedef to aid clarity.
  typedef testing::NiceMock<MockPosixProcessOperations> StubPosixProcessOperations;
--class MockEventHandlerRegister : public mg::EventHandlerRegister
--{
--public:
--    ~MockEventHandlerRegister() noexcept {}
--
--    MOCK_METHOD2(register_signal_handler,
--                 void(std::initializer_list<int>,
--                      std::function<void(int)> const&));
--
--    MOCK_METHOD2(register_fd_handler,
--                 void(std::initializer_list<int>,
--                      std::function<void(int)> const&));
--};
--
  ACTION_TEMPLATE(SetIoctlPointee,
                  HAS_1_TEMPLATE_PARAMS(typename, T),
                  AND_1_VALUE_PARAMS(param))
@@ -254,7 +240,7 @@
      struct termios fake_tc_attr;
      std::function<void(int)> sig_handler;
      MockVTFileOperations mock_fops;
--    MockEventHandlerRegister mock_event_handler_register;
++    mtd::MockEventHandlerRegister mock_event_handler_register;
  };
 === modified file 'tests/unit-tests/test_asio_main_loop.cpp'
 --- tests/unit-tests/test_asio_main_loop.cpp	2014-06-12 15:35:08 +0000
 +++ tests/unit-tests/test_asio_main_loop.cpp	2014-06-16 22:41:10 +0000
@@ -16,8 +16,8 @@
   * Authored by: Alexandros Frantzis <alexandros.frantzis@canonical.com>
   */
--#include "mir/asio_main_loop.h"
--#include "mir/time/high_resolution_clock.h"
++#include "src/server/asio_main_loop.h"
++
  #include "mir_test/pipe.h"
  #include "mir_test/auto_unblock_thread.h"
  #include "mir_test/signal.h"
@@ -45,68 +45,7 @@
  class AsioMainLoopTest : public ::testing::Test
+ {
  public:
--    mir::AsioMainLoop ml{std::make_shared<mir::time::HighResolutionClock>()};
--};
--
--class AdvanceableClock : public mir::time::Clock
--{
--public:
--    mir::time::Timestamp sample() const override
--    {
--        std::lock_guard<std::mutex> lock(time_mutex);
--        return current_time;
--    }
--    void advance_by(std::chrono::milliseconds const step, mir::AsioMainLoop & ml)
--    {
--        bool done = false;
--        std::mutex checkpoint_mutex;
--        std::condition_variable checkpoint;
--
--        {
--            std::lock_guard<std::mutex> lock(time_mutex);
--            current_time += step;
--        }
--        auto evaluate_clock_alarm = ml.notify_in(
--            std::chrono::milliseconds{0},
--            [&done, &checkpoint_mutex, &checkpoint]
--            {
--                std::unique_lock<std::mutex> lock(checkpoint_mutex);
--                done = true;
--                checkpoint.notify_one();
--            });
--
--        std::unique_lock<std::mutex> lock(checkpoint_mutex);
--        while(!done) checkpoint.wait(lock);
--
--    }
--private:
--    mutable std::mutex time_mutex;
--    mir::time::Timestamp current_time{
--        []
--        {
--           mir::time::HighResolutionClock clock;
--           return clock.sample();
--        }()
--        };
--};
--
--
--class AsioMainLoopAlarmTest : public ::testing::Test
--{
--public:
--    std::shared_ptr<AdvanceableClock> clock = std::make_shared<AdvanceableClock>();
--    mir::AsioMainLoop ml{clock};
--    int call_count{0};
--    mt::WaitObject wait;
--    std::chrono::milliseconds delay{50};
--
--    struct UnblockMainLoop : mt::AutoUnblockThread
--    {
--        UnblockMainLoop(mir::AsioMainLoop & loop)
--            : mt::AutoUnblockThread([&loop]() {loop.stop();},
--                                    [&loop]() {loop.run();})
--        {}
--    };
++    mir::AsioMainLoop ml;
  };
  class Counter
@@ -258,6 +197,7 @@
      ml.register_fd_handler(
          {p.read_fd()},
++        this,
          [&handled_fd, &data_read, this](int fd)
+         {
              handled_fd = fd;
@@ -282,6 +222,7 @@
      ml.register_fd_handler(
          {pipes[0].read_fd(), pipes[1].read_fd()},
++        this,
          [&handled_fds, &elems_read, &num_handled_fds](int fd)
+         {
              handled_fds.push_back(fd);
@@ -329,6 +270,7 @@
      ml.register_fd_handler(
          {pipes[0].read_fd()},
++        this,
          [&handled_fds, &elems_read, this](int fd)
+         {
              EXPECT_EQ(static_cast<ssize_t>(sizeof(elems_read[0])),
@@ -344,6 +286,7 @@
      ml.register_fd_handler(
          {pipes[1].read_fd()},
++        this,
          [&handled_fds, &elems_read, this](int fd)
+         {
              EXPECT_EQ(static_cast<ssize_t>(sizeof(elems_read[1])),
@@ -359,6 +302,7 @@
      ml.register_fd_handler(
          {pipes[2].read_fd()},
++        this,
          [&handled_fds, &elems_read, this](int fd)
+         {
              EXPECT_EQ(static_cast<ssize_t>(sizeof(elems_read[2])),
@@ -390,185 +334,42 @@
      EXPECT_EQ(elems_to_send[2], elems_read[2]);
+ }
--TEST_F(AsioMainLoopAlarmTest, main_loop_runs_until_stop_called)
--{
--    auto mainloop_started = std::make_shared<mt::Signal>();
--
--    auto fire_on_mainloop_start = ml.notify_in(std::chrono::milliseconds{0},
--                                               [mainloop_started]()
--    {
--        mainloop_started->raise();
--    });
--
--    UnblockMainLoop unblocker(ml);
--
--    ASSERT_TRUE(mainloop_started->wait_for(std::chrono::milliseconds{100}));
--
--    auto timer_fired = std::make_shared<mt::Signal>();
--    auto alarm = ml.notify_in(std::chrono::milliseconds{10}, [timer_fired]
--    {
--        timer_fired->raise();
--    });
--
--    clock->advance_by(std::chrono::milliseconds{10}, ml);
--    EXPECT_TRUE(timer_fired->wait_for(std::chrono::milliseconds{500}));
--
--    ml.stop();
--    // Main loop should be stopped now
--
--    timer_fired = std::make_shared<mt::Signal>();
--    auto should_not_fire =  ml.notify_in(std::chrono::milliseconds{0},
--                                         [timer_fired]()
--    {
--        timer_fired->raise();
--    });
--
--    EXPECT_FALSE(timer_fired->wait_for(std::chrono::milliseconds{100}));
--}
--
--TEST_F(AsioMainLoopAlarmTest, alarm_starts_in_pending_state)
--{
--    auto alarm = ml.notify_in(delay, [this]() {});
--
--    UnblockMainLoop unblocker(ml);
--
--    EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
--}
--
--TEST_F(AsioMainLoopAlarmTest, alarm_fires_with_correct_delay)
--{
--    UnblockMainLoop unblocker(ml);
--
--    auto alarm = ml.notify_in(delay, [](){});
--
--    clock->advance_by(delay - std::chrono::milliseconds{1}, ml);
--    EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
--
--    clock->advance_by(delay, ml);
--    EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
--}
--
--TEST_F(AsioMainLoopAlarmTest, multiple_alarms_fire)
--{
--    using namespace testing;
--
--    int const alarm_count{10};
--    Counter call_count;
--    std::array<std::unique_ptr<mir::time::Alarm>, alarm_count> alarms;
--
--    for (auto& alarm : alarms)
--        alarm = ml.notify_in(delay, [&call_count](){++call_count;});
--
--    UnblockMainLoop unblocker(ml);
--    clock->advance_by(delay, ml);
--
--    call_count.wait_for(delay, alarm_count);
--    EXPECT_THAT(call_count, Eq(alarm_count));
--
--    for (auto const& alarm : alarms)
--        EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
--}
--
--TEST_F(AsioMainLoopAlarmTest, alarm_changes_to_triggered_state)
--{
--    auto alarm_fired = std::make_shared<mt::Signal>();
--    auto alarm = ml.notify_in(std::chrono::milliseconds{5}, [alarm_fired]()
--    {
--        alarm_fired->raise();
--    });
--
--    UnblockMainLoop unblocker(ml);
--
--    clock->advance_by(delay, ml);
--    ASSERT_TRUE(alarm_fired->wait_for(std::chrono::milliseconds{100}));
--
--    EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
--}
--
--TEST_F(AsioMainLoopAlarmTest, cancelled_alarm_doesnt_fire)
--{
--    UnblockMainLoop unblocker(ml);
--    auto alarm = ml.notify_in(std::chrono::milliseconds{100},
--                              [](){ FAIL() << "Alarm handler of canceld alarm called";});
--
--    EXPECT_TRUE(alarm->cancel());
--
--    EXPECT_EQ(mir::time::Alarm::cancelled, alarm->state());
--
--    clock->advance_by(std::chrono::milliseconds{100}, ml);
--
--    EXPECT_EQ(mir::time::Alarm::cancelled, alarm->state());
--}
--
--TEST_F(AsioMainLoopAlarmTest, destroyed_alarm_doesnt_fire)
--{
--    auto alarm = ml.notify_in(std::chrono::milliseconds{200},
--                              [](){ FAIL() << "Alarm handler of destroyed alarm called"; });
--
--    UnblockMainLoop unblocker(ml);
--
--    alarm.reset(nullptr);
--    clock->advance_by(std::chrono::milliseconds{200}, ml);
--}
--
--TEST_F(AsioMainLoopAlarmTest, rescheduled_alarm_fires_again)
--{
--    std::atomic<int> call_count{0};
--
--    auto alarm = ml.notify_in(std::chrono::milliseconds{0}, [&call_count]()
--    {
--        if (call_count++ > 1)
--            FAIL() << "Alarm called too many times";
--    });
--
--    UnblockMainLoop unblocker(ml);
--
--    clock->advance_by(std::chrono::milliseconds{0}, ml);
--    ASSERT_EQ(mir::time::Alarm::triggered, alarm->state());
--
--    alarm->reschedule_in(std::chrono::milliseconds{100});
--    EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
--
--    clock->advance_by(std::chrono::milliseconds{100}, ml);
--    EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
--}
--
--TEST_F(AsioMainLoopAlarmTest, rescheduled_alarm_cancels_previous_scheduling)
--{
--    std::atomic<int> call_count{0};
--
--    auto alarm = ml.notify_in(std::chrono::milliseconds{100}, [&call_count]()
--    {
--        call_count++;
--    });
--
--    UnblockMainLoop unblocker(ml);
--    clock->advance_by(std::chrono::milliseconds{90}, ml);
--
--    EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
--    EXPECT_EQ(0, call_count);
--    EXPECT_TRUE(alarm->reschedule_in(std::chrono::milliseconds{100}));
--    EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
--
--    clock->advance_by(std::chrono::milliseconds{110}, ml);
--
--    EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
--    EXPECT_EQ(1, call_count);
--}
--
--TEST_F(AsioMainLoopAlarmTest, alarm_fires_at_correct_time_point)
--{
--    mir::time::Timestamp real_soon = clock->sample() + std::chrono::milliseconds{120};
--
--    auto alarm = ml.notify_at(real_soon, []{});
--
--    UnblockMainLoop unblocker(ml);
--
--    clock->advance_by(std::chrono::milliseconds{119}, ml);
--    EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
--
--    clock->advance_by(std::chrono::milliseconds{1}, ml);
--    EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
++TEST_F(AsioMainLoopTest, unregister_prevents_callback_and_does_not_harm_other_callbacks)
++{
++    mt::Pipe p1, p2;
++    char const data_to_write{'a'};
++    int p1_handler_never_triggers{-1};
++    int p2_handler_executes{-1};
++    char data_read{0};
++
++    ml.register_fd_handler(
++        {p1.read_fd()},
++        this,
++        [&p1_handler_never_triggers](int fd)
++        {
++            p1_handler_never_triggers = fd;
++        });
++
++    ml.register_fd_handler(
++        {p2.read_fd()},
++        this+2,
++        [&p2_handler_executes,&data_read,this](int fd)
++        {
++            p2_handler_executes = fd;
++            EXPECT_EQ(1, read(fd, &data_read, 1));
++            ml.stop();
++        });
++
++    ml.unregister_fd_handler(this);
++
++    EXPECT_EQ(-1, send(p1.write_fd(), &data_to_write, 1, MSG_NOSIGNAL));
++    EXPECT_EQ(1, write(p2.write_fd(), &data_to_write, 1));
++
++    ml.run();
++
++    EXPECT_EQ(data_to_write, data_read);
++    EXPECT_EQ(-1, p1_handler_never_triggers);
++    EXPECT_EQ(p2.read_fd(), p2_handler_executes);
+ }
  TEST_F(AsioMainLoopTest, dispatches_action)
@@ -718,7 +519,7 @@
+         {
              int const id = 0;
              actions.push_back(id);
--
++
              for (int i = 1; i < num_actions; ++i)
+             {
                  ml.enqueue(
 === added file 'tests/unit-tests/test_asio_server_action_queue.cpp'
 --- tests/unit-tests/test_asio_server_action_queue.cpp	1970-01-01 00:00:00 +0000
 +++ tests/unit-tests/test_asio_server_action_queue.cpp	2014-06-16 22:41:10 +0000
@@ -0,0 +1,219 @@
++/*
++ * Copyright © 2013 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: Alexandros Frantzis <alexandros.frantzis@canonical.com>
++ */
++
++#include "src/server/asio_server_action_queue.h"
++
++#include "mir_test/pipe.h"
++#include "mir_test/auto_unblock_thread.h"
++#include "mir_test/wait_object.h"
++
++#include <gtest/gtest.h>
++#include <gmock/gmock.h>
++
++#include <thread>
++#include <atomic>
++#include <functional>
++#include <mutex>
++#include <condition_variable>
++#include <array>
++#include <boost/throw_exception.hpp>
++
++#include <sys/types.h>
++#include <unistd.h>
++
++namespace mt = mir::test;
++
++namespace
++{
++
++class AsioServerActionQueueTest : public ::testing::Test
++{
++public:
++    boost::asio::io_service service;
++    mir::AsioServerActionQueue queue{service};
++};
++
++}
++
++TEST_F(AsioServerActionQueueTest, dispatches_action)
++{
++    using namespace testing;
++
++    int num_actions{0};
++    int const owner{0};
++
++    queue.enqueue(
++        &owner,
++        [&]
++        {
++            ++num_actions;
++            service.stop();
++        });
++
++    service.run();
++
++    EXPECT_THAT(num_actions, Eq(1));
++}
++
++TEST_F(AsioServerActionQueueTest, dispatches_multiple_actions_in_order)
++{
++    using namespace testing;
++
++    int const num_actions{5};
++    std::vector<int> actions;
++    int const owner{0};
++
++    for (int i = 0; i < num_actions; ++i)
++    {
++        queue.enqueue(
++            &owner,
++            [&,i]
++            {
++                actions.push_back(i);
++                if (i == num_actions - 1)
++                    service.stop();
++            });
++    }
++
++    service.run();
++
++    ASSERT_THAT(actions.size(), Eq(num_actions));
++    for (int i = 0; i < num_actions; ++i)
++        EXPECT_THAT(actions[i], Eq(i)) << "i = " << i;
++}
++
++TEST_F(AsioServerActionQueueTest, does_not_dispatch_paused_actions)
++{
++    using namespace testing;
++
++    std::vector<int> actions;
++    int const owner1{0};
++    int const owner2{0};
++
++    queue.enqueue(
++        &owner1,
++
++        [&]
++        {
++            int const id = 0;
++            actions.push_back(id);
++        });
++
++    queue.enqueue(
++        &owner2,
++        [&]
++        {
++            int const id = 1;
++            actions.push_back(id);
++        });
++
++    queue.enqueue(
++        &owner1,
++        [&]
++        {
++            int const id = 2;
++            actions.push_back(id);
++        });
++
++    queue.enqueue(
++        &owner2,
++        [&]
++        {
++            int const id = 3;
++            actions.push_back(id);
++            service.stop();
++        });
++
++    queue.pause_processing_for(&owner1);
++
++    service.run();
++
++    ASSERT_THAT(actions.size(), Eq(2));
++    EXPECT_THAT(actions[0], Eq(1));
++    EXPECT_THAT(actions[1], Eq(3));
++}
++
++TEST_F(AsioServerActionQueueTest, dispatches_resumed_actions)
++{
++    using namespace testing;
++
++    std::vector<int> actions;
++    void const* const owner1_ptr{&actions};
++    int const owner2{0};
++
++    queue.enqueue(
++        owner1_ptr,
++        [&]
++        {
++            int const id = 0;
++            actions.push_back(id);
++            service.stop();
++        });
++
++    queue.enqueue(
++        &owner2,
++        [&]
++        {
++            int const id = 1;
++            actions.push_back(id);
++            queue.resume_processing_for(owner1_ptr);
++        });
++
++    queue.pause_processing_for(owner1_ptr);
++
++    service.run();
++
++    ASSERT_THAT(actions.size(), Eq(2));
++    EXPECT_THAT(actions[0], Eq(1));
++    EXPECT_THAT(actions[1], Eq(0));
++}
++
++TEST_F(AsioServerActionQueueTest, handles_enqueue_from_within_action)
++{
++    using namespace testing;
++
++    std::vector<int> actions;
++    int const num_actions{10};
++    void const* const owner{&num_actions};
++
++    queue.enqueue(
++        owner,
++        [&]
++        {
++            int const id = 0;
++            actions.push_back(id);
++
++            for (int i = 1; i < num_actions; ++i)
++            {
++                queue.enqueue(
++                    owner,
++                    [&,i]
++                    {
++                        actions.push_back(i);
++                        if (i == num_actions - 1)
++                            service.stop();
++                    });
++            }
++        });
++
++    service.run();
++
++    ASSERT_THAT(actions.size(), Eq(num_actions));
++    for (int i = 0; i < num_actions; ++i)
++        EXPECT_THAT(actions[i], Eq(i)) << "i = " << i;
++}
 === added file 'tests/unit-tests/test_asio_timer_service.cpp'
 --- tests/unit-tests/test_asio_timer_service.cpp	1970-01-01 00:00:00 +0000
 +++ tests/unit-tests/test_asio_timer_service.cpp	2014-06-16 22:41:10 +0000
@@ -0,0 +1,312 @@
++/*
++ * Copyright © 2014 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: Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#include "src/server/asio_timer_service.h"
++
++#include "mir/time/high_resolution_clock.h"
++#include "mir_test/auto_unblock_thread.h"
++#include "mir_test/wait_object.h"
++
++#include <gtest/gtest.h>
++#include <gmock/gmock.h>
++
++#include <thread>
++#include <atomic>
++#include <functional>
++#include <mutex>
++#include <condition_variable>
++#include <array>
++#include <boost/throw_exception.hpp>
++
++#include <sys/types.h>
++#include <unistd.h>
++
++namespace mt = mir::test;
++
++namespace
++{
++
++class AdvanceableClock : public mir::time::Clock
++{
++public:
++    mir::time::Timestamp sample() const override
++    {
++        std::lock_guard<std::mutex> lock(time_mutex);
++        return current_time;
++    }
++    void advance_by(std::chrono::milliseconds const step, mir::AsioTimerService & timer_service)
++    {
++        {
++            std::lock_guard<std::mutex> lock(time_mutex);
++            current_time += step;
++        }
++
++        bool done = false;
++        auto evaluate_clock_alarm = timer_service.notify_in(
++            std::chrono::milliseconds{0},
++            [&done, this]
++            {
++                std::unique_lock<std::mutex> lock(update_time_checkpoint_mutex);
++                done = true;
++                update_time_checkpoint.notify_one();
++            });
++
++        std::unique_lock<std::mutex> lock(update_time_checkpoint_mutex);
++        while(!done) update_time_checkpoint.wait(lock);
++    }
++
++private:
++    mutable std::mutex time_mutex;
++
++    std::mutex update_time_checkpoint_mutex;
++    std::condition_variable update_time_checkpoint;
++
++
++    mir::time::Timestamp current_time{
++        []
++        {
++           mir::time::HighResolutionClock clock;
++           return clock.sample();
++        }()
++        };
++};
++
++
++class AsioTimerServiceTest : public ::testing::Test
++{
++public:
++    std::shared_ptr<AdvanceableClock> clock = std::make_shared<AdvanceableClock>();
++    mir::AsioTimerService timer_service{clock};
++    int call_count{0};
++    mt::WaitObject wait;
++    std::chrono::milliseconds delay{50};
++
++    struct UnblockTimerService : mt::AutoUnblockThread
++    {
++        UnblockTimerService(mir::AsioTimerService & timer_service)
++            : mt::AutoUnblockThread([&timer_service]() {timer_service.stop();},
++                                    [&timer_service]() {timer_service.run();})
++        {}
++    };
++};
++
++class Counter
++{
++public:
++    int operator++()
++    {
++        std::lock_guard<decltype(mutex)> lock(mutex);
++        cv.notify_one();
++        return ++counter;
++    }
++
++    bool wait_for(std::chrono::milliseconds const& delay, int expected)
++    {
++        std::unique_lock<decltype(mutex)> lock(mutex);
++        return cv.wait_for(lock, delay, [&]{ return counter == expected;});
++    }
++
++    operator int() const
++    {
++        std::lock_guard<decltype(mutex)> lock(mutex);
++        return counter;
++    }
++
++private:
++    std::mutex mutable mutex;
++    std::condition_variable cv;
++    int counter{0};
++};
++
++}
++
++TEST_F(AsioTimerServiceTest, runs_until_stopped)
++{
++    std::mutex checkpoint_mutex;
++    std::condition_variable checkpoint;
++    bool hit_checkpoint{false};
++
++    auto fire_on_timer_service_start = timer_service.notify_in(
++        std::chrono::milliseconds{0},
++        [&checkpoint_mutex, &checkpoint, &hit_checkpoint]()
++        {
++            std::unique_lock<decltype(checkpoint_mutex)> lock(checkpoint_mutex);
++            hit_checkpoint = true;
++            checkpoint.notify_all();
++        });
++
++    UnblockTimerService unblocker(timer_service);
++
++    // TODO time dependency:
++    {
++        std::unique_lock<decltype(checkpoint_mutex)> lock(checkpoint_mutex);
++        ASSERT_TRUE(checkpoint.wait_for(lock, std::chrono::milliseconds{500}, [&hit_checkpoint]() { return hit_checkpoint; }));
++    }
++
++    auto alarm = timer_service.notify_in(std::chrono::milliseconds{10}, [this]
++    {
++        wait.notify_ready();
++    });
++
++    clock->advance_by(std::chrono::milliseconds{10}, timer_service);
++    EXPECT_NO_THROW(wait.wait_until_ready(std::chrono::milliseconds{500}));
++
++    timer_service.stop();
++    // Timer Service should be stopped now
++
++    hit_checkpoint = false;
++    auto should_not_fire =  timer_service.notify_in(std::chrono::milliseconds{0},
++                                         [&checkpoint_mutex, &checkpoint, &hit_checkpoint]()
++    {
++        std::unique_lock<decltype(checkpoint_mutex)> lock(checkpoint_mutex);
++        hit_checkpoint = true;
++        checkpoint.notify_all();
++    });
++
++    std::unique_lock<decltype(checkpoint_mutex)> lock(checkpoint_mutex);
++    EXPECT_FALSE(checkpoint.wait_for(lock, std::chrono::milliseconds{50}, [&hit_checkpoint]() { return hit_checkpoint; }));
++}
++
++TEST_F(AsioTimerServiceTest, alarm_starts_in_pending_state)
++{
++    auto alarm = timer_service.notify_in(delay, [this]() {});
++
++    UnblockTimerService unblocker(timer_service);
++
++    EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
++}
++
++TEST_F(AsioTimerServiceTest, alarm_fires_with_correct_delay)
++{
++    auto alarm = timer_service.notify_in(delay, [](){});
++
++    UnblockTimerService unblocker(timer_service);
++
++    clock->advance_by(delay - std::chrono::milliseconds{1}, timer_service);
++    EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
++
++    clock->advance_by(delay, timer_service);
++    EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
++}
++
++TEST_F(AsioTimerServiceTest, multiple_alarms_fire)
++{
++    using namespace testing;
++
++    int const alarm_count{10};
++    Counter call_count;
++    std::array<std::unique_ptr<mir::time::Alarm>, alarm_count> alarms;
++
++    for (auto& alarm : alarms)
++        alarm = timer_service.notify_in(delay, [&call_count](){++call_count;});
++
++    UnblockTimerService unblocker(timer_service);
++    clock->advance_by(delay, timer_service);
++
++    call_count.wait_for(delay, alarm_count);
++    EXPECT_THAT(call_count, Eq(alarm_count));
++
++    for (auto const& alarm : alarms)
++        EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
++}
++
++TEST_F(AsioTimerServiceTest, cancelled_alarm_doesnt_fire)
++{
++    UnblockTimerService unblocker(timer_service);
++    auto alarm = timer_service.notify_in(std::chrono::milliseconds{100},
++                                         [](){ FAIL() << "Alarm handler of canceld alarm called";});
++
++    EXPECT_TRUE(alarm->cancel());
++
++    EXPECT_EQ(mir::time::Alarm::cancelled, alarm->state());
++
++    clock->advance_by(std::chrono::milliseconds{100}, timer_service);
++
++    EXPECT_EQ(mir::time::Alarm::cancelled, alarm->state());
++}
++
++TEST_F(AsioTimerServiceTest, destroyed_alarm_doesnt_fire)
++{
++    auto alarm = timer_service.notify_in(std::chrono::milliseconds{200},
++                                         [](){ FAIL() << "Alarm handler of destroyed alarm called"; });
++
++    UnblockTimerService unblocker(timer_service);
++
++    alarm.reset(nullptr);
++    clock->advance_by(std::chrono::milliseconds{200}, timer_service);
++}
++
++TEST_F(AsioTimerServiceTest, rescheduled_alarm_fires_again)
++{
++    std::atomic<int> call_count{0};
++
++    auto alarm = timer_service.notify_in(std::chrono::milliseconds{0}, [&call_count]()
++    {
++        if (call_count++ > 1)
++            FAIL() << "Alarm called too many times";
++    });
++
++    UnblockTimerService unblocker(timer_service);
++
++    clock->advance_by(std::chrono::milliseconds{0}, timer_service);
++    ASSERT_EQ(mir::time::Alarm::triggered, alarm->state());
++
++    alarm->reschedule_in(std::chrono::milliseconds{100});
++    EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
++
++    clock->advance_by(std::chrono::milliseconds{100}, timer_service);
++    EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
++}
++
++TEST_F(AsioTimerServiceTest, rescheduled_alarm_cancels_previous_scheduling)
++{
++    std::atomic<int> call_count{0};
++
++    auto alarm = timer_service.notify_in(std::chrono::milliseconds{100}, [&call_count]()
++    {
++        call_count++;
++    });
++
++    UnblockTimerService unblocker(timer_service);
++    clock->advance_by(std::chrono::milliseconds{90}, timer_service);
++
++    EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
++    EXPECT_EQ(0, call_count);
++    EXPECT_TRUE(alarm->reschedule_in(std::chrono::milliseconds{100}));
++    EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
++
++    clock->advance_by(std::chrono::milliseconds{110}, timer_service);
++
++    EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
++    EXPECT_EQ(1, call_count);
++}
++
++TEST_F(AsioTimerServiceTest, alarm_fires_at_correct_time_point)
++{
++    mir::time::Timestamp real_soon = clock->sample() + std::chrono::milliseconds{120};
++
++    auto alarm = timer_service.notify_at(real_soon, []{});
++
++    UnblockTimerService unblocker(timer_service);
++
++    clock->advance_by(std::chrono::milliseconds{119}, timer_service);
++    EXPECT_EQ(mir::time::Alarm::pending, alarm->state());
++
++    clock->advance_by(std::chrono::milliseconds{1}, timer_service);
++    EXPECT_EQ(mir::time::Alarm::triggered, alarm->state());
++}
 === added file 'tests/unit-tests/test_synchronous_server_action.cpp'
 --- tests/unit-tests/test_synchronous_server_action.cpp	1970-01-01 00:00:00 +0000
 +++ tests/unit-tests/test_synchronous_server_action.cpp	2014-06-16 22:41:10 +0000
@@ -0,0 +1,78 @@
++/*
++ * Copyright © 2014 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: Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#include "src/server/synchronous_server_action.h"
++#include "mir_test_doubles/mock_server_action_queue.h"
++
++#include <thread>
++
++namespace mtd = mir::test::doubles;
++
++TEST(SynchronousServerActionTest, just_executes_action_if_queue_is_not_running)
++{
++    using namespace ::testing;
++
++    NiceMock<mtd::MockServerActionQueue> mock_queue;
++    EXPECT_CALL(mock_queue, enqueue(_,_)).Times(0);
++    int val = 0;
++    boost::optional<std::thread::id> empty_queue_thread_id;
++    {
++        mir::SynchronousServerAction action(mock_queue, empty_queue_thread_id, [&val]{val=1;});
++    }
++    EXPECT_EQ(val, 1);
++}
++
++TEST(SynchronousServerActionTest, just_executes_action_if_thread_identical)
++{
++    using namespace ::testing;
++
++    NiceMock<mtd::MockServerActionQueue> mock_queue;
++    EXPECT_CALL(mock_queue, enqueue(_,_)).Times(0);
++    int val = 0;
++    boost::optional<std::thread::id> this_one{std::this_thread::get_id()};
++    {
++        mir::SynchronousServerAction action(mock_queue, this_one, [&val]{val=1;});
++    }
++    EXPECT_EQ(val, 1);
++}
++
++TEST(SynchronousServerActionTest, enqueues_action_if_thread_differs)
++{
++    using namespace ::testing;
++
++    class DirectlyExecutingServerActionQueue : public StrictMock<mtd::MockServerActionQueue>
++    {
++        void enqueue(void const* ptr, mir::ServerAction const& action) override
++        {
++            action();
++            StrictMock<mtd::MockServerActionQueue>::enqueue(ptr, action);
++        }
++    };
++
++    DirectlyExecutingServerActionQueue execute_it;
++    EXPECT_CALL(execute_it, enqueue(_, _));
++
++    int val = 0;
++    std::thread::id arbitrary_thread;
++    boost::optional<std::thread::id> not_this_one{arbitrary_thread};
++
++    {
++        mir::SynchronousServerAction action(execute_it, not_this_one, [&val]{val=1;});
++    }
++    EXPECT_EQ(1,val);
++}