From 20f961cbd8666fba6ae7c014c687e5f453645e59 Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Tue, 15 Sep 2015 15:37:42 +0200 Subject: [PATCH] cli_monitor, launcher: handle exiting subsystems Until now, the CLI monitor and the laucher allowed the user to explitly kill subsystems but both used to ignore gracefully exiting subsystems. It was the user's job to remove the remains of those subsystems. The patch takes the burden of manually killing exited subsystems from the user. Fixes #1685 --- repos/gems/src/app/launcher/main.cc | 11 ++++++- repos/gems/src/app/launcher/menu_dialog.h | 5 +-- .../gems/src/app/launcher/subsystem_manager.h | 32 ++++++++++++++++--- repos/os/include/cli_monitor/child.h | 26 +++++++++++++-- repos/os/src/app/cli_monitor/child.h | 6 ++-- repos/os/src/app/cli_monitor/gdb_command.h | 15 ++++++--- repos/os/src/app/cli_monitor/main.cc | 22 +++++++++++-- repos/os/src/app/cli_monitor/start_command.h | 9 ++++-- 8 files changed, 105 insertions(+), 21 deletions(-) diff --git a/repos/gems/src/app/launcher/main.cc b/repos/gems/src/app/launcher/main.cc index d7ef59362..f0dc78f8b 100644 --- a/repos/gems/src/app/launcher/main.cc +++ b/repos/gems/src/app/launcher/main.cc @@ -49,7 +49,10 @@ struct Launcher::Main */ Nitpicker::Connection nitpicker; - Subsystem_manager subsystem_manager { ep, cap }; + Genode::Signal_rpc_member
_exited_child_dispatcher = + { ep, *this, &Main::_handle_exited_child }; + + Subsystem_manager subsystem_manager { ep, cap, _exited_child_dispatcher }; Menu_dialog menu_dialog { ep, cap, *env()->ram_session(), report_rom_slave, subsystem_manager, nitpicker }; @@ -68,6 +71,12 @@ struct Launcher::Main void handle_xray_update(unsigned); + void _handle_exited_child(unsigned) + { + auto kill_child_fn = [&] (Child_base::Label label) { menu_dialog.kill(label); }; + + subsystem_manager.for_each_exited_child(kill_child_fn); + } /** * Constructor diff --git a/repos/gems/src/app/launcher/menu_dialog.h b/repos/gems/src/app/launcher/menu_dialog.h index 0ff7e137e..c8e8ef74f 100644 --- a/repos/gems/src/app/launcher/menu_dialog.h +++ b/repos/gems/src/app/launcher/menu_dialog.h @@ -325,10 +325,9 @@ class Launcher::Menu_dialog : Input_event_handler, Dialog_generator, if (!_running(hovered)) _start(hovered); - } else { - _touch(""); } + _touch(""); _clicked = Label(""); _click_in_progress = false; } @@ -360,6 +359,8 @@ class Launcher::Menu_dialog : Input_event_handler, Dialog_generator, _context_dialog.visible(false); } + void kill(Child_base::Label const &label) { _kill(label); } + void update() { if (_elements.first()) { diff --git a/repos/gems/src/app/launcher/subsystem_manager.h b/repos/gems/src/app/launcher/subsystem_manager.h index 3f90f4fb9..586acd714 100644 --- a/repos/gems/src/app/launcher/subsystem_manager.h +++ b/repos/gems/src/app/launcher/subsystem_manager.h @@ -73,7 +73,8 @@ class Launcher::Subsystem_manager Cap_session &cap_session, size_t ram_quota, size_t ram_limit, - Signal_context_capability yield_response_sig_cap) + Signal_context_capability yield_response_sig_cap, + Signal_context_capability exit_sig_cap) : Child_base(ram, label.string(), @@ -81,7 +82,8 @@ class Launcher::Subsystem_manager cap_session, ram_quota, ram_limit, - yield_response_sig_cap) + yield_response_sig_cap, + exit_sig_cap) { } }; @@ -139,6 +141,8 @@ class Launcher::Subsystem_manager _try_response_to_resource_request(); } + Genode::Signal_context_capability _exited_child_sig_cap; + Ram _ram { ram_preservation_from_config(), _yield_broadcast_dispatcher, _resource_avail_dispatcher }; @@ -179,9 +183,10 @@ class Launcher::Subsystem_manager public: - Subsystem_manager(Server::Entrypoint &ep, Cap_session &cap) + Subsystem_manager(Server::Entrypoint &ep, Cap_session &cap, + Genode::Signal_context_capability exited_child_sig_cap) : - _ep(ep), _cap(cap) + _ep(ep), _cap(cap), _exited_child_sig_cap(exited_child_sig_cap) { } /** @@ -204,7 +209,8 @@ class Launcher::Subsystem_manager Child *child = new (env()->heap()) Child(_ram, label, binary_name.string(), _cap, ram_config.quantum, ram_config.limit, - _yield_broadcast_dispatcher); + _yield_broadcast_dispatcher, + _exited_child_sig_cap); /* configure child */ try { @@ -232,6 +238,22 @@ class Launcher::Subsystem_manager } } } + + /** + * Call functor for each exited child + * + * The functor takes a 'Child_base::Label' as argument. + */ + template + void for_each_exited_child(FUNC const &func) + { + Child *next = nullptr; + for (Child *child = _children.first(); child; child = next) { + next = child->next(); + if (child->exited()) + func(child->label()); + } + } }; #endif /* _SUBSYSTEM_MANAGER_H_ */ diff --git a/repos/os/include/cli_monitor/child.h b/repos/os/include/cli_monitor/child.h index 71826c335..bd9bec541 100644 --- a/repos/os/include/cli_monitor/child.h +++ b/repos/os/include/cli_monitor/child.h @@ -26,6 +26,7 @@ /* CLI-monitor includes */ #include + class Child_base : public Genode::Child_policy { public: @@ -94,6 +95,11 @@ class Child_base : public Genode::Child_policy Genode::Signal_context_capability _yield_response_sigh_cap; + Genode::Signal_context_capability _exit_sig_cap; + + /* true if child is scheduled for destruction */ + bool _exited = false; + public: Child_base(Ram &ram, @@ -102,7 +108,8 @@ class Child_base : public Genode::Child_policy Genode::Cap_session &cap_session, Genode::size_t ram_quota, Genode::size_t ram_limit, - Genode::Signal_context_capability yield_response_sig_cap) + Genode::Signal_context_capability yield_response_sig_cap, + Genode::Signal_context_capability exit_sig_cap) : _ram(ram), _label(label), @@ -117,7 +124,8 @@ class Child_base : public Genode::Child_policy _child(_binary_rom.dataspace(), _resources.pd.cap(), _resources.ram.cap(), _resources.cpu.cap(), _resources.rm.cap(), &_entrypoint, this), - _yield_response_sigh_cap(yield_response_sig_cap) + _yield_response_sigh_cap(yield_response_sig_cap), + _exit_sig_cap(exit_sig_cap) { } Label label() const { return _label; } @@ -252,6 +260,11 @@ class Child_base : public Genode::Child_policy requested_ram_quota()); } + /** + * Return true if child exited and should be destructed + */ + bool exited() const { return _exited; } + /**************************** ** Child_policy interface ** @@ -311,6 +324,15 @@ class Child_base : public Genode::Child_policy _resource_args = args; try_response_to_resource_request(); } + + void exit(int exit_value) override + { + PINF("subsystem \"%s\" exited with value %d", name(), exit_value); + _exited = true; + + /* trigger destruction of the child */ + Genode::Signal_transmitter(_exit_sig_cap).submit(); + } }; #endif /* _INCLUDE__CLI_MONITOR__CHILD_H_ */ diff --git a/repos/os/src/app/cli_monitor/child.h b/repos/os/src/app/cli_monitor/child.h index f06a86029..cd66d2a58 100644 --- a/repos/os/src/app/cli_monitor/child.h +++ b/repos/os/src/app/cli_monitor/child.h @@ -30,7 +30,8 @@ struct Child : Child_base, List::Element Genode::Cap_session &cap_session, Genode::size_t ram_quota, Genode::size_t ram_limit, - Genode::Signal_context_capability yield_response_sig_cap) + Genode::Signal_context_capability yield_response_sig_cap, + Genode::Signal_context_capability exit_sig_cap) : Child_base(ram, label, @@ -38,7 +39,8 @@ struct Child : Child_base, List::Element cap_session, ram_quota, ram_limit, - yield_response_sig_cap), + yield_response_sig_cap, + exit_sig_cap), argument(label, "subsystem") { } }; diff --git a/repos/os/src/app/cli_monitor/gdb_command.h b/repos/os/src/app/cli_monitor/gdb_command.h index 4f87a4af9..d52ff0f6e 100644 --- a/repos/os/src/app/cli_monitor/gdb_command.h +++ b/repos/os/src/app/cli_monitor/gdb_command.h @@ -36,6 +36,8 @@ class Gdb_command_child : public Child private: Genode::Signal_context_capability _kill_gdb_sig_cap; + Genode::Signal_context_capability _exit_sig_cap; + Terminal::Session &_terminal; bool _kill_requested; @@ -56,10 +58,12 @@ class Gdb_command_child : public Child Genode::size_t ram_limit, Genode::Signal_context_capability yield_response_sig_cap, Genode::Signal_context_capability kill_gdb_sig_cap, + Genode::Signal_context_capability exit_sig_cap, Terminal::Session &terminal) : Child(ram, label, binary, cap_session, ram_quota, ram_limit, - yield_response_sig_cap), + yield_response_sig_cap, exit_sig_cap), _kill_gdb_sig_cap(kill_gdb_sig_cap), + _exit_sig_cap(exit_sig_cap), _terminal(terminal), _kill_requested(false) { } @@ -138,6 +142,7 @@ class Gdb_command : public Command Subsystem_config_registry &_subsystem_configs; Signal_context_capability _yield_response_sigh_cap; Signal_context_capability _kill_gdb_sig_cap; + Signal_context_capability _exit_sig_cap; /** * Generate the config node for the GDB subsystem @@ -412,7 +417,7 @@ class Gdb_command : public Command child = new (Genode::env()->heap()) Gdb_command_child(_ram, label, "init", _cap, ram, ram_limit, _yield_response_sigh_cap, _kill_gdb_sig_cap, - terminal); + _exit_sig_cap, terminal); /* configure child */ try { @@ -455,13 +460,15 @@ class Gdb_command : public Command Gdb_command(Ram &ram, Genode::Cap_session &cap, Child_registry &children, Subsystem_config_registry &subsustem_configs, Signal_context_capability yield_response_sigh_cap, - Signal_context_capability kill_gdb_sig_cap) + Signal_context_capability kill_gdb_sig_cap, + Signal_context_capability exit_sig_cap) : Command("gdb", "create new subsystem with GDB"), _ram(ram), _children(children), _cap(cap), _subsystem_configs(subsustem_configs), _yield_response_sigh_cap(yield_response_sigh_cap), - _kill_gdb_sig_cap(kill_gdb_sig_cap) + _kill_gdb_sig_cap(kill_gdb_sig_cap), + _exit_sig_cap(exit_sig_cap) { add_parameter(new Parameter("--ram", Parameter::NUMBER, "initial RAM quota")); add_parameter(new Parameter("--ram-limit", Parameter::NUMBER, "limit for expanding RAM quota")); diff --git a/repos/os/src/app/cli_monitor/main.cc b/repos/os/src/app/cli_monitor/main.cc index 64df9e700..09f0e7741 100644 --- a/repos/os/src/app/cli_monitor/main.cc +++ b/repos/os/src/app/cli_monitor/main.cc @@ -131,6 +131,10 @@ int main(int argc, char **argv) static Signal_context_capability kill_gdb_sig_cap = sig_rec.manage(&kill_gdb_sig_ctx); + static Signal_context exited_child_sig_ctx; + static Signal_context_capability exited_child_sig_cap = + sig_rec.manage(&exited_child_sig_ctx); + static Ram ram(ram_preservation_from_config(), sig_rec.manage(&yield_broadcast_sig_ctx), sig_rec.manage(&resource_avail_sig_ctx)); @@ -142,10 +146,12 @@ int main(int argc, char **argv) commands.insert(new Gdb_command(ram, cap, children, subsystem_config_registry(), yield_response_sig_cap, - kill_gdb_sig_cap)); + kill_gdb_sig_cap, + exited_child_sig_cap)); commands.insert(new Start_command(ram, cap, children, subsystem_config_registry(), - yield_response_sig_cap)); + yield_response_sig_cap, + exited_child_sig_cap)); commands.insert(new Status_command(ram, children)); commands.insert(new Yield_command(children)); commands.insert(new Ram_command(children)); @@ -212,6 +218,18 @@ int main(int argc, char **argv) continue; } + if (signal.context() == &exited_child_sig_ctx) { + Child *next = nullptr; + for (Child *child = children.first(); child; child = next) { + next = child->next(); + if (child->exited()) { + children.remove(child); + Genode::destroy(Genode::env()->heap(), child); + } + } + continue; + } + if (!line_editor.is_complete()) continue; diff --git a/repos/os/src/app/cli_monitor/start_command.h b/repos/os/src/app/cli_monitor/start_command.h index 48bf65249..21ce27b0b 100644 --- a/repos/os/src/app/cli_monitor/start_command.h +++ b/repos/os/src/app/cli_monitor/start_command.h @@ -33,6 +33,7 @@ class Start_command : public Command Subsystem_config_registry &_subsystem_configs; List _arguments; Signal_context_capability _yield_response_sigh_cap; + Signal_context_capability _exit_sig_cap; void _execute_subsystem(char const *name, Command_line &cmd, Terminal::Session &terminal, @@ -105,7 +106,7 @@ class Start_command : public Command try { child = new (Genode::env()->heap()) Child(_ram, label, binary_name, _cap, ram, ram_limit, - _yield_response_sigh_cap); + _yield_response_sigh_cap, _exit_sig_cap); } catch (Genode::Rom_connection::Rom_connection_failed) { tprintf(terminal, "Error: could not obtain ROM module \"%s\"\n", @@ -145,12 +146,14 @@ class Start_command : public Command Start_command(Ram &ram, Genode::Cap_session &cap, Child_registry &children, Subsystem_config_registry &subsustem_configs, - Signal_context_capability yield_response_sigh_cap) + Signal_context_capability yield_response_sigh_cap, + Signal_context_capability exit_sig_cap) : Command("start", "create new subsystem"), _ram(ram), _children(children), _cap(cap), _subsystem_configs(subsustem_configs), - _yield_response_sigh_cap(yield_response_sigh_cap) + _yield_response_sigh_cap(yield_response_sigh_cap), + _exit_sig_cap(exit_sig_cap) { add_parameter(new Parameter("--count", Parameter::NUMBER, "number of instances")); add_parameter(new Parameter("--ram", Parameter::NUMBER, "initial RAM quota"));