Fix double-free bugs on failure to send a write message. (#13051)

There were two separate bugs here:

1) For a group write, we were violating the contract for
WriteClient::SendWriteRequest, which is that the caller must call
Shutdown on failure but the WriteClient itself will do it on success
(and call OnDone in the process).  We were unconditionally calling
Shutdown and OnDone inside SetWriteRequest, even on failure.

2) WriteInteraction was violating the contract for
WriteClientHandle::SendWriteRequest, which is different: it always
guarantees it will call OnDone. But the consumer was assuming that
OnDone would only be called if SendWriteRequest returned success.

src/app/WriteClient.cpp

@@ -248,14 +248,20 @@ CHIP_ERROR WriteClient::SendWriteRequest(const SessionHandle & session, System::
         ChipLogError(DataManagement, "Write client failed to SendWriteRequest");
         ClearExistingExchangeContext();
     }
-
-    if (session.IsGroupSession())
+    else
     {
-        // Always shutdown on Group communication
-        ChipLogDetail(DataManagement, "Closing on group Communication ");
+        // TODO: Ideally this would happen async, but to make sure that we
+        // handle this object dying (e.g. due to IM enging shutdown) while the
+        // async bits are pending we'd need to malloc some state bit that we can
+        // twiddle if we die.  For now just do the OnDone callback sync.
+        if (session.IsGroupSession())
+        {
+            // Always shutdown on Group communication
+            ChipLogDetail(DataManagement, "Closing on group Communication ");
 
-        // onDone is called
-        ShutdownInternal();
+            // onDone is called
+            ShutdownInternal();
+        }
     }
 
     return err;

src/app/WriteClient.h

@@ -248,6 +248,9 @@ class WriteClientHandle
     /**
      *  Finalize the message and send it to the desired node. The underlying write object will always be released, and the user
      * should not use this object after calling this function.
+     *
+     * Note: In failure cases this will _synchronously_ invoke OnDone on the
+     * WriteClient::Callback before returning.
      */
     CHIP_ERROR SendWriteRequest(const SessionHandle & session, System::Clock::Timeout timeout = kImMessageTimeout);
 

src/controller/WriteInteraction.h

@@ -106,6 +106,11 @@ CHIP_ERROR WriteAttribute(const SessionHandle & sessionHandle, chip::EndpointId
     VerifyOrReturnError(callback != nullptr, CHIP_ERROR_NO_MEMORY);
 
     ReturnErrorOnFailure(app::InteractionModelEngine::GetInstance()->NewWriteClient(handle, callback.get(), aTimedWriteTimeoutMs));
+
+    // At this point the handle will ensure our callback's OnDone is always
+    // called.
+    callback.release();
+
     if (sessionHandle.IsGroupSession())
     {
         ReturnErrorOnFailure(
@@ -119,7 +124,6 @@ CHIP_ERROR WriteAttribute(const SessionHandle & sessionHandle, chip::EndpointId
 
     ReturnErrorOnFailure(handle.SendWriteRequest(sessionHandle));
 
-    callback.release();
     return CHIP_NO_ERROR;
 }