Skip to content

Commit d8d1756

Browse files
committed
refactor: Rename CloseSession to MarkSessionDefunct and add MarkSessionForEviction
This commit renames the Python function CloseSession to MarkSessionDefunct to better reflect its actual behavior and align with the C++ API. The function marks a session as temporarily defunct when there's a suspected loss of synchronization with the peer, but allows the session to become active again if messages are received. Additionally, adds a new function MarkSessionForEviction that permanently marks a session for removal by detaching all SessionHolders and releasing references, which will deallocate the session if no references remain. Once marked for eviction, the session can never become active again. Both functions have been properly documented to explain their behavior, parameters, and differences. All references to CloseSession throughout the codebase have been updated to use MarkSessionDefunct.
1 parent cf45d5d commit d8d1756

File tree

9 files changed

+79
-21
lines changed

9 files changed

+79
-21
lines changed

src/controller/python/ChipDeviceController-ScriptBinding.cpp

+16-2
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,8 @@ PyChipError pychip_DeviceController_SetCheckMatchingFabric(bool check);
159159
struct IcdRegistrationParameters;
160160
PyChipError pychip_DeviceController_SetIcdRegistrationParameters(bool enabled, const IcdRegistrationParameters * params);
161161
PyChipError pychip_DeviceController_ResetCommissioningParameters();
162-
PyChipError pychip_DeviceController_CloseSession(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid);
162+
PyChipError pychip_DeviceController_MarkSessionDefunct(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid);
163+
PyChipError pychip_DeviceController_MarkSessionForEviction(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid);
163164
PyChipError pychip_DeviceController_EstablishPASESessionIP(chip::Controller::DeviceCommissioner * devCtrl, const char * peerAddrStr,
164165
uint32_t setupPINCode, chip::NodeId nodeid, uint16_t port);
165166
PyChipError pychip_DeviceController_EstablishPASESessionBLE(chip::Controller::DeviceCommissioner * devCtrl, uint32_t setupPINCode,
@@ -661,7 +662,7 @@ PyChipError pychip_DeviceController_ResetCommissioningParameters()
661662
return ToPyChipError(CHIP_NO_ERROR);
662663
}
663664

664-
PyChipError pychip_DeviceController_CloseSession(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid)
665+
PyChipError pychip_DeviceController_MarkSessionDefunct(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid)
665666
{
666667
//
667668
// Since we permit multiple controllers per fabric and each is associated with a unique fabric index, closing a session
@@ -678,6 +679,19 @@ PyChipError pychip_DeviceController_CloseSession(chip::Controller::DeviceCommiss
678679
return ToPyChipError(CHIP_NO_ERROR);
679680
}
680681

682+
PyChipError pychip_DeviceController_MarkSessionForEviction(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid)
683+
{
684+
devCtrl->SessionMgr()->ForEachMatchingSessionOnLogicalFabric(ScopedNodeId(nodeid, devCtrl->GetFabricIndex()),
685+
[](auto * session) {
686+
if (session->IsActiveSession())
687+
{
688+
session->MarkForEviction();
689+
}
690+
});
691+
692+
return ToPyChipError(CHIP_NO_ERROR);
693+
}
694+
681695
PyChipError pychip_DeviceController_EstablishPASESessionIP(chip::Controller::DeviceCommissioner * devCtrl, const char * peerAddrStr,
682696
uint32_t setupPINCode, chip::NodeId nodeid, uint16_t port)
683697
{

src/controller/python/chip/ChipDeviceCtrl.py

+51-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#
2-
# Copyright (c) 2020-2022 Project CHIP Authors
2+
# Copyright (c) 2020-2025 Project CHIP Authors
33
# Copyright (c) 2019-2020 Google, LLC.
44
# Copyright (c) 2013-2018 Nest Labs, Inc.
55
# All rights reserved.
@@ -702,12 +702,52 @@ def ExpireSessions(self, nodeid):
702702

703703
self._ChipStack.Call(lambda: self._dmLib.pychip_ExpireSessions(self.devCtrl, nodeid)).raise_on_error()
704704

705-
# TODO: This needs to be called MarkSessionDefunct
706-
def CloseSession(self, nodeid):
705+
def MarkSessionDefunct(self, nodeid):
706+
'''
707+
Marks a previously active session with the specified node as defunct to temporarily prevent it
708+
from being used with new exchanges to send or receive messages. This should be called when there
709+
is suspicion of a loss-of-sync with the session state on the associated peer, such as evidence
710+
of transport failure.
711+
712+
If messages are received thereafter on this session, the session will be put back into the Active state.
713+
714+
This function should only be called on an active session.
715+
This will NOT detach any existing SessionHolders.
716+
717+
Parameters:
718+
nodeid (int): The node ID of the device whose session should be marked as defunct.
719+
720+
Raises:
721+
RuntimeError: If the controller is not active.
722+
PyChipError: If the operation fails.
723+
'''
724+
self.CheckIsActive()
725+
726+
self._ChipStack.Call(
727+
lambda: self._dmLib.pychip_DeviceController_MarkSessionDefunct(
728+
self.devCtrl, nodeid)
729+
).raise_on_error()
730+
731+
def MarkSessionForEviction(self, nodeid):
732+
'''
733+
Marks the session with the specified node for eviction. It will first detach all SessionHolders
734+
attached to this session by calling 'OnSessionReleased' on each of them. This will force them
735+
to release their reference to the session. If there are no more references left, the session
736+
will then be de-allocated.
737+
738+
Once marked for eviction, the session SHALL NOT ever become active again.
739+
740+
Parameters:
741+
nodeid (int): The node ID of the device whose session should be marked for eviction.
742+
743+
Raises:
744+
RuntimeError: If the controller is not active.
745+
PyChipError: If the operation fails.
746+
'''
707747
self.CheckIsActive()
708748

709749
self._ChipStack.Call(
710-
lambda: self._dmLib.pychip_DeviceController_CloseSession(
750+
lambda: self._dmLib.pychip_DeviceController_MarkSessionForEviction(
711751
self.devCtrl, nodeid)
712752
).raise_on_error()
713753

@@ -1873,9 +1913,13 @@ def _InitLib(self):
18731913
c_void_p, c_uint64, _DeviceUnpairingCompleteFunct]
18741914
self._dmLib.pychip_DeviceController_UnpairDevice.restype = PyChipError
18751915

1876-
self._dmLib.pychip_DeviceController_CloseSession.argtypes = [
1916+
self._dmLib.pychip_DeviceController_MarkSessionDefunct.argtypes = [
18771917
c_void_p, c_uint64]
1878-
self._dmLib.pychip_DeviceController_CloseSession.restype = PyChipError
1918+
self._dmLib.pychip_DeviceController_MarkSessionDefunct.restype = PyChipError
1919+
1920+
self._dmLib.pychip_DeviceController_MarkSessionForEviction.argtypes = [
1921+
c_void_p, c_uint64]
1922+
self._dmLib.pychip_DeviceController_MarkSessionForEviction.restype = PyChipError
18791923

18801924
self._dmLib.pychip_DeviceController_GetAddressAndPort.argtypes = [
18811925
c_void_p, c_uint64, c_char_p, c_uint64, POINTER(c_uint16)]
@@ -2261,7 +2305,7 @@ async def CommissionOnNetwork(self, nodeId: int, setupPinCode: int,
22612305
return await asyncio.futures.wrap_future(ctx.future)
22622306

22632307
def get_rcac(self):
2264-
'''
2308+
'''
22652309
Passes captured RCAC data back to Python test modules for validation
22662310
- Setting buffer size to max size mentioned in spec:
22672311
- Ref: https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/06c4d55962954546ecf093c221fe1dab57645028/policies/matter_certificate_policy.adoc#615-key-sizes

src/controller/python/chip/commissioning/pase.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ def __enter__(self) -> Session:
3939
device=self.devCtrl.GetConnectedDeviceSync(self.node_id, allowPASE=True, timeoutMs=1000))
4040

4141
def __exit__(self, type, value, traceback):
42-
self.devCtrl.CloseSession(self.node_id)
42+
self.devCtrl.MarkSessionDefunct(self.node_id)
4343
if self.is_ble:
4444
self.devCtrl.CloseBLEConnection(self.is_ble)
4545

src/controller/python/test/test_scripts/base.py

+6-6
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,7 @@ async def TestCaseEviction(self, nodeid: int):
671671
# the controller and target.
672672
#
673673
for x in range(minimumSupportedFabrics * minimumCASESessionsPerFabric * 2):
674-
self.devCtrl.CloseSession(nodeid)
674+
self.devCtrl.MarkSessionDefunct(nodeid)
675675
await self.devCtrl.ReadAttribute(nodeid, [(Clusters.BasicInformation.Attributes.ClusterRevision)])
676676

677677
self.logger.info("Testing CASE defunct logic")
@@ -700,7 +700,7 @@ def OnValueChange(path: Attribute.TypedAttributePath, transaction: Attribute.Sub
700700
#
701701
# This marks the session defunct.
702702
#
703-
self.devCtrl.CloseSession(nodeid)
703+
self.devCtrl.MarkSessionDefunct(nodeid)
704704

705705
#
706706
# Now write the attribute from fabric2, give it some time before checking if the report
@@ -732,7 +732,7 @@ def OnValueChange(path: Attribute.TypedAttributePath, transaction: Attribute.Sub
732732
sub.SetAttributeUpdateCallback(OnValueChange)
733733

734734
for x in range(minimumSupportedFabrics * minimumCASESessionsPerFabric * 2):
735-
self.devCtrl2.CloseSession(nodeid)
735+
self.devCtrl2.MarkSessionDefunct(nodeid)
736736
await self.devCtrl2.ReadAttribute(nodeid, [(Clusters.BasicInformation.Attributes.ClusterRevision)])
737737

738738
#
@@ -760,7 +760,7 @@ def OnValueChange(path: Attribute.TypedAttributePath, transaction: Attribute.Sub
760760
sub.SetAttributeUpdateCallback(OnValueChange)
761761

762762
for x in range(minimumSupportedFabrics * minimumCASESessionsPerFabric * 2):
763-
self.devCtrl.CloseSession(nodeid)
763+
self.devCtrl.MarkSessionDefunct(nodeid)
764764
await self.devCtrl.ReadAttribute(nodeid, [(Clusters.BasicInformation.Attributes.ClusterRevision)])
765765

766766
await self.devCtrl.WriteAttribute(nodeid, [(1, Clusters.UnitTesting.Attributes.Int8u(6))])
@@ -1072,7 +1072,7 @@ async def OnResubscriptionSucceeded(transaction):
10721072
def TestCloseSession(self, nodeid: int):
10731073
self.logger.info(f"Closing sessions with device {nodeid}")
10741074
try:
1075-
self.devCtrl.CloseSession(nodeid)
1075+
self.devCtrl.MarkSessionDefunct(nodeid)
10761076
return True
10771077
except Exception as ex:
10781078
self.logger.exception(
@@ -1476,7 +1476,7 @@ async def TestSubscriptionResumptionCapacityStep2(self, nodeid: int, endpoint: i
14761476

14771477
self.logger.info("Send a new subscription request from the second controller")
14781478
# Close previous session so that the second controller will res-establish the session with the remote device
1479-
self.devCtrl.CloseSession(nodeid)
1479+
self.devCtrl.MarkSessionDefunct(nodeid)
14801480
await self.devCtrl.ReadAttribute(nodeid, [(endpoint, Clusters.BasicInformation.Attributes.NodeLabel)], None,
14811481
False, reportInterval=(1, 50),
14821482
keepSubscriptions=True, autoResubscribe=False)

src/python_testing/TC_CGEN_2_8.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ async def test_TC_CGEN_2_8(self):
130130
)
131131

132132
# Close the commissioner session with the device to clean up resources
133-
commissioner.CloseSession(nodeid=self.dut_node_id)
133+
commissioner.MarkSessionDefunct(nodeid=self.dut_node_id)
134134

135135
# Step 5: Factory reset is handled by test operator
136136
self.step(5)

src/python_testing/TC_CGEN_2_9.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ async def test_TC_CGEN_2_9(self):
158158
await self.remove_commissioner_fabric()
159159

160160
# Close the commissioner session with the device to clean up resources
161-
commissioner.CloseSession(nodeid=self.dut_node_id)
161+
commissioner.MarkSessionDefunct(nodeid=self.dut_node_id)
162162

163163
# Step 6: Put device in commissioning mode (requiring user input, so skip in CI)
164164
self.step(6)

src/python_testing/TC_ICDM_3_4.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ async def test_TC_ICDM_3_4(self):
128128
paaTrustStorePath=str(self.config.paa_trust_store_path),
129129
catTags=self.config.controller_cat_tags
130130
)
131-
devCtrl.CloseSession(self.dut_node_id)
131+
devCtrl.MarkSessionDefunct(self.dut_node_id)
132132
icdCounter2 = await self._read_icdm_attribute_expect_success(attribute=attributes.ICDCounter)
133133
asserts.assert_greater_equal(icdCounter2, icdCounter1,
134134
"ICDCounter have reboot is not greater or equal to the ICDCounter read before the reboot.")

src/test_driver/mbed/integration_tests/common/utils.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ def close_connection(devCtrl, nodeId):
235235
:return: true if successful, otherwise false
236236
"""
237237
try:
238-
devCtrl.CloseSession(nodeId)
238+
devCtrl.MarkSessionDefunct(nodeId)
239239
except exceptions.ChipStackException as ex:
240240
log.error("Close session failed: {}".format(str(ex)))
241241
return False

src/test_driver/openiotsdk/integration-tests/common/utils.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ def disconnect_device(devCtrl, nodeId):
108108
:return: node ID if connection successful or None if failed
109109
"""
110110
try:
111-
devCtrl.CloseSession(nodeId)
111+
devCtrl.MarkSessionDefunct(nodeId)
112112
except exceptions.ChipStackException as ex:
113113
log.error("CloseSession failed {}".format(str(ex)))
114114
return False

0 commit comments

Comments
 (0)