Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove raw SessionHolder::Get #18490

Merged
merged 2 commits into from
May 18, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/app/OperationalDeviceProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ CHIP_ERROR OperationalDeviceProxy::UpdateDeviceData(const Transport::PeerAddress
return CHIP_NO_ERROR;
}

mSecureSession.Get()->AsSecureSession()->SetPeerAddress(addr);
mSecureSession.Get().Value()->AsSecureSession()->SetPeerAddress(addr);
}

return err;
Expand Down Expand Up @@ -305,7 +305,7 @@ CHIP_ERROR OperationalDeviceProxy::Disconnect()
ReturnErrorCodeIf(mState != State::SecureConnected, CHIP_ERROR_INCORRECT_STATE);
if (mSecureSession)
{
mInitParams.sessionManager->ExpirePairing(mSecureSession.Get());
mInitParams.sessionManager->ExpirePairing(mSecureSession.Get().Value());
}
MoveToState(State::Initialized);

Expand Down
2 changes: 1 addition & 1 deletion src/app/OperationalDeviceProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy,

Messaging::ExchangeManager * GetExchangeManager() const override { return mInitParams.exchangeMgr; }

chip::Optional<SessionHandle> GetSecureSession() const override { return mSecureSession.ToOptional(); }
chip::Optional<SessionHandle> GetSecureSession() const override { return mSecureSession.Get(); }

Transport::PeerAddress GetPeerAddress() const { return mDeviceAddress; }

Expand Down
8 changes: 6 additions & 2 deletions src/app/ReadClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,9 @@ CHIP_ERROR ReadClient::SendReadRequest(ReadPrepareParams & aReadPrepareParams)
ReturnErrorOnFailure(request.EndOfReadRequestMessage().GetError());
ReturnErrorOnFailure(writer.Finalize(&msgBuf));

mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get(), this);
VerifyOrReturnError(aReadPrepareParams.mSessionHolder, CHIP_ERROR_MISSING_SECURE_SESSION);

mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get().Value(), this);
VerifyOrReturnError(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY);

mpExchangeCtx->SetResponseTimeout(aReadPrepareParams.mTimeout);
Expand Down Expand Up @@ -913,7 +915,9 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara
ReturnErrorOnFailure(err = request.EndOfSubscribeRequestMessage().GetError());
ReturnErrorOnFailure(writer.Finalize(&msgBuf));

mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get(), this);
VerifyOrReturnError(aReadPrepareParams.mSessionHolder, CHIP_ERROR_MISSING_SECURE_SESSION);

mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get().Value(), this);
VerifyOrReturnError(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY);
mpExchangeCtx->SetResponseTimeout(kImMessageTimeout);

Expand Down
4 changes: 2 additions & 2 deletions src/app/ReadHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ CHIP_ERROR ReadHandler::SendStatusReport(Protocols::InteractionModel::Status aSt
{
VerifyOrReturnLogError(mpExchangeCtx == nullptr, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnLogError(mSessionHandle, CHIP_ERROR_INCORRECT_STATE);
mpExchangeCtx = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext(mSessionHandle.Get(), this);
mpExchangeCtx = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext(mSessionHandle.Get().Value(), this);
}
VerifyOrReturnLogError(mpExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE);

Expand All @@ -240,7 +240,7 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b
{
VerifyOrReturnLogError(mpExchangeCtx == nullptr, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnLogError(mSessionHandle, CHIP_ERROR_INCORRECT_STATE);
mpExchangeCtx = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext(mSessionHandle.Get(), this);
mpExchangeCtx = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext(mSessionHandle.Get().Value(), this);
}

VerifyOrReturnLogError(mpExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE);
Expand Down
7 changes: 4 additions & 3 deletions src/controller/CommissioneeDeviceProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ CHIP_ERROR CommissioneeDeviceProxy::SendCommands(app::CommandSender * commandObj
{
VerifyOrReturnError(mSecureSession, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(commandObj != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
return commandObj->SendCommandRequest(mSecureSession.Get(), timeout);
VerifyOrReturnError(mSecureSession.Get().HasValue(), CHIP_ERROR_MISSING_SECURE_SESSION);
mrjerryjohns marked this conversation as resolved.
Show resolved Hide resolved
return commandObj->SendCommandRequest(mSecureSession.Get().Value(), timeout);
}

void CommissioneeDeviceProxy::OnSessionReleased()
Expand All @@ -60,7 +61,7 @@ CHIP_ERROR CommissioneeDeviceProxy::CloseSession()
ReturnErrorCodeIf(mState != ConnectionState::SecureConnected, CHIP_ERROR_INCORRECT_STATE);
if (mSecureSession)
{
mSessionManager->ExpirePairing(mSecureSession.Get());
mSessionManager->ExpirePairing(mSecureSession.Get().Value());
}
mState = ConnectionState::NotConnected;
mPairing.Clear();
Expand All @@ -87,7 +88,7 @@ CHIP_ERROR CommissioneeDeviceProxy::UpdateDeviceData(const Transport::PeerAddres
return CHIP_NO_ERROR;
}

Transport::SecureSession * secureSession = mSecureSession.Get()->AsSecureSession();
Transport::SecureSession * secureSession = mSecureSession.Get().Value()->AsSecureSession();
secureSession->SetPeerAddress(addr);

return CHIP_NO_ERROR;
Expand Down
2 changes: 1 addition & 1 deletion src/controller/CommissioneeDeviceProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionDelegate
CHIP_ERROR SetPeerId(ByteSpan rcac, ByteSpan noc) override;
const Transport::PeerAddress & GetPeerAddress() const { return mDeviceAddress; }

chip::Optional<SessionHandle> GetSecureSession() const override { return mSecureSession.ToOptional(); }
chip::Optional<SessionHandle> GetSecureSession() const override { return mSecureSession.Get(); }

Messaging::ExchangeManager * GetExchangeManager() const override { return mExchangeMgr; }

Expand Down
2 changes: 1 addition & 1 deletion src/controller/python/chip/clusters/attribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ OnReportBeginCallback gOnReportEndCallback = nullptr;

void PythonResubscribePolicy(uint32_t aNumCumulativeRetries, uint32_t & aNextSubscriptionIntervalMsec, bool & aShouldResubscribe)
{
aShouldResubscribe = false;
aShouldResubscribe = true;
}

class ReadClientCallback : public ReadClient::Callback
Expand Down
3 changes: 3 additions & 0 deletions src/lib/core/CHIPError.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,9 @@ bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err)
case CHIP_ERROR_IM_MALFORMED_STATUS_RESPONSE_MESSAGE.AsInteger():
desc = "Malformed Interaction Model Status Response Message";
break;
case CHIP_ERROR_MISSING_SECURE_SESSION.AsInteger():
desc = "Missing secure session";
break;
}
#endif // !CHIP_CONFIG_SHORT_ERROR_STR

Expand Down
7 changes: 6 additions & 1 deletion src/lib/core/CHIPError.h
Original file line number Diff line number Diff line change
Expand Up @@ -2389,8 +2389,13 @@ using CHIP_ERROR = ::chip::ChipError;
#define CHIP_ERROR_IM_MALFORMED_STATUS_RESPONSE_MESSAGE CHIP_CORE_ERROR(0xe1)

/**
* @}
* @def CHIP_ERROR_MISSING_SECURE_SESSION
*
* @brief
*
* A secure session is needed to do work, but is missing/is not present.
*/
#define CHIP_ERROR_MISSING_SECURE_SESSION CHIP_CORE_ERROR(0xe2)
mrjerryjohns marked this conversation as resolved.
Show resolved Hide resolved

// clang-format on

Expand Down
13 changes: 10 additions & 3 deletions src/messaging/ExchangeContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,17 @@ CHIP_ERROR ExchangeContext::SendMessage(Protocols::Id protocolId, uint8_t msgTyp
return CHIP_ERROR_INTERNAL;
}

//
// Our session should still be valid, since it had been evicted, all exchanges on that session would have been torn down
mrjerryjohns marked this conversation as resolved.
Show resolved Hide resolved
// as a side-effect. So for us to get here to send a message and still not have a session, something terribly wrong must
// have happened.
//
VerifyOrDie(mSession);
mrjerryjohns marked this conversation as resolved.
Show resolved Hide resolved

// Create a new scope for `err`, to avoid shadowing warning previous `err`.
CHIP_ERROR err = mDispatch.SendMessage(GetExchangeMgr()->GetSessionManager(), mSession.Get(), mExchangeId, IsInitiator(),
GetReliableMessageContext(), reliableTransmissionRequested, protocolId, msgType,
std::move(msgBuf));
CHIP_ERROR err = mDispatch.SendMessage(GetExchangeMgr()->GetSessionManager(), mSession.Get().Value(), mExchangeId,
IsInitiator(), GetReliableMessageContext(), reliableTransmissionRequested,
protocolId, msgType, std::move(msgBuf));
if (err != CHIP_NO_ERROR && IsResponseExpected())
{
CancelResponseTimer();
Expand Down
8 changes: 7 additions & 1 deletion src/messaging/ExchangeContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,13 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,

ExchangeMessageDispatch & GetMessageDispatch() { return mDispatch; }

SessionHandle GetSessionHandle() const { return mSession.Get(); }
SessionHandle GetSessionHandle() const
{
VerifyOrDie(mSession);
auto sessionHandle = mSession.Get();
return std::move(sessionHandle.Value());
}

bool HasSessionHandle() const { return mSession; }

uint16_t GetExchangeId() const { return mExchangeId; }
Expand Down
4 changes: 3 additions & 1 deletion src/protocols/echo/EchoClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,10 @@ CHIP_ERROR EchoClient::SendEchoRequest(System::PacketBufferHandle && payload, Me
mExchangeCtx = nullptr;
}

VerifyOrReturnError(mSecureSession, CHIP_ERROR_INVALID_MESSAGE_TYPE);

// Create a new exchange context.
mExchangeCtx = mExchangeMgr->NewContext(mSecureSession.Get(), this);
mExchangeCtx = mExchangeMgr->NewContext(mSecureSession.Get().Value(), this);
if (mExchangeCtx == nullptr)
{
return CHIP_ERROR_NO_MEMORY;
Expand Down
9 changes: 5 additions & 4 deletions src/protocols/secure_channel/PairingSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ void PairingSession::Finish()
CHIP_ERROR err = ActivateSecureSession(address);
if (err == CHIP_NO_ERROR)
{
mDelegate->OnSessionEstablished(mSecureSessionHolder.Get());
VerifyOrDie(mSecureSessionHolder);
mDelegate->OnSessionEstablished(mSecureSessionHolder.Get().Value());
}
else
{
Expand Down Expand Up @@ -155,14 +156,14 @@ void PairingSession::Clear()

if (mSecureSessionHolder)
{
SessionHandle session = mSecureSessionHolder.Get();
auto session = mSecureSessionHolder.Get();
// Call Release before ExpirePairing because we don't want to receive OnSessionReleased() event here
mSecureSessionHolder.Release();
if (!session->AsSecureSession()->IsActiveSession() && mSessionManager != nullptr)
if (!session.Value()->AsSecureSession()->IsActiveSession() && mSessionManager != nullptr)
{
// Make sure to clean up our pending session, since we're the only
// ones who have access to it do do so.
mSessionManager->ExpirePairing(session);
mSessionManager->ExpirePairing(session.Value());
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/transport/SessionHolder.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ class SessionHolder : public SessionDelegate, public IntrusiveListNodeBase
void Release();

operator bool() const { return mSession.HasValue(); }
SessionHandle Get() const { return SessionHandle{ mSession.Value().Get() }; }
Optional<SessionHandle> ToOptional() const
Optional<SessionHandle> Get() const
{
return mSession.HasValue() ? chip::MakeOptional<SessionHandle>(Get()) : chip::Optional<SessionHandle>::Missing();
return mSession.HasValue() ? chip::MakeOptional<SessionHandle>(mSession.Value().Get())
: chip::Optional<SessionHandle>::Missing();
mrjerryjohns marked this conversation as resolved.
Show resolved Hide resolved
}

Transport::Session * operator->() const { return &mSession.Value().Get(); }
Expand Down