Display server main control loop

[Thomas Voß]

On 03.04.2013 17:51, Alexandros Frantzis wrote:
> On Tue, Apr 02, 2013 at 03:40:45PM +0300, Alexandros Frantzis wrote:
>> Hi all,
>>
>> while working on adding VT switching I noticed that the code started to
>> become more complicated, as I needed to handle asynchronous events (a
>> signal in this case) and coordinate cross-component operations.
>>
>> One way to improve the situation is to add a main control loop to the
>> display server. This is *not* meant to be a loop for all kinds of events
>> (e.g.  input, graphics), only for external asynchronous events that need
>> to change the state of the main components of the display server in a
>> coordinated manner.
>>
>> Such events are:
>>
>> * Quitting (signals)
>> * VT switching (and more generally, display server pause/resume) (signals)
>> * Display reconfiguration (e.g. watching a udev FD on the desktop)
>>
>> Although, we could conceivably handle these in a distributed manner, I
>> feel we would be adding a lot of complexity and fragility to the system.
>>
>> There are a couple of mechanisms we could use to implement a main
>> control loop and that could handle events from all our sources (signals
>> and FDs for now).  We could use boost asio, or we could implement our
>> own mechanism, using signalfd to route the signals to an fd we can
>> select() on.
>>
>> The main loop could live in the DisplayServer::run() method, replacing
>> the wait on the exit_cv condition variable that is currently there, or
>> it could run in a separate thread like SignalDispatcher currently does.
>> I prefer the first approach since the main display server thread is idle
>> anyway, and it's a natural place for such an event loop.
>>
>> Thoughts? Any other events of this kind that we should be aware of?
>>
>> Thanks,
>> Alexandros
>>
> 
> To get a better feeling of what's involved, I created a prototype
> abstraction for a main loop and an implementation using boost asio.
> Get it and use it with:
> 
> $ bzr branch lp:~afrantzis/+junk/mainloop
> $ cd mainloop && make
> $ ./mainloop
> 
> The main loop abstraction was designed so that event creation and
> handling are completely dissociated. The display server (or other
> consumer) can handle an event without caring if it is coming from a
> signal, a read from a fd, or another mechanism.  So, we could have
> something like this in the mir main loop:
> 

This looks pretty good to me, some thoughts on the separation of
concerns, though:
> auto main_loop = main_loop_factory->create_main_loop();
> 
> auto pause_event = platform->create_pause_event(main_loop_factory);

I would rather prefer:

PauseEvent pause_event(main_loop);
pause_event.async_wait(pause_handler);

QuitEvent quit_event(main_loop);
quit_event.async_wait(quit_handler);

DisplayConfigurationEvent disp_config_event(main_loop);
disp_config_event.async_wait(disp_conf_handler);

See boost::asio::signal_set as an example: Different parties interested
in a certain set of signals just setup their respective signal set and
execute an (async) wait on the respective set. Behind the scenes, the
signal_set class sets up all of the handlers, and associates all of the
operation with one io_service, which forces handler execution to the
thread that the io_service.run method is executed on.

To this end, a mir::EventService would need to be created, and the
respective events, including custom handler types (not only void()), are
known to the EventService. On startup, the service is registered with
the io_service representing the main event loop with
boost::asio::add_service<mir::EventService>(io_service).

A class QuitEvent would then do:

class QuitEvent
{
public:
	QuitEvent(const std::shared_ptr<mir::MainLoop>& main_loop) :
io_service(main_loop->io_service()) {}

	void async_wait(QuitEventHandler handler)
	{
	
boost::asio::use_service<mir::EventService>(io_service).enqueue_handler_for_quit_event(handler);
	}
private:
	boost::asio::io_service& io_service;
};

>From my perspective, keeping the handler definition local, without
encoding everything within the main loop avoids tight coupling, and the
main loop is stripped down to only the synchronization concern.

Makes sense?

Thanks,

  Thomas

> auto disp_conf_event = platform->create_display_configuration_event(main_loop_factory);
> auto quit_event = main_loop_factory->create_signal_event(SIGINT);
> 
> main_loop->register_event_handler(pause_event, pause_handler);
> main_loop->register_event_handler(disp_conf_event, disp_conf_handler);
> main_loop->register_event_handler(quit_event, [main_loop]{main_loop->stop();});
> 
> main_loop->run();
> 
> Thoughts?
> 
> Thanks,
> Alexandros
>