Skip to content

Commit

Permalink
Fix session holder.
Browse files Browse the repository at this point in the history
  • Loading branch information
mrjerryjohns committed May 16, 2022
1 parent 1e45e8a commit f62a0b4
Show file tree
Hide file tree
Showing 14 changed files with 50 additions and 23 deletions.
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);
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)

// clang-format on

Expand Down
9 changes: 8 additions & 1 deletion src/messaging/ExchangeContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,15 @@ 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
// 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);

// Create a new scope for `err`, to avoid shadowing warning previous `err`.
CHIP_ERROR err = mDispatch.SendMessage(GetExchangeMgr()->GetSessionManager(), mSession.Get(), mExchangeId, IsInitiator(),
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())
Expand Down
7 changes: 6 additions & 1 deletion src/messaging/ExchangeContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,12 @@ 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
5 changes: 2 additions & 3 deletions src/transport/SessionHolder.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,9 @@ 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();
}

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

0 comments on commit f62a0b4

Please sign in to comment.