Code review comment for lp:~albaguirre/mir/fix-tsan-findings

I'd really like to approve a lot of these changes as they are obviously correct.

But the rework of CompositingFunctor::operator() isn't...

~~~~

+ //Appease TSan, avoid destructor and this thread accessing the same shared_ptr instance
+ auto disp_listener = std::move(display_listener);

As display_listener is const, isn't this simpler as:

    auto const disp_listener = display_listener;

Or even better, just make it a value capture in the lines below? Vis:

        auto display_registration = mir::raii::paired_calls(
            [display_listener=display_listener, &group=group]{group.for_each_display_buffer([&](mg::DisplayBuffer& buffer)
                { display_listener->add_display(buffer.view_area()); });},
            [display_listener=display_listener, &group=group]{group.for_each_display_buffer([&](mg::DisplayBuffer& buffer)
                { display_listener->remove_display(buffer.view_area()); });});

~~~~

- auto on_startup_failure = on_unwind(
- [this]
- {
- if (started_future.wait_for(0s) != std::future_status::ready)
- started.set_exception(std::current_exception());
- });
-
...
     catch(...)
     {
+ try
+ {
+ //Move the promise so that the promise destructor occurs here rather than in the thread
+ //destroying CompositingFunctor, mostly to appease TSan
+ auto promise = std::move(started);
+ promise.set_exception(std::current_exception());
+ }
+ catch(...)
+ {
+ }

So we no longer use "on_unwind()" because the destructor of the moved promise will throw after we set_exception()? And then we eat the exception anyway? That seems an overly complex mechanism to destroy CompositingFunctor::promise.