pw_rpc: Fix Call not getting reset on default constructor assignment

When trying to reinitialize an already used Call object to default,
the move constructor runs but ends up exiting early before actually
resetting the call internal state due to this conditional:

  if (!other.active_locked()) {
    return;  // Nothing else to do; this call is already closed.
  }

A default Call has state_ initialized to 0, so active_locked()
returns false so none of the assignments happen on `this`.

We fix this by letting the bulk of the move assignments happen
regardless of active state, except for those parts that apply
specifically to active calls.

Bug: 371211198
Change-Id: I8cb18d06bd91db41822937a57b31351049678222
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/239718
Docs-Not-Needed: Austin Foxley <afoxley@google.com>
Commit-Queue: Austin Foxley <afoxley@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
Lint: Lint 🤖 <android-build-ayeaye@system.gserviceaccount.com>

pw_rpc/call.cc

@@ -144,13 +144,9 @@ void Call::MoveFrom(Call& other) {
   PW_DCHECK(!active_locked());
   PW_DCHECK(!awaiting_cleanup() && !other.awaiting_cleanup());
 
-  if (!other.active_locked()) {
-    return;  // Nothing else to do; this call is already closed.
-  }
-
   // An active call with an executing callback cannot be moved. Derived call
   // classes must wait for callbacks to finish before calling MoveFrom.
-  PW_DCHECK(!other.CallbacksAreRunning());
+  PW_DCHECK(!other.active_locked() || !other.CallbacksAreRunning());
 
   // Copy all members from the other call.
   endpoint_ = other.endpoint_;
@@ -171,11 +167,12 @@ void Call::MoveFrom(Call& other) {
   on_error_ = std::move(other.on_error_);
   on_next_ = std::move(other.on_next_);
 
-  // Mark the other call inactive, unregister it, and register this one.
-  other.MarkClosed();
-
-  endpoint().UnregisterCall(other);
-  endpoint().RegisterUniqueCall(*this);
+  if (other.active_locked()) {
+    // Mark the other call inactive, unregister it, and register this one.
+    other.MarkClosed();
+    endpoint().UnregisterCall(other);
+    endpoint().RegisterUniqueCall(*this);
+  }
 }
 
 void Call::WaitUntilReadyForMove(Call& destination, Call& source) {

pw_rpc/call_test.cc

@@ -337,6 +337,14 @@ TEST_F(ServerReaderWriterTest, Move_ClearsCallAndChannelId) {
   EXPECT_EQ(reader_writer_.channel_id_locked(), 0u);
 }
 
+TEST_F(ServerReaderWriterTest, DefaultConstructorAssign_Reset) {
+  reader_writer_ = {};
+
+  RpcLockGuard lock;
+  EXPECT_EQ(reader_writer_.service_id(), 0u);
+  EXPECT_EQ(reader_writer_.method_id(), 0u);
+}
+
 TEST_F(ServerReaderWriterTest, Move_SourceAwaitingCleanup_CleansUpCalls) {
   std::optional<Status> on_error_cb;
   reader_writer_.set_on_error([&on_error_cb](Status error) {

pw_rpc/pw_rpc_private/fake_server_reader_writer.h

@@ -55,6 +55,8 @@ class FakeServerReaderWriter : private ServerCall {
   // `PW_LOCKS_EXCLUDED(rpc_lock())` on their original definitions does not
   // appear to carry through here.
   using Call::active;
+  using Call::method_id;
+  using Call::service_id;
   using Call::set_on_error;
   using Call::set_on_next;
   using ServerCall::set_on_completion_requested;