From 7e08bba25ce4e44fac345c27b8edfbf52098954e Mon Sep 17 00:00:00 2001 From: Emery Hemingway Date: Thu, 12 Jul 2018 17:49:56 +0200 Subject: [PATCH] Cached_fs_rom: fix congestion error When the cached_fs_rom saturates the packet stream of its File_system session it will call the session request handler recursively as pending transfers are completed. This is bad because the content of the XML node currently being processed will change. The session request handler can no longer be called directly, but the "schedule" method will submit a signal to the request handler, and requests will be processed after the current operation has completed. --- repos/os/src/server/cached_fs_rom/main.cc | 19 ++----- .../server/cached_fs_rom/session_requests.h | 57 ++++++++++++------- 2 files changed, 41 insertions(+), 35 deletions(-) diff --git a/repos/os/src/server/cached_fs_rom/main.cc b/repos/os/src/server/cached_fs_rom/main.cc index 55149f248..c812b76ab 100755 --- a/repos/os/src/server/cached_fs_rom/main.cc +++ b/repos/os/src/server/cached_fs_rom/main.cc @@ -310,18 +310,6 @@ struct Cached_fs_rom::Main final : Genode::Session_request_handler Io_signal_handler
packet_handler { env.ep(), *this, &Main::handle_packets }; - /** - * Signal handler to disable blocking behavior in 'Expanding_parent_client' - */ - Signal_handler
resource_handler { - env.ep(), *this, &Main::handle_resources }; - - /** - * Process requests again if parent gives us an upgrade - */ - void handle_resources() { - session_requests.process(); } - /** * Return true when a cache element is freed */ @@ -489,7 +477,7 @@ struct Cached_fs_rom::Main final : Genode::Session_request_handler { rom.process_packet(pkt); if (rom.completed()) - session_requests.process(); + session_requests.schedule(); stray_pkt = false; }); @@ -500,12 +488,13 @@ struct Cached_fs_rom::Main final : Genode::Session_request_handler Main(Genode::Env &env) : env(env) { - env.parent().resource_avail_sigh(resource_handler); + /* process the requests when more resources are made available */ + env.parent().resource_avail_sigh(session_requests); fs.sigh_ack_avail(packet_handler); /* process any requests that have already queued */ - session_requests.process(); + session_requests.schedule(); } }; diff --git a/repos/os/src/server/cached_fs_rom/session_requests.h b/repos/os/src/server/cached_fs_rom/session_requests.h index 3e59dcf9b..35d527281 100644 --- a/repos/os/src/server/cached_fs_rom/session_requests.h +++ b/repos/os/src/server/cached_fs_rom/session_requests.h @@ -35,7 +35,7 @@ struct Genode::Session_request_handler : Interface }; -class Genode::Session_requests_rom +class Genode::Session_requests_rom : public Signal_handler { private: @@ -44,21 +44,7 @@ class Genode::Session_requests_rom Attached_rom_dataspace _parent_rom; - Signal_handler _handler; - - public: - - Session_requests_rom(Genode::Env &env, - Session_request_handler &requests_handler) - : _parent(env.parent()), - _requests_handler(requests_handler), - _parent_rom(env, "session_requests"), - _handler(env.ep(), *this, &Session_requests_rom::process) - { - _parent_rom.sigh(_handler); - } - - void process() + void _process() { _parent_rom.update(); Xml_node requests = _parent_rom.xml(); @@ -69,10 +55,18 @@ class Genode::Session_requests_rom request.attribute_value("id", ~0UL) }; typedef Session_state::Name Name; - Name const name = request.attribute_value("service", Name()); - typedef Session_state::Args Args; - Args const args = request.sub_node("args").decoded_content(); + + Name name { }; + Args args { }; + + try { + name = request.attribute_value("service", Name()); + args = request.sub_node("args").decoded_content(); + } catch (...) { + Genode::error("failed to parse request ", request); + return; + } try { _requests_handler.handle_session_create(name, id, args); } catch (Service_denied) { @@ -94,7 +88,12 @@ class Genode::Session_requests_rom request.attribute_value("id", ~0UL) }; typedef Session_state::Args Args; - Args const args = request.sub_node("args").decoded_content(); + Args args { }; + try { args = request.sub_node("args").decoded_content(); } + catch (...) { + Genode::error("failed to parse request ", request); + return; + } _requests_handler.handle_session_upgrade(id, args); }; @@ -115,6 +114,24 @@ class Genode::Session_requests_rom /* create new sessions */ requests.for_each_sub_node("create", create_fn); } + + public: + + Session_requests_rom(Genode::Env &env, + Session_request_handler &requests_handler) + : Signal_handler(env.ep(), *this, &Session_requests_rom::_process), + _parent(env.parent()), + _requests_handler(requests_handler), + _parent_rom(env, "session_requests") + { + _parent_rom.sigh(*this); + } + + /** + * Post a signal to this requests handler + */ + void schedule() { + Signal_transmitter(*this).submit(); } }; #endif