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 <elipsitz@google.com>
Reviewed-by: Ali Zhang <alizhang@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
diff --git a/pw_software_update/bundled_update_service_pwpb.cc b/pw_software_update/bundled_update_service_pwpb.cc
index 0f26b07..2d7e1c7 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::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",
