From f6374988c89239c18d2dafba301bfd2c5704eafc Mon Sep 17 00:00:00 2001 From: Vratislav Podzimek Date: Tue, 10 Dec 2024 15:33:06 +0100 Subject: [PATCH 1/2] fix: Add a UpdateDownloadCancel state to the mender-update's state machine To make sure we always cancel the update download in case of an error. We need to be able to handle HTTPS/network/... errors as well as artifact parsing errors (incl. checksum mismatches,...). Instead of adding `download_client->Cancel()` calls to many places and risking missing some more, we introduce a new state the UpdateDownload state transitions into on failure and then things continue to the `download_error` state that handles the related state scripts, etc. Ticket: MEN-7810 Changelog: Fix download failure to always do a proper cancellation and cleanup of internal HTTP stuctures to avoid breaking future HTTP requests. Fixes `bad_version` error. Signed-off-by: Vratislav Podzimek --- src/mender-update/daemon/state_machine.hpp | 1 + src/mender-update/daemon/state_machine/state_machine.cpp | 5 ++++- src/mender-update/daemon/states.cpp | 8 ++++++-- src/mender-update/daemon/states.hpp | 5 +++++ 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/mender-update/daemon/state_machine.hpp b/src/mender-update/daemon/state_machine.hpp index 42a150d57..1d3224229 100644 --- a/src/mender-update/daemon/state_machine.hpp +++ b/src/mender-update/daemon/state_machine.hpp @@ -72,6 +72,7 @@ class StateMachine { PollForDeploymentState poll_for_deployment_state_; SendStatusUpdateState send_download_status_state_; UpdateDownloadState update_download_state_; + UpdateDownloadCancelState update_download_cancel_state_; SendStatusUpdateState send_install_status_state_; UpdateInstallState update_install_state_; diff --git a/src/mender-update/daemon/state_machine/state_machine.cpp b/src/mender-update/daemon/state_machine/state_machine.cpp index 4623fba64..365f50dbf 100644 --- a/src/mender-update/daemon/state_machine/state_machine.cpp +++ b/src/mender-update/daemon/state_machine/state_machine.cpp @@ -124,9 +124,12 @@ StateMachine::StateMachine(Context &ctx, events::EventLoop &event_loop) : main_states_.AddTransition(update_download_state_, se::Success, ss.download_leave_, tf::Immediate); main_states_.AddTransition(update_download_state_, se::StateLoopDetected, state_loop_state_, tf::Immediate); - main_states_.AddTransition(update_download_state_, se::Failure, ss.download_error_, tf::Immediate); + main_states_.AddTransition(update_download_state_, se::Failure, update_download_cancel_state_, tf::Immediate); main_states_.AddTransition(update_download_state_, se::NothingToDo, ss.download_leave_save_provides, tf::Immediate); + // Cannot fail because download cancellation is a void function as there's nothing to do if it fails, anyway. + main_states_.AddTransition(update_download_cancel_state_, se::Success, ss.download_error_, tf::Immediate); + main_states_.AddTransition(ss.download_leave_, se::Success, send_install_status_state_, tf::Immediate); main_states_.AddTransition(ss.download_leave_, se::Failure, ss.download_error_, tf::Immediate); diff --git a/src/mender-update/daemon/states.cpp b/src/mender-update/daemon/states.cpp index d157d3ec4..73b0bae47 100644 --- a/src/mender-update/daemon/states.cpp +++ b/src/mender-update/daemon/states.cpp @@ -280,7 +280,6 @@ void UpdateDownloadState::OnEnter(Context &ctx, sm::EventPoster &pos if (resp->GetStatusCode() != http::StatusOK) { log::Error( "Unexpected status code while fetching artifact: " + resp->GetStatusMessage()); - ctx.download_client->Cancel(); poster.PostEvent(StateEvent::Failure); return; } @@ -288,7 +287,6 @@ void UpdateDownloadState::OnEnter(Context &ctx, sm::EventPoster &pos auto http_reader = resp->MakeBodyAsyncReader(); if (!http_reader) { log::Error(http_reader.error().String()); - ctx.download_client->Cancel(); poster.PostEvent(StateEvent::Failure); return; } @@ -455,6 +453,12 @@ void UpdateDownloadState::DoDownload(Context &ctx, sm::EventPoster & } } +void UpdateDownloadCancelState::OnEnter(Context &ctx, sm::EventPoster &poster) { + log::Debug("Entering DownloadCancel state"); + ctx.download_client->Cancel(); + poster.PostEvent(StateEvent::Success); +} + SendStatusUpdateState::SendStatusUpdateState(optional status) : status_(status), mode_(FailureMode::Ignore) { diff --git a/src/mender-update/daemon/states.hpp b/src/mender-update/daemon/states.hpp index 95e431743..915109ad7 100644 --- a/src/mender-update/daemon/states.hpp +++ b/src/mender-update/daemon/states.hpp @@ -103,6 +103,11 @@ class UpdateDownloadState : virtual public StateType { static void DoDownload(Context &ctx, sm::EventPoster &poster); }; +class UpdateDownloadCancelState : virtual public StateType { +public: + void OnEnter(Context &ctx, sm::EventPoster &poster) override; +}; + class SendStatusUpdateState : virtual public StateType { public: // Ignore-failure version. From 6a0c8900c86f88ccd07b21d69d0a3a58f268fa1d Mon Sep 17 00:00:00 2001 From: Vratislav Podzimek Date: Tue, 10 Dec 2024 16:06:27 +0100 Subject: [PATCH 2/2] fix: Cancel the previous request before scheduling a new one in HTTP resumer The `http::Client()` class is designed to always have only one HTTP request in progress. Thus, before scheduling a new request using the same `http::Client` instance, cancel the previous request to make sure everything is properly reset for the new one. Ticket: MEN-7810 Changelog: Fix download resuming to reset the HTTP state and avoid repeatedly hitting the same error in case of a bad state Signed-off-by: Vratislav Podzimek --- src/common/http/http_resumer.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/common/http/http_resumer.cpp b/src/common/http/http_resumer.cpp index fbc1764af..b6ab0c55a 100644 --- a/src/common/http/http_resumer.cpp +++ b/src/common/http/http_resumer.cpp @@ -423,6 +423,9 @@ http::OutgoingRequestPtr DownloadResumerClient::RemainingRangeRequest() const { }; error::Error DownloadResumerClient::ScheduleNextResumeRequest() { + // In any case, make sure the previous HTTP request is cancelled. + client_.Cancel(); + auto exp_interval = retry_.backoff.NextInterval(); if (!exp_interval) { return http::MakeError(