Code review comment for lp:~andreas-pokorny/mir/split-main-loop-and-fix-races

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...);