From 89056eab62bab00fd86835e974004d6259ab2a1f Mon Sep 17 00:00:00 2001 From: Eli Lipsitz Date: Mon, 26 Sep 2022 23:38:07 +0000 Subject: [PATCH] pw_software_update: Fix recursive lock acquisition A recent change (I278f16834e95a4020bd7b8586f1878a475b82d60) added a lock acquisition to BundledUpdateService::SetTransferred, because the SET_ERROR macro requires the internal lock. However, it calls NotifyTransferSucceeded, which also acquires the lock. This means that any calls to SetTransferred deadlock on a recursive acquisition. This commit fixes the issue by moving the outer lock acquisition into the if statement that calls SET_ERROR, which is mutually exclusive with the call to NotifyTransferSucceeded. Tested: The SetTransferred RPC method no longer hangs. Change-Id: I0ee7994fa5e093a05e349876406d0a2bd89fc3e7 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/112170 Pigweed-Auto-Submit: Eli Lipsitz Reviewed-by: Ali Zhang Commit-Queue: Auto-Submit --- pw_software_update/bundled_update_service_pwpb.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pw_software_update/bundled_update_service_pwpb.cc b/pw_software_update/bundled_update_service_pwpb.cc index 0f26b071d..2d7e1c75b 100644 --- a/pw_software_update/bundled_update_service_pwpb.cc +++ b/pw_software_update/bundled_update_service_pwpb.cc @@ -122,11 +122,11 @@ Status BundledUpdateService::Start(const StartRequest::Message& request, Status BundledUpdateService::SetTransferred( const pw::protobuf::Empty::Message&, BundledUpdateStatus::Message& response) { - std::lock_guard lock(mutex_); const BundledUpdateState::Enum state = status_.acquire()->state; if (state != BundledUpdateState::Enum::kTransferring && state != BundledUpdateState::Enum::kInactive) { + std::lock_guard lock(mutex_); SET_ERROR(BundledUpdateResult::Enum::kUnknownError, "SetTransferred() can only be called from TRANSFERRING or " "INACTIVE state. State: %d",