From df7de17435249a4581b0243ef1b5f4933cb402ae Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Mon, 22 Mar 2021 18:42:15 +0100 Subject: [PATCH] vfs/cbe_trust_anchor: sync hashfile-handle close Closing the hashfile handle after a write operation wasn't synchronised to the actual end of the write operation. Issuing a write operation at the back end returns successfull as soon as the back end has acknowledged that it will execute the operation. However, the actual writing of the data might still be in progress at this point. But the plugin used to close the file handle and declare the operation finished at this point which led to warnings about acks on unknown file handles and leaking resources. Now, the plugin issues a sync operation directly after the write operation and waits for the sync to complete. This ensures that the plugin doesn't declare the operation finished too early. Ref #4032 --- repos/gems/include/cbe/vfs/io_job.h | 1 + .../gems/src/lib/vfs/cbe_trust_anchor/vfs.cc | 45 ++++++++++++++----- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/repos/gems/include/cbe/vfs/io_job.h b/repos/gems/include/cbe/vfs/io_job.h index 1000ccc616..9c9356ac5f 100644 --- a/repos/gems/include/cbe/vfs/io_job.h +++ b/repos/gems/include/cbe/vfs/io_job.h @@ -285,6 +285,7 @@ namespace Util { bool completed() const { return _complete; } bool succeeded() const { return _success; } + Operation op() const { return _op; } void print(Genode::Output &out) const { diff --git a/repos/gems/src/lib/vfs/cbe_trust_anchor/vfs.cc b/repos/gems/src/lib/vfs/cbe_trust_anchor/vfs.cc index 5981db9c15..dcac255fe7 100644 --- a/repos/gems/src/lib/vfs/cbe_trust_anchor/vfs.cc +++ b/repos/gems/src/lib/vfs/cbe_trust_anchor/vfs.cc @@ -105,7 +105,7 @@ class Trust_anchor }; Job _job { Job::NONE }; - enum class Job_state { NONE, PENDING, IN_PROGRESS, COMPLETE }; + enum class Job_state { NONE, PENDING, IN_PROGRESS, FINAL_SYNC, COMPLETE }; Job_state _job_state { Job_state::NONE }; bool _job_success { false }; @@ -351,7 +351,18 @@ class Trust_anchor } [[fallthrough]]; case Job_state::IN_PROGRESS: - if (!_write_hash_file_finished()) { + if (!_write_op_on_hash_file_is_in_final_sync_step()) { + break; + } + + _job_state = Job_state::FINAL_SYNC; + _job_success = true; + + progress |= true; + [[fallthrough]]; + case Job_state::FINAL_SYNC: + + if (!_final_sync_of_write_op_on_hash_file_finished()) { break; } @@ -610,9 +621,6 @@ class Trust_anchor if (!_hash_io_job.constructed()) { return true; } - - // XXX trigger sync - bool const progress = _hash_io_job->execute(); bool const completed = _hash_io_job->completed(); if (completed) { @@ -624,6 +632,12 @@ class Trust_anchor return progress && completed; } + void _start_sync_at_hash_io_job() + { + _hash_io_job.construct(*_hash_handle, Util::Io_job::Operation::SYNC, + _hash_io_job_buffer, 0); + } + void _close_hash_handle() { _vfs_env.root_dir().close(_hash_handle); @@ -665,25 +679,34 @@ class Trust_anchor _hash_io_job_buffer, 0); if (_hash_io_job->execute() && _hash_io_job->completed()) { - _close_hash_handle(); + _start_sync_at_hash_io_job(); } return true; } - bool _write_hash_file_finished() + bool _write_op_on_hash_file_is_in_final_sync_step() + { + if (_hash_io_job->op() == Util::Io_job::Operation::SYNC) { + return true; + } + bool const progress = _hash_io_job->execute(); + bool const completed = _hash_io_job->completed(); + if (completed) { + _start_sync_at_hash_io_job(); + } + return progress && completed; + } + + bool _final_sync_of_write_op_on_hash_file_finished() { if (!_hash_io_job.constructed()) { return true; } - - // XXX trigger sync - bool const progress = _hash_io_job->execute(); bool const completed = _hash_io_job->completed(); if (completed) { _close_hash_handle(); } - return progress && completed; }