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.

Sign up for GitHub

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

Improve logic for deciding whether to re-establish CASE in ReadClient. #23627

Merged
merged 2 commits into from
Nov 17, 2022
Merged

Improve logic for deciding whether to re-establish CASE in ReadClient. #23627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee28e16
Improve logic for deciding whether to re-establish CASE in ReadClient.
bzbarsky-apple Nov 16, 2022
f4a1615
Address review comments.
bzbarsky-apple Nov 16, 2022
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
60 changes: 40 additions & 20 deletions src/app/ReadClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,14 @@ CHIP_ERROR ReadClient::ScheduleResubscription(uint32_t aTimeTillNextResubscripti
mReadPrepareParams.mSessionHolder.Grab(aNewSessionHandle.Value());
}

mDoCaseOnNextResub = aReestablishCASE;
mForceCaseOnNextResub = aReestablishCASE;
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
if (mForceCaseOnNextResub && mReadPrepareParams.mSessionHolder)
{
// Mark our existing session defunct, so that we will try to
// re-establish it when the timer fires (unless something re-establishes
// before then).
mReadPrepareParams.mSessionHolder->AsSecureSession()->MarkAsDefunct();
}

ReturnErrorOnFailure(
InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer(
Expand Down Expand Up @@ -1014,7 +1021,16 @@ CHIP_ERROR ReadClient::SendSubscribeRequestImpl(const ReadPrepareParams & aReadP
VerifyOrReturnError(aReadPrepareParams.mSessionHolder, CHIP_ERROR_MISSING_SECURE_SESSION);

auto exchange = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHolder.Get().Value(), this);
VerifyOrReturnError(exchange != nullptr, CHIP_ERROR_NO_MEMORY);
if (exchange == nullptr)
{
if (aReadPrepareParams.mSessionHolder->AsSecureSession()->IsActiveSession())
{
return CHIP_ERROR_NO_MEMORY;
}

// Trying to subscribe with a defunct session somehow.
return CHIP_ERROR_INCORRECT_STATE;
}
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved

mExchange.Grab(exchange);

Expand Down Expand Up @@ -1082,30 +1098,34 @@ void ReadClient::OnResubscribeTimerCallback(System::Layer * apSystemLayer, void

CHIP_ERROR err;

ChipLogProgress(DataManagement, "OnResubscribeTimerCallback: DoCASE = %d", _this->mDoCaseOnNextResub);
ChipLogProgress(DataManagement, "OnResubscribeTimerCallback: ForceCASE = %d", _this->mForceCaseOnNextResub);
_this->mNumRetries++;

if (_this->mDoCaseOnNextResub)
bool allowResubscribeOnError = true;
if (!_this->mReadPrepareParams.mSessionHolder ||
!_this->mReadPrepareParams.mSessionHolder->AsSecureSession()->IsActiveSession())
{
// We don't have an active CASE session. We need to go ahead and set
// one up, if we can.
ChipLogProgress(DataManagement, "Trying to establish a CASE session");
auto * caseSessionManager = InteractionModelEngine::GetInstance()->GetCASESessionManager();
VerifyOrExit(caseSessionManager != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
if (caseSessionManager)
{
caseSessionManager->FindOrEstablishSession(_this->mPeer, &_this->mOnConnectedCallback,
&_this->mOnConnectionFailureCallback);
return;
}

//
// We need to mark our session as defunct explicitly since the assessment of a liveness failure
// is usually triggered by the absence of any exchange activity that would have otherwise
// automatically marked the session as defunct on a response timeout.
//
// Doing so will ensure that the subsequent call to FindOrEstablishSession will not bind to
// an existing established session but rather trigger establishing a new one.
//
if (_this->mReadPrepareParams.mSessionHolder)
if (_this->mForceCaseOnNextResub)
{
_this->mReadPrepareParams.mSessionHolder->AsSecureSession()->MarkAsDefunct();
// Caller asked us to force CASE but we have no way to do CASE.
// Just stop trying.
allowResubscribeOnError = false;
}

caseSessionManager->FindOrEstablishSession(_this->mPeer, &_this->mOnConnectedCallback,
&_this->mOnConnectionFailureCallback);
return;
// No way to send our subscribe request.
err = CHIP_ERROR_INCORRECT_STATE;
ExitNow();
}

err = _this->SendSubscribeRequest(_this->mReadPrepareParams);
Expand All @@ -1115,11 +1135,11 @@ void ReadClient::OnResubscribeTimerCallback(System::Layer * apSystemLayer, void
{
//
// Call Close (which should trigger re-subscription again) EXCEPT if we got here because we didn't have a valid
// CASESessionManager pointer when mDoCaseOnNextResub was true.
// CASESessionManager pointer when mForceCaseOnNextResub was true.
//
// In that case, don't permit re-subscription to occur.
//
_this->Close(err, err != CHIP_ERROR_INCORRECT_STATE);
_this->Close(err, allowResubscribeOnError);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/app/ReadClient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ class ReadClient : public Messaging::ExchangeDelegate
InteractionType mInteractionType = InteractionType::Read;
Timestamp mEventTimestamp;

bool mDoCaseOnNextResub = true;
bool mForceCaseOnNextResub = true;

chip::Callback::Callback<OnDeviceConnected> mOnConnectedCallback;
chip::Callback::Callback<OnDeviceConnectionFailure> mOnConnectionFailureCallback;
Expand Down