[Python] Fix threading model

src/controller/python/BUILD.gn

@@ -44,6 +44,8 @@ shared_library("ChipDeviceCtrl") {
     "chip/setup_payload/Parser.cpp",
   ]
 
+  sources += [ "CommonStackInit.cpp" ]
+
   if (chip_controller) {
     sources += [
       "ChipCommissionableNodeController-ScriptBinding.cpp",
@@ -63,7 +65,6 @@ shared_library("ChipDeviceCtrl") {
       "chip/internal/ChipThreadWork.h",
       "chip/internal/CommissionerImpl.cpp",
       "chip/logging/LoggingRedirect.cpp",
-      "chip/native/StackInit.cpp",
     ]
   } else {
     sources += [

src/controller/python/ChipDeviceController-ScriptBinding.cpp

@@ -40,12 +40,6 @@
 #include <inttypes.h>
 #include <net/if.h>
 
-#include "ChipDeviceController-ScriptDevicePairingDelegate.h"
-#include "ChipDeviceController-StorageDelegate.h"
-
-#include "chip/interaction_model/Delegate.h"
-
-#include <app/CommandSender.h>
 #include <app/DeviceProxy.h>
 #include <app/InteractionModelEngine.h>
 #include <app/server/Dnssd.h>
@@ -55,6 +49,12 @@
 #include <controller/CommissioningDelegate.h>
 #include <controller/CommissioningWindowOpener.h>
 #include <controller/ExampleOperationalCredentialsIssuer.h>
+
+#include <controller/python/ChipDeviceController-ScriptDevicePairingDelegate.h>
+#include <controller/python/ChipDeviceController-StorageDelegate.h>
+#include <controller/python/CommonStackInit.h>
+#include <controller/python/chip/interaction_model/Delegate.h>
+
 #include <credentials/GroupDataProviderImpl.h>
 #include <credentials/PersistentStorageOpCertStore.h>
 #include <credentials/attestation_verifier/DefaultDeviceAttestationVerifier.h>
@@ -71,6 +71,10 @@
 #include <setup_payload/QRCodeSetupPayloadParser.h>
 #include <system/SystemClock.h>
 
+#include <platform/CommissionableDataProvider.h>
+#include <platform/PlatformManager.h>
+#include <platform/TestOnlyCommissionableDataProvider.h>
+
 using namespace chip;
 using namespace chip::Ble;
 using namespace chip::Controller;
@@ -106,7 +110,8 @@ chip::NodeId kDefaultLocalDeviceId = chip::kTestControllerNodeId;
 chip::NodeId kRemoteDeviceId       = chip::kTestDeviceNodeId;
 
 extern "C" {
-ChipError::StorageType pychip_DeviceController_StackInit();
+ChipError::StorageType pychip_DeviceController_StackInit(uint32_t bluetoothAdapterId);
+ChipError::StorageType pychip_DeviceController_StackShutdown();
 
 ChipError::StorageType pychip_DeviceController_NewDeviceController(chip::Controller::DeviceCommissioner ** outDevCtrl,
                                                                    chip::NodeId localDeviceId, bool useTestCommissioner);
@@ -176,8 +181,6 @@ ChipError::StorageType pychip_DeviceCommissioner_CloseBleConnection(chip::Contro
 uint8_t pychip_DeviceController_GetLogFilter();
 void pychip_DeviceController_SetLogFilter(uint8_t category);
 
-ChipError::StorageType pychip_Stack_Init();
-ChipError::StorageType pychip_Stack_Shutdown();
 const char * pychip_Stack_ErrorToString(ChipError::StorageType err);
 const char * pychip_Stack_StatusReportToString(uint32_t profileId, uint16_t statusCode);
 void pychip_Stack_SetLogFunct(LogMessageFunct logFunct);
@@ -187,8 +190,6 @@ ChipError::StorageType pychip_GetConnectedDeviceByNodeId(chip::Controller::Devic
 ChipError::StorageType pychip_GetDeviceBeingCommissioned(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId,
                                                          CommissioneeDeviceProxy ** proxy);
 uint64_t pychip_GetCommandSenderHandle(chip::DeviceProxy * device);
-// CHIP Stack objects
-ChipError::StorageType pychip_BLEMgrImpl_ConfigureBle(uint32_t bluetoothAdapterId);
 
 chip::ChipError::StorageType pychip_InteractionModel_ShutdownSubscription(SubscriptionId subscriptionId);
 
@@ -220,10 +221,19 @@ chip::Controller::Python::StorageAdapter * pychip_Storage_GetStorageAdapter()
     return sStorageAdapter;
 }
 
-ChipError::StorageType pychip_DeviceController_StackInit()
+ChipError::StorageType pychip_DeviceController_StackInit(uint32_t bluetoothAdapterId)
 {
+    ReturnErrorOnFailure(chip::Controller::Python::CommonStackInit().AsInteger());
+
     VerifyOrDie(sStorageAdapter != nullptr);
 
+#if CHIP_DEVICE_LAYER_TARGET_LINUX && CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE
+    // By default, Linux device is configured as a BLE peripheral while the controller needs a BLE central.
+    CHIP_ERROR err =
+        chip::DeviceLayer::Internal::BLEMgrImpl().ConfigureBle(/* BLE adapter ID */ bluetoothAdapterId, /* BLE central */ true);
+    VerifyOrReturnError(err == CHIP_NO_ERROR, err.AsInteger());
+#endif
+
     FactoryInitParams factoryParams;
     factoryParams.fabricIndependentStorage = sStorageAdapter;
 
@@ -236,6 +246,12 @@ ChipError::StorageType pychip_DeviceController_StackInit()
 
     factoryParams.enableServerInteractions = true;
 
+    // Hack needed due to the fact that DnsSd server uses the CommissionableDataProvider even
+    // when never starting operational advertising. This will not be used but prevents
+    // null pointer dereferences.
+    static chip::DeviceLayer::TestOnlyCommissionableDataProvider TestOnlyCommissionableDataProvider;
+    chip::DeviceLayer::SetCommissionableDataProvider(&TestOnlyCommissionableDataProvider);
+
     ReturnErrorOnFailure(DeviceControllerFactory::GetInstance().Init(factoryParams).AsInteger());
 
     //
@@ -249,6 +265,34 @@ ChipError::StorageType pychip_DeviceController_StackInit()
     //
     DeviceControllerFactory::GetInstance().RetainSystemState();
 
+    //
+    // Finally, start up the main Matter thread. Any further interactions with the stack
+    // will now need to happen on the Matter thread, OR protected with the stack lock.
+    //
+    ReturnErrorOnFailure(chip::DeviceLayer::PlatformMgr().StartEventLoopTask().AsInteger());
+
+    return CHIP_NO_ERROR.AsInteger();
+}
+
+ChipError::StorageType pychip_DeviceController_StackShutdown()
+{
+    ChipLogError(Controller, "Shutting down the stack...");
+
+    //
+    // Let's stop the Matter thread, and wait till the event loop has stopped.
+    //
+    ReturnErrorOnFailure(chip::DeviceLayer::PlatformMgr().StopEventLoopTask().AsInteger());
+
+    //
+    // There is the symmetric call to match the Retain called at stack initialization
+    // time. This will release all resources (if there are no other controllers active).
+    //
+    DeviceControllerFactory::GetInstance().ReleaseSystemState();
+
+    DeviceControllerFactory::GetInstance().Shutdown();
+
+    chip::Controller::Python::CommonStackShutdown();
+
     return CHIP_NO_ERROR.AsInteger();
 }
 
@@ -557,38 +601,6 @@ ChipError::StorageType pychip_ScriptDevicePairingDelegate_SetCommissioningStatus
     return CHIP_NO_ERROR.AsInteger();
 }
 
-ChipError::StorageType pychip_Stack_Init()
-{
-    CHIP_ERROR err = CHIP_NO_ERROR;
-
-    err = chip::Platform::MemoryInit();
-    SuccessOrExit(err);
-
-#if !CHIP_SYSTEM_CONFIG_USE_SOCKETS
-
-    ExitNow(err = CHIP_ERROR_NOT_IMPLEMENTED);
-
-#else /* CHIP_SYSTEM_CONFIG_USE_SOCKETS */
-
-#endif /* CHIP_SYSTEM_CONFIG_USE_SOCKETS */
-
-exit:
-    if (err != CHIP_NO_ERROR)
-        pychip_Stack_Shutdown();
-
-    return err.AsInteger();
-}
-
-ChipError::StorageType pychip_Stack_Shutdown()
-{
-    //
-    // There is the symmetric call to match the Retain called at stack initialization
-    // time.
-    //
-    DeviceControllerFactory::GetInstance().ReleaseSystemState();
-    return CHIP_NO_ERROR.AsInteger();
-}
-
 const char * pychip_Stack_ErrorToString(ChipError::StorageType err)
 {
     return chip::ErrorStr(CHIP_ERROR(err));

src/controller/python/CommonStackInit.cpp

@@ -0,0 +1,67 @@
+/*
+ *
+ *    Copyright (c) 2020-2022 Project CHIP Authors
+ *    Copyright (c) 2019-2020 Google LLC.
+ *    Copyright (c) 2013-2018 Nest Labs, Inc.
+ *    All rights reserved.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+/**
+ *    @file
+ *      Implementation of the native methods expected by the Python
+ *      version of Chip Device Manager.
+ *
+ */
+
+#include <controller/python/CommonStackInit.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <system/SystemError.h>
+#include <system/SystemLayer.h>
+
+#include <lib/support/CHIPMem.h>
+#include <lib/support/CodeUtils.h>
+#include <lib/support/ScopedBuffer.h>
+#include <lib/support/logging/CHIPLogging.h>
+#include <platform/CHIPDeviceLayer.h>
+
+namespace chip {
+namespace Controller {
+namespace Python {
+
+CHIP_ERROR CommonStackInit()
+{
+    ReturnErrorOnFailure(chip::Platform::MemoryInit());
+    ReturnErrorOnFailure(chip::DeviceLayer::PlatformMgr().InitChipStack());
+    return CHIP_NO_ERROR;
+}
+
+void CommonStackShutdown()
+{
+    chip::DeviceLayer::PlatformMgr().Shutdown();
+
+#if 0 //
+      // We cannot actually call this because the destructor for the MdnsContexts singleton on Darwin only gets called
+      // on termination of the program, and that unfortunately makes a bunch of Platform::MemoryFree calls.
+      //
+    chip::Platform::MemoryShutdown();
+#endif
+}
+
+} // namespace Python
+} // namespace Controller
+} // namespace chip

src/controller/python/CommonStackInit.h

@@ -0,0 +1,47 @@
+/*
+ *
+ *    Copyright (c) 2020-2022 Project CHIP Authors
+ *    Copyright (c) 2019-2020 Google LLC.
+ *    Copyright (c) 2013-2018 Nest Labs, Inc.
+ *    All rights reserved.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+/**
+ *    @file
+ *      Implementation of the native methods expected by the Python
+ *      version of Chip Device Manager.
+ *
+ */
+#include <lib/core/CHIPError.h>
+
+namespace chip {
+namespace Controller {
+namespace Python {
+
+//
+// This encapsulates stack initialization logic that is common to both
+// controller and server initialization sequences.
+//
+CHIP_ERROR CommonStackInit();
+
+//
+// This encapsulates stack shutdown logic that is common to both
+// controller and server shutdown sequences.
+//
+void CommonStackShutdown();
+
+} // namespace Python
+} // namespace Controller
+} // namespace chip

src/controller/python/chip/ChipStack.py

@@ -247,22 +247,17 @@ def HandleChipThreadRun(callback):
         # set by other modules(BLE) that require service by thread while thread blocks.
         self.blockingCB = None
 
-        # Initialize the chip library
-        res = self._ChipStackLib.pychip_Stack_Init()
-        if res != 0:
-            raise self.ErrorToException(res)
+        #
+        # Storage has to be initialized BEFORE initializing the stack, since the latter
+        # requires a PersistentStorageDelegate to be provided to DeviceControllerFactory.
+        #
+        self._persistentStorage = PersistentStorage(persistentStoragePath)
 
         if (bluetoothAdapter is None):
             bluetoothAdapter = 0
 
-        res = self._ChipStackLib.pychip_BLEMgrImpl_ConfigureBle(
-            bluetoothAdapter)
-        if res != 0:
-            raise self.ErrorToException(res)
-
-        self._persistentStorage = PersistentStorage(persistentStoragePath)
-
-        res = self._ChipStackLib.pychip_DeviceController_StackInit()
+        # Initialize the chip stack.
+        res = self._ChipStackLib.pychip_DeviceController_StackInit(bluetoothAdapter)
         if res != 0:
             raise self.ErrorToException(res)
 
@@ -321,7 +316,7 @@ def Shutdown(self):
         # Make sure PersistentStorage is destructed before chipStack
         # to avoid accessing builtins.chipStack after destruction.
         self._persistentStorage = None
-        self.Call(lambda: self._ChipStackLib.pychip_Stack_Shutdown())
+        self.Call(lambda: self._ChipStackLib.pychip_DeviceController_StackShutdown())
         self.networkLock = None
         self.completeEvent = None
         self._ChipStackLib = None
@@ -459,10 +454,10 @@ def _AllDirsToRoot(self, dir):
     def _loadLib(self):
         if self._ChipStackLib is None:
             self._ChipStackLib = CDLL(self.LocateChipDLL())
-            self._ChipStackLib.pychip_Stack_Init.argtypes = []
-            self._ChipStackLib.pychip_Stack_Init.restype = c_uint32
-            self._ChipStackLib.pychip_Stack_Shutdown.argtypes = []
-            self._ChipStackLib.pychip_Stack_Shutdown.restype = c_uint32
+            self._ChipStackLib.pychip_DeviceController_StackInit.argtypes = [c_uint32]
+            self._ChipStackLib.pychip_DeviceController_StackInit.restype = c_uint32
+            self._ChipStackLib.pychip_DeviceController_StackShutdown.argtypes = []
+            self._ChipStackLib.pychip_DeviceController_StackShutdown.restype = c_uint32
             self._ChipStackLib.pychip_Stack_StatusReportToString.argtypes = [
                 c_uint32,
                 c_uint16,
@@ -474,10 +469,6 @@ def _loadLib(self):
                 _LogMessageFunct]
             self._ChipStackLib.pychip_Stack_SetLogFunct.restype = c_uint32
 
-            self._ChipStackLib.pychip_BLEMgrImpl_ConfigureBle.argtypes = [
-                c_uint32]
-            self._ChipStackLib.pychip_BLEMgrImpl_ConfigureBle.restype = c_uint32
-
             self._ChipStackLib.pychip_DeviceController_PostTaskOnChipThread.argtypes = [
                 _ChipThreadTaskRunnerFunct, py_object]
             self._ChipStackLib.pychip_DeviceController_PostTaskOnChipThread.restype = c_uint32