From 2e91930eff5ffa17a7f7f5d6a833362194bd1664 Mon Sep 17 00:00:00 2001
From: Austin Foxley <afoxley@google.com>
Date: Fri, 4 Oct 2024 20:50:47 +0000
Subject: [PATCH] pw_rpc: Fix crash on call cleanup
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The fix for b/371211198 exposed a potential invalid pointer access now
that call fields get properly reset on being reinitialized. Check that
endpoint_ was not cleared while HandlePayload was not holding rpc lock.

Bug: 371211198
Change-Id: I66a146b2e0db5f8c3b0ef50d049e0db342c99095
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/240138
Lint: Lint 🤖 <android-build-ayeaye@system.gserviceaccount.com>
Presubmit-Verified: CQ Bot Account <pigweed-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: Austin Foxley <afoxley@google.com>
Docs-Not-Needed: Austin Foxley <afoxley@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
---
 pw_rpc/call.cc | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/pw_rpc/call.cc b/pw_rpc/call.cc
index efed2e8b3d..976c781b53 100644
--- a/pw_rpc/call.cc
+++ b/pw_rpc/call.cc
@@ -331,8 +331,15 @@ void Call::HandlePayload(ConstByteSpan payload) {
     on_next_ = std::move(on_next_local);
   }
 
-  // Clean up calls in case decoding failed.
-  endpoint_->CleanUpCalls();
+  // The call could have been reinitialized and cleaned up already by another
+  // thread that acquired the rpc_lock() while on_next_local was executing
+  // without lock held.
+  if (endpoint_ != nullptr) {
+    // Clean up calls in case decoding failed.
+    endpoint_->CleanUpCalls();
+  } else {
+    rpc_lock().unlock();
+  }
 }
 
 void Call::CloseClientCall() {