Code review comment for lp:~andreas-pokorny/mir/move-config-changer-to-input-thread

> + return
> std::make_shared<mi::ExternalInputDeviceHub>(the_default_input_device_hub(),
> + the_main_loop());
>
> The ExternalInputDeviceHub doesn't hold a strong reference to the
> the_default_input_device_hub(), so
> depending on the order of initialization (i.e., if we haven't already stored a
> strong reference to default_input_device_hub()) we will end up immediately
> destroying the default hub we just created.
>
> Perhaps this doesn't actually happen in our current configuration, but it is
> currently not easy to reason about the correctness of the lifecycle, and this
> could be a pain point in future refactorings.

Yes the actual input device hub is kept alive via loaded platform / input manager / DisplayServer and also via ipc factory / Connector / DisplayServer. I am using a weak_ptr to avoid a cycle between hub and observers.

> > void mi::ExternalInputDeviceHub::remove_observer
> > mi::ExternalInputDeviceHub::for_each_input_device
> > mi::ExternalInputDeviceHub::for_each_mutable_input_device
>
> In the above function we are using the result of weak_ptr.lock() without
> checking if it's valid. Is it safe to do so?

Changed that..

I think I could also wrap weak_ptr to make it match Element concept of ThreadSafeList