Merge branch 'master' into feature/fix-tv-app-install-flow-and-suppor…

…ted-clusters
project-chip · Jun 22, 2024 · 04a996d · 04a996d
2 parents d7f3aea + c829a2c
commit 04a996d
Show file tree

Hide file tree

Showing 22 changed files with 199 additions and 25 deletions.
diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml
diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp
@@ -596,6 +596,12 @@ void Server::Shutdown()
 #if CHIP_CONFIG_ENABLE_ICD_SERVER
     app::InteractionModelEngine::GetInstance()->SetICDManager(nullptr);
 #endif // CHIP_CONFIG_ENABLE_ICD_SERVER
+    // Shut down any remaining sessions (and hence exchanges) before we do any
+    // futher teardown.  CASE handshakes have been shut down already via
+    // shutting down mCASESessionManager and mCASEServer above; shutting
+    // down mCommissioningWindowManager will shut down any PASE handshakes we
+    // have going on.
+    mSessions.ExpireAllSecureSessions();
     mCommissioningWindowManager.Shutdown();
     mMessageCounterManager.Shutdown();
     mExchangeMgr.Shutdown();

diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp
@@ -476,6 +476,15 @@ void DeviceControllerSystemState::Shutdown()
         mCASESessionManager = nullptr;
     }
 
+    // The above took care of CASE handshakes, and shutting down all the
+    // controllers should have taken care of the PASE handshakes.  Clean up any
+    // outstanding secure sessions (shouldn't really be any, since controllers
+    // should have handled that, but just in case).
+    if (mSessionMgr != nullptr)
+    {
+        mSessionMgr->ExpireAllSecureSessions();
+    }
+
     // mCASEClientPool and mSessionSetupPool must be deallocated
     // after mCASESessionManager, which uses them.
 

diff --git a/src/messaging/ExchangeMgr.h b/src/messaging/ExchangeMgr.h
@@ -80,9 +80,13 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate
      *  Shutdown the ExchangeManager. This terminates this instance
      *  of the object and releases all held resources.
      *
+     * Please see documentation for SessionManager::Shutdown() for ordering
+     * dependecies between that and this Shutdown() method.
+     *
      *  @note
      *     The protocol should only call this function after ensuring that
-     *     there are no active ExchangeContext objects. Furthermore, it is the
+     *     there are no active ExchangeContext objects (again, see
+     *     SessionManager::Shutdown() documentation). Furthermore, it is the
      *     onus of the application to de-allocate the ExchangeManager
      *     object after calling ExchangeManager::Shutdown().
      */

diff --git a/src/python_testing/TC_TIMESYNC_2_1.py b/src/python_testing/TC_TIMESYNC_2_1.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import ipaddress
 from datetime import timedelta
 

diff --git a/src/python_testing/TC_TIMESYNC_2_10.py b/src/python_testing/TC_TIMESYNC_2_10.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import queue
 import time
 import typing

diff --git a/src/python_testing/TC_TIMESYNC_2_11.py b/src/python_testing/TC_TIMESYNC_2_11.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import queue
 import time
 import typing

diff --git a/src/python_testing/TC_TIMESYNC_2_12.py b/src/python_testing/TC_TIMESYNC_2_12.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import queue
 import time
 import typing

diff --git a/src/python_testing/TC_TIMESYNC_2_13.py b/src/python_testing/TC_TIMESYNC_2_13.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import queue
 import time
 

diff --git a/src/python_testing/TC_TIMESYNC_2_2.py b/src/python_testing/TC_TIMESYNC_2_2.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 from datetime import timedelta
 
 import chip.clusters as Clusters

diff --git a/src/python_testing/TC_TIMESYNC_2_4.py b/src/python_testing/TC_TIMESYNC_2_4.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import typing
 from datetime import timedelta
 

diff --git a/src/python_testing/TC_TIMESYNC_2_5.py b/src/python_testing/TC_TIMESYNC_2_5.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import typing
 
 import chip.clusters as Clusters

diff --git a/src/python_testing/TC_TIMESYNC_2_6.py b/src/python_testing/TC_TIMESYNC_2_6.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import typing
 
 import chip.clusters as Clusters

diff --git a/src/python_testing/TC_TIMESYNC_2_7.py b/src/python_testing/TC_TIMESYNC_2_7.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import time
 import typing
 from datetime import timedelta

diff --git a/src/python_testing/TC_TIMESYNC_2_8.py b/src/python_testing/TC_TIMESYNC_2_8.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import time
 import typing
 from datetime import datetime, timedelta, timezone

diff --git a/src/python_testing/TC_TIMESYNC_2_9.py b/src/python_testing/TC_TIMESYNC_2_9.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --PICS src/app/tests/suites/certification/ci-pics-values --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import typing
 from datetime import timedelta
 

diff --git a/src/python_testing/TC_TIMESYNC_3_1.py b/src/python_testing/TC_TIMESYNC_3_1.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import chip.clusters as Clusters
 from matter_testing_support import MatterBaseTest, async_test_body, default_matter_test_main
 from mobly import asserts

diff --git a/src/python_testing/TC_TestEventTrigger.py b/src/python_testing/TC_TestEventTrigger.py
@@ -15,6 +15,13 @@
 #    limitations under the License.
 #
 
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json --enable-key 000102030405060708090a0b0c0d0e0f
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --bool-arg allow_sdk_dac:true --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import logging
 
 import chip.clusters as Clusters

diff --git a/src/python_testing/TestBatchInvoke.py b/src/python_testing/TestBatchInvoke.py
@@ -14,6 +14,14 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 #
+
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import logging
 
 import chip.clusters as Clusters

diff --git a/src/python_testing/TestGroupTableReports.py b/src/python_testing/TestGroupTableReports.py
@@ -14,6 +14,14 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 #
+
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+
 import logging
 import queue
 from typing import List

diff --git a/src/transport/SessionManager.cpp b/src/transport/SessionManager.cpp
@@ -166,10 +166,15 @@ void SessionManager::Shutdown()
     // Ensure that we don't create new sessions as we iterate our session table.
     mState = State::kNotReady;
 
-    mSecureSessions.ForEachSession([&](auto session) {
-        session->MarkForEviction();
-        return Loop::Continue;
-    });
+    // Just in case some consumer forgot to do it, expire all our secure
+    // sessions.  Note that this stands a good chance of crashing with a
+    // null-deref if there are in fact any secure sessions left, since they will
+    // try to notify their exchanges, which will then try to operate on
+    // partially-shut-down objects.
+    ExpireAllSecureSessions();
+
+    // We don't have a safe way to check or affect the state of our
+    // mUnauthenticatedSessions.  We can only hope they got shut down properly.
 
     mMessageCounterManager = nullptr;
 
@@ -542,6 +547,14 @@ void SessionManager::ExpireAllPASESessions()
     });
 }
 
+void SessionManager::ExpireAllSecureSessions()
+{
+    mSecureSessions.ForEachSession([&](auto session) {
+        session->MarkForEviction();
+        return Loop::Continue;
+    });
+}
+
 void SessionManager::MarkSessionsAsDefunct(const ScopedNodeId & node, const Optional<Transport::SecureSession::Type> & type)
 {
     mSecureSessions.ForEachSession([&node, &type](auto session) {

diff --git a/src/transport/SessionManager.h b/src/transport/SessionManager.h
@@ -345,7 +345,20 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate, public FabricTabl
         return CHIP_NO_ERROR;
     }
 
+    /**
+     * Expire all sessions for a given peer, as identified by a specific fabric
+     * index and node ID.
+     */
     void ExpireAllSessions(const ScopedNodeId & node);
+
+    /**
+     * Expire all sessions associated with the given fabric index.
+     *
+     * *NOTE* This is generally all sessions for a given fabric _EXCEPT_ if there are multiple
+     *        FabricInfo instances in the FabricTable that collide on the same logical fabric (i.e
+     *        root public key + fabric ID tuple).  This can ONLY happen if multiple controller
+     *        instances on the same fabric is permitted and each is assigned a unique fabric index.
+     */
     void ExpireAllSessionsForFabric(FabricIndex fabricIndex);
 
     /**
@@ -376,6 +389,12 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate, public FabricTabl
 
     void ExpireAllPASESessions();
 
+    /**
+     * Expire all secure sessions.  See documentation for Shutdown on when it's
+     * appropriate to use this.
+     */
+    void ExpireAllSecureSessions();
+
     /**
      * @brief
      *   Marks all active sessions that match provided arguments as defunct.
@@ -422,6 +441,15 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate, public FabricTabl
      * @brief
      *  Shutdown the Secure Session Manager. This terminates this instance
      *  of the object and reset it's state.
+     *
+     * The proper order of shutdown for SessionManager is as follows:
+     *
+     * 1) Call ExpireAllSecureSessions() on the SessionManager, and ensure that any unauthenticated
+     *    sessions (e.g. CASEServer and CASESessionManager instances, or anything that does PASE
+     *    handshakes) are released.
+     * 2) Shut down the exchange manager, so that it's no longer referencing
+     *    the to-be-shut-down SessionManager.
+     * 3) Shut down the SessionManager.
      */
     void Shutdown();