Skip to content

Commit

Permalink
Updates to fix fallout from #19502.
Browse files Browse the repository at this point in the history
We need to allow messages on inactive sessions to reach the exchange manager,
because such sessions need to be able to deliver an MRP ack to an exchange
waiting for one.

We also don't want to crash on an attempt to transition from Inactive to Defunct
state; the transition should just be ignored.  This way if we start trying to
transitionin to Defunct on MRP delivery failures we will not start crashing if
such a failure happens on an Inactive session.
  • Loading branch information
bzbarsky-apple committed Jun 21, 2022
1 parent 89c4bac commit 3a0aaf1
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 4 deletions.
8 changes: 7 additions & 1 deletion src/messaging/ExchangeMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,13 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const
packetHeader.GetDestinationGroupId().Value());
}

// Do not handle unsolicited messages on a inactive session.
// Do not handle messages that don't match an existing exchange on a
// inactive session, since we should not be creating new exchanges there.
if (!session->IsActiveSession()) {
ChipLogProgress(ExchangeManager, "Dropping message on inactive session that does not match an existing exchange");
return;
}

// If it's not a duplicate message, search for an unsolicited message handler if it is marked as being sent by an initiator.
// Since we didn't find an existing exchange that matches the message, it must be an unsolicited message. However all
// unsolicited messages must be marked as being from an initiator.
Expand Down
11 changes: 9 additions & 2 deletions src/transport/SecureSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ const char * SecureSession::StateToString(State state) const
return "kPendingEviction";
break;

case State::kInactive:
return "kInactive";
break;

default:
return "???";
break;
Expand Down Expand Up @@ -89,9 +93,12 @@ void SecureSession::MarkAsDefunct()

case State::kInactive:
//
// Once a session is marked Inactive, we CANNOT bring it back to either being active or defunct.
// Once a session is marked Inactive, we CANNOT bring it back to either
// being active or defunct. But consumers may not really know this
// session is already inactive. Just ignore the call and stay in
// kInactive state.
//
FALLTHROUGH;
return;
case State::kPendingEviction:
//
// Once a session is headed for eviction, we CANNOT bring it back to either being active or defunct.
Expand Down
1 change: 1 addition & 0 deletions src/transport/SecureSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ class SecureSession : public Session, public ReferenceCounted<SecureSession, Sec
bool IsEstablishing() const { return mState == State::kEstablishing; }
bool IsPendingEviction() const { return mState == State::kPendingEviction; }
bool IsDefunct() const { return mState == State::kDefunct; }
bool IsInactive() const { return mState == State::kInactive; }
const char * GetStateStr() const { return StateToString(mState); }

/*
Expand Down
2 changes: 1 addition & 1 deletion src/transport/SessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ void SessionManager::SecureUnicastMessageDispatch(const PacketHeader & packetHea

Transport::SecureSession * secureSession = session.Value()->AsSecureSession();

if (!secureSession->IsDefunct() && !secureSession->IsActiveSession())
if (!secureSession->IsDefunct() && !secureSession->IsActiveSession() && !secureSession->IsInactive())
{
ChipLogError(Inet, "Secure transport received message on a session in an invalid state (state = '%s')",
secureSession->GetStateStr());
Expand Down

0 comments on commit 3a0aaf1

Please sign in to comment.