Skip to content

Commit

Permalink
Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
tehampson committed Aug 4, 2022
1 parent af29fe6 commit ead0907
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 13 deletions.
2 changes: 1 addition & 1 deletion src/app/clusters/ota-requestor/DefaultOTARequestor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ void DefaultOTARequestor::DisconnectFromProvider()
}

auto providerNodeId = GetProviderScopedId();
mServer->GetSecureSessionManager().MarkSessionAsDefunct(providerNodeId, MakeOptional(Transport::SecureSession::Type::kCASE));
mServer->GetSecureSessionManager().MarkSessionsAsDefunct(providerNodeId, MakeOptional(Transport::SecureSession::Type::kCASE));
}

// Requestor is directed to cancel image update in progress. All the Requestor state is
Expand Down
2 changes: 1 addition & 1 deletion src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ CHIP_ERROR DeviceController::DisconnectDevice(NodeId nodeId)
{
ChipLogProgress(Controller, "Force close session for node 0x%" PRIx64, nodeId);

if (SessionMgr()->MarkSessionAsDefunct(GetPeerScopedId(nodeId), MakeOptional(Transport::SecureSession::Type::kCASE)))
if (SessionMgr()->MarkSessionsAsDefunct(GetPeerScopedId(nodeId), MakeOptional(Transport::SecureSession::Type::kCASE)))
{
return CHIP_NO_ERROR;
}
Expand Down
6 changes: 6 additions & 0 deletions src/controller/python/chip/ChipDeviceCtrl.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ class DCState(enum.IntEnum):


class DeviceProxyWrapper():
''' Encapsulates a pointer to OperationalDeviceProxy on the c++ side that needs to be
freed when DeviceProxyWrapper goes out of scope. There is a potential issue where
if this is copied around that a double free will occure, but how this is used today
that is not an issue that needs to be accounted for and it will become very apparent
if that happens.
'''
def __init__(self, deviceProxy: ctypes.c_void_p, dmLib=None):
self._deviceProxy = deviceProxy
self._dmLib = dmLib
Expand Down
21 changes: 13 additions & 8 deletions src/transport/SessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -445,15 +445,20 @@ void SessionManager::ExpireAllPASESessions()
});
}

bool SessionManager::MarkSessionAsDefunct(const ScopedNodeId & node, const Optional<Transport::SecureSession::Type> & type)
bool SessionManager::MarkSessionsAsDefunct(const ScopedNodeId & node, const Optional<Transport::SecureSession::Type> & type)
{
auto optionalSessionHandle = FindSecureSessionForNode(node, type);
if (optionalSessionHandle.HasValue())
{
optionalSessionHandle.Value()->AsSecureSession()->MarkAsDefunct();
return true;
}
return false;
bool found = false;
mSecureSessions.ForEachSession([&node, &type, &found](auto session) {
if (session->IsActiveSession() && session->GetPeer() == node &&
(!type.HasValue() || type.Value() == session->GetSecureSessionType()))
{
session->AsSecureSession()->MarkAsDefunct();
found = true;
}
return Loop::Continue;
});

return found;
}

Optional<SessionHandle> SessionManager::AllocateSession(SecureSession::Type secureSessionType,
Expand Down
6 changes: 3 additions & 3 deletions src/transport/SessionManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -351,14 +351,14 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate, public FabricTabl

/**
* @brief
* Mark most recently active session that matches provided arguments as defunct.
* Marks all active session that matches provided arguments as defunct.
*
* @param node Scoped node ID of the session we should mark as defunct.
* @param node Scoped node ID of the active sessions we should mark as defunct.
* @param type Type of session we are looking to mark as defunct. If matching
* against all types of sessions is desired, NullOptional should
* be passed into type.
*/
bool MarkSessionAsDefunct(const ScopedNodeId & node, const Optional<Transport::SecureSession::Type> & type);
bool MarkSessionsAsDefunct(const ScopedNodeId & node, const Optional<Transport::SecureSession::Type> & type);

/**
* @brief
Expand Down

0 comments on commit ead0907

Please sign in to comment.