diff --git a/src/app/WriteClient.cpp b/src/app/WriteClient.cpp index 36501ee780d35f..ce1df1a0b2e633 100644 --- a/src/app/WriteClient.cpp +++ b/src/app/WriteClient.cpp @@ -248,14 +248,20 @@ CHIP_ERROR WriteClient::SendWriteRequest(SessionHandle session, System::Clock::T 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; diff --git a/src/app/WriteClient.h b/src/app/WriteClient.h index 1a9a93dae7ffd3..5151c2732aaa2b 100644 --- a/src/app/WriteClient.h +++ b/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(SessionHandle session, System::Clock::Timeout timeout = kImMessageTimeout); diff --git a/src/controller/WriteInteraction.h b/src/controller/WriteInteraction.h index 59c49761484135..037158e8b2cd53 100644 --- a/src/controller/WriteInteraction.h +++ b/src/controller/WriteInteraction.h @@ -106,6 +106,11 @@ CHIP_ERROR WriteAttribute(SessionHandle sessionHandle, chip::EndpointId endpoint 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(SessionHandle sessionHandle, chip::EndpointId endpoint ReturnErrorOnFailure(handle.SendWriteRequest(sessionHandle)); - callback.release(); return CHIP_NO_ERROR; }