Skip to content

Commit

Permalink
[IM] Remove CommandSender object pool in interaction model engine (#9877
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@erjiaqing
erjiaqing authored Oct 8, 2021
1 parent 14a04e9 commit 6f73ebc
Show file tree
Hide file tree
Showing 29 changed files with 3,176 additions and 2,916 deletions.
175 changes: 88 additions & 87 deletions src/app/Command.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,43 +33,35 @@
namespace chip {
namespace app {

CHIP_ERROR Command::Init(Messaging::ExchangeManager * apExchangeMgr, InteractionModelDelegate * apDelegate)
Command::Command(Messaging::ExchangeManager * apExchangeMgr)
{
CHIP_ERROR err = CHIP_NO_ERROR;
// Error if already initialized.
VerifyOrExit(apExchangeMgr != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
VerifyOrExit(mpExchangeMgr == nullptr, err = CHIP_ERROR_INCORRECT_STATE);

mpExchangeMgr = apExchangeMgr;
mpDelegate = apDelegate;
err = Reset();
SuccessOrExit(err);

exit:
return err;
}

CHIP_ERROR Command::Reset()
CHIP_ERROR Command::AllocateBuffer()
{
CHIP_ERROR err = CHIP_NO_ERROR;
CommandList::Builder commandListBuilder;
AbortExistingExchangeContext();

mCommandMessageWriter.Reset();
if (!mBufferAllocated)
{
mCommandMessageWriter.Reset();

System::PacketBufferHandle commandPacket = System::PacketBufferHandle::New(chip::app::kMaxSecureSduLengthBytes);
VerifyOrExit(!commandPacket.IsNull(), err = CHIP_ERROR_NO_MEMORY);
System::PacketBufferHandle commandPacket = System::PacketBufferHandle::New(chip::app::kMaxSecureSduLengthBytes);
VerifyOrExit(!commandPacket.IsNull(), err = CHIP_ERROR_NO_MEMORY);

mCommandMessageWriter.Init(std::move(commandPacket));
err = mInvokeCommandBuilder.Init(&mCommandMessageWriter);
SuccessOrExit(err);
mCommandMessageWriter.Init(std::move(commandPacket));
err = mInvokeCommandBuilder.Init(&mCommandMessageWriter);
SuccessOrExit(err);

commandListBuilder = mInvokeCommandBuilder.CreateCommandListBuilder();
err = commandListBuilder.GetError();
SuccessOrExit(err);
MoveToState(CommandState::Initialized);
commandListBuilder = mInvokeCommandBuilder.CreateCommandListBuilder();
err = commandListBuilder.GetError();
SuccessOrExit(err);

mCommandIndex = 0;

mCommandIndex = 0;
mBufferAllocated = true;
}

exit:
return err;
Expand Down Expand Up @@ -123,30 +115,19 @@ CHIP_ERROR Command::ProcessCommandMessage(System::PacketBufferHandle && payload,
return err;
}

void Command::Shutdown()
{
VerifyOrReturn(mState != CommandState::Uninitialized);
AbortExistingExchangeContext();
ShutdownInternal();
}

void Command::ShutdownInternal()
{
mCommandMessageWriter.Reset();

mpExchangeMgr = nullptr;
mpExchangeCtx = nullptr;
mpDelegate = nullptr;
ClearState();

mCommandIndex = 0;
}

CHIP_ERROR Command::PrepareCommand(const CommandPathParams & aCommandPathParams, bool aStartDataStruct)
{
CHIP_ERROR err = CHIP_NO_ERROR;
CommandDataElement::Builder commandDataElement;
VerifyOrExit(mState == CommandState::Initialized || mState == CommandState::AddCommand, err = CHIP_ERROR_INCORRECT_STATE);

err = AllocateBuffer();
SuccessOrExit(err);

//
// We must not be in the middle of preparing a command, or having prepared or sent one.
//
VerifyOrExit(mState == CommandState::Idle, err = CHIP_ERROR_INCORRECT_STATE);

commandDataElement = mInvokeCommandBuilder.GetCommandListBuilder().CreateCommandDataElementBuilder();
err = commandDataElement.GetError();
SuccessOrExit(err);
Expand All @@ -159,32 +140,50 @@ CHIP_ERROR Command::PrepareCommand(const CommandPathParams & aCommandPathParams,
err = commandDataElement.GetWriter()->StartContainer(TLV::ContextTag(CommandDataElement::kCsTag_Data),
TLV::kTLVType_Structure, mDataElementContainerType);
}

MoveToState(CommandState::AddingCommand);

exit:
return err;
}

TLV::TLVWriter * Command::GetCommandDataElementTLVWriter()
{
return mInvokeCommandBuilder.GetCommandListBuilder().GetCommandDataElementBuilder().GetWriter();
if (mState != CommandState::AddingCommand)
{
return nullptr;
}
else
{
return mInvokeCommandBuilder.GetCommandListBuilder().GetCommandDataElementBuilder().GetWriter();
}
}

CHIP_ERROR Command::Finalize(System::PacketBufferHandle & commandPacket)
{
VerifyOrReturnError(mState == CommandState::AddedCommand, CHIP_ERROR_INCORRECT_STATE);
return mCommandMessageWriter.Finalize(&commandPacket);
}

CHIP_ERROR Command::FinishCommand(bool aEndDataStruct)
{
CHIP_ERROR err = CHIP_NO_ERROR;

VerifyOrReturnError(mState == CommandState::AddingCommand, err = CHIP_ERROR_INCORRECT_STATE);

CommandDataElement::Builder commandDataElement = mInvokeCommandBuilder.GetCommandListBuilder().GetCommandDataElementBuilder();
if (aEndDataStruct)
{
err = commandDataElement.GetWriter()->EndContainer(mDataElementContainerType);
SuccessOrExit(err);
ReturnErrorOnFailure(commandDataElement.GetWriter()->EndContainer(mDataElementContainerType));
}
commandDataElement.EndOfCommandDataElement();
err = commandDataElement.GetError();
SuccessOrExit(err);
MoveToState(CommandState::AddCommand);

exit:
return err;
ReturnErrorOnFailure(commandDataElement.EndOfCommandDataElement().GetError());
ReturnErrorOnFailure(mInvokeCommandBuilder.GetCommandListBuilder().EndOfCommandList().GetError());
ReturnErrorOnFailure(mInvokeCommandBuilder.EndOfInvokeCommand().GetError());

MoveToState(CommandState::AddedCommand);

return CHIP_NO_ERROR;
}

CHIP_ERROR Command::ConstructCommandPath(const CommandPathParams & aCommandPathParams,
Expand All @@ -206,54 +205,61 @@ CHIP_ERROR Command::ConstructCommandPath(const CommandPathParams & aCommandPathP
return commandPath.GetError();
}

CHIP_ERROR Command::AbortExistingExchangeContext()
void Command::Abort()
{
// Discard any existing exchange context. Effectively we can only have one Echo exchange with
// a single node at any one time.
//
// If the exchange context hasn't already been gracefully closed
// (signaled by setting it to null), then we need to forcibly
// tear it down.
//
if (mpExchangeCtx != nullptr)
{
mpExchangeCtx->Abort();
mpExchangeCtx = nullptr;
}

return CHIP_NO_ERROR;
}

CHIP_ERROR Command::FinalizeCommandsMessage(System::PacketBufferHandle & commandPacket)
void Command::Close()
{
CHIP_ERROR err = CHIP_NO_ERROR;
CommandList::Builder commandListBuilder;
VerifyOrExit(mState == CommandState::AddCommand, err = CHIP_ERROR_INCORRECT_STATE);
commandListBuilder = mInvokeCommandBuilder.GetCommandListBuilder().EndOfCommandList();
err = commandListBuilder.GetError();
SuccessOrExit(err);

mInvokeCommandBuilder.EndOfInvokeCommand();
err = mInvokeCommandBuilder.GetError();
SuccessOrExit(err);
//
// Shortly after this call to close and when handling an inbound message, it's entirely possible
// for this object (courtesy of its derived class) to be destroyed
// *before* the call unwinds all the way back to ExchangeContext::HandleMessage.
//
// As part of tearing down the exchange, there is logic there to invoke the delegate to notify
// it of impending closure - which is this object, which just got destroyed!
//
// So prevent a use-after-free, set delegate to null.
//
// For more details, see #10344.
//
if (mpExchangeCtx != nullptr)
{
mpExchangeCtx->SetDelegate(nullptr);
}

err = mCommandMessageWriter.Finalize(&commandPacket);
SuccessOrExit(err);
exit:
return err;
mpExchangeCtx = nullptr;
}

const char * Command::GetStateStr() const
{
#if CHIP_DETAIL_LOGGING
switch (mState)
{
case CommandState::Uninitialized:
return "Uninitialized";
case CommandState::Idle:
return "Idle";

case CommandState::AddingCommand:
return "AddingCommand";

case CommandState::Initialized:
return "Initialized";
case CommandState::AddedCommand:
return "AddedCommand";

case CommandState::AddCommand:
return "AddCommand";
case CommandState::CommandSent:
return "CommandSent";

case CommandState::Sending:
return "Sending";
case CommandState::AwaitingDestruction:
return "AwaitingDestruction";
}
#endif // CHIP_DETAIL_LOGGING
return "N/A";
Expand All @@ -265,10 +271,5 @@ void Command::MoveToState(const CommandState aTargetState)
ChipLogDetail(DataManagement, "ICR moving to [%10.10s]", GetStateStr());
}

void Command::ClearState(void)
{
MoveToState(CommandState::Uninitialized);
}

} // namespace app
} // namespace chip
87 changes: 41 additions & 46 deletions src/app/Command.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,45 +56,28 @@ class Command

enum class CommandState
{
Uninitialized = 0, ///< The invoke command message has not been initialized
Initialized, ///< The invoke command message has been initialized and is ready
AddCommand, ///< The invoke command message has added Command
Sending, ///< The invoke command message has sent out the invoke command
Idle, ///< Default state that the object starts out in, where no work has commenced
AddingCommand, ///< In the process of adding a command.
AddedCommand, ///< A command has been completely encoded and is awaiting transmission.
CommandSent, ///< The command has been sent successfully.
AwaitingDestruction, ///< The object has completed its work and is awaiting destruction by the application.
};

/**
* Initialize the Command object. Within the lifetime
* of this instance, this method is invoked once after object
* construction until a call to Shutdown is made to terminate the
* instance.
*
* @param[in] apExchangeMgr A pointer to the ExchangeManager object.
* @param[in] apDelegate InteractionModelDelegate set by application.
*
* @retval #CHIP_ERROR_INCORRECT_STATE If the state is not equal to
* CommandState::NotInitialized.
* @retval #CHIP_NO_ERROR On success.
/*
* Destructor - as part of destruction, it will abort the exchange context
* if a valid one still exists.
*
* See Abort() for details on when that might occur.
*/
CHIP_ERROR Init(Messaging::ExchangeManager * apExchangeMgr, InteractionModelDelegate * apDelegate);
virtual ~Command() { Abort(); }

/**
* Shutdown the Command. This terminates this instance
* of the object and releases all held resources.
/*
* A set of methods to construct command request or response payloads
*/
void Shutdown();

/**
* Finalize Command Message TLV Builder and finalize command message
*
* @return CHIP_ERROR
*
*/
CHIP_ERROR FinalizeCommandsMessage(System::PacketBufferHandle & commandPacket);

CHIP_ERROR PrepareCommand(const CommandPathParams & aCommandPathParams, bool aStartDataStruct = true);
TLV::TLVWriter * GetCommandDataElementTLVWriter();
CHIP_ERROR FinishCommand(bool aEndDataStruct = true);
CHIP_ERROR Finalize(System::PacketBufferHandle & commandPacket);
virtual CHIP_ERROR AddStatusCode(const ConcreteCommandPath & aCommandPath,
const Protocols::SecureChannel::GeneralStatusCode aGeneralCode,
const Protocols::Id aProtocolId, const Protocols::InteractionModel::Status aStatus)
Expand All @@ -111,38 +94,50 @@ class Command
*/
Messaging::ExchangeContext * GetExchangeContext() const { return mpExchangeCtx; }

CHIP_ERROR Reset();

virtual ~Command() = default;

bool IsFree() const { return mState == CommandState::Uninitialized; };
virtual CHIP_ERROR ProcessCommandDataElement(CommandDataElement::Parser & aCommandElement) = 0;

protected:
CHIP_ERROR AbortExistingExchangeContext();
Command(Messaging::ExchangeManager * apExchangeMgr);

/*
* Allocates a packet buffer used for encoding an invoke request/response payload.
*
* This can be called multiple times safely, as it will only allocate the buffer once for the lifetime
* of this object.
*/
CHIP_ERROR AllocateBuffer();

/*
* The actual closure of the exchange happens automatically in the exchange layer.
* This function just sets the internally tracked exchange pointer to null to align
* with the exchange layer so as to prevent further closure if Abort() is called later.
*/
void Close();

void MoveToState(const CommandState aTargetState);
CHIP_ERROR ProcessCommandMessage(System::PacketBufferHandle && payload, CommandRoleId aCommandRoleId);
CHIP_ERROR ConstructCommandPath(const CommandPathParams & aCommandPathParams, CommandDataElement::Builder aCommandDataElement);
void ClearState();
const char * GetStateStr() const;

/**
* Internal shutdown method that we use when we know what's going on with
* our exchange and don't need to manually close it.
*/
void ShutdownInternal();

InvokeCommand::Builder mInvokeCommandBuilder;
Messaging::ExchangeManager * mpExchangeMgr = nullptr;
Messaging::ExchangeContext * mpExchangeCtx = nullptr;
InteractionModelDelegate * mpDelegate = nullptr;
uint8_t mCommandIndex = 0;
CommandState mState = CommandState::Uninitialized;
CommandState mState = CommandState::Idle;
chip::System::PacketBufferTLVWriter mCommandMessageWriter;

private:
/*
* This forcibly closes the exchange context if a valid one is pointed to. Such a situation does
* not arise during normal message processing flows that all normally call Close() above. This can only
* arise due to application-initiated destruction of the object when this object is handling receiving/sending
* message payloads.
*/
void Abort();

friend class TestCommandInteraction;
TLV::TLVType mDataElementContainerType = TLV::kTLVType_NotSpecified;
chip::System::PacketBufferTLVWriter mCommandMessageWriter;
bool mBufferAllocated = false;
};
} // namespace app
} // namespace chip
Loading

0 comments on commit 6f73ebc

Please sign in to comment.