Fix Python Threading Model

This fixes a number of issues with the Python REPL's threading model,
namely:

- Multiple, duplicated methods for stack initialization.
- Stack initialization split across really odd places
- Incorrect thread initialization
- Incorrect order of initialization

src/controller/python/BUILD.gn

@@ -63,7 +63,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

@@ -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;
@@ -107,6 +111,7 @@ chip::NodeId kRemoteDeviceId       = chip::kTestDeviceNodeId;
 
 extern "C" {
 ChipError::StorageType pychip_DeviceController_StackInit();
+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);
@@ -222,6 +225,15 @@ chip::Controller::Python::StorageAdapter * pychip_Storage_GetStorageAdapter()
 
 ChipError::StorageType pychip_DeviceController_StackInit()
 {
+    ReturnErrorOnFailure(chip::Platform::MemoryInit().AsInteger());
+
+    auto err = chip::DeviceLayer::PlatformMgr().InitChipStack();
+    if (err != CHIP_NO_ERROR)
+    {
+        ChipLogError(DeviceLayer, "Failed to initialize CHIP stack: platform init failed: %s", chip::ErrorStr(err));
+        return err.AsInteger();
+    }
+
     VerifyOrDie(sStorageAdapter != nullptr);
 
     FactoryInitParams factoryParams;
@@ -236,6 +248,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 +267,30 @@ 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()
+{
+    //
+    // 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();
+
+    chip::DeviceLayer::PlatformMgr().Shutdown();
+
     return CHIP_NO_ERROR.AsInteger();
 }
 
@@ -313,6 +355,18 @@ void pychip_DeviceController_SetLogFilter(uint8_t category)
 #endif
 }
 
+ChipError::StorageType pychip_BLEMgrImpl_ConfigureBle(uint32_t bluetoothAdapterId)
+{
+#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
+
+    return CHIP_NO_ERROR.AsInteger();
+}
+
 ChipError::StorageType pychip_DeviceController_ConnectBLE(chip::Controller::DeviceCommissioner * devCtrl, uint16_t discriminator,
                                                           uint32_t setupPINCode, chip::NodeId nodeid)
 {
@@ -557,38 +611,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/chip/ChipStack.py

@@ -247,8 +247,14 @@ 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()
+        #
+        # Storage has to be initialized BEFORE initializing the stack, since the latter
+        # requires a PersistentStorageDelegate to be provided to DeviceControllerFactory.
+        #
+        self._persistentStorage = PersistentStorage(persistentStoragePath)
+
+        # Initialize the chip stack.
+        res = self._ChipStackLib.pychip_DeviceController_StackInit()
         if res != 0:
             raise self.ErrorToException(res)
 
@@ -260,12 +266,6 @@ def HandleChipThreadRun(callback):
         if res != 0:
             raise self.ErrorToException(res)
 
-        self._persistentStorage = PersistentStorage(persistentStoragePath)
-
-        res = self._ChipStackLib.pychip_DeviceController_StackInit()
-        if res != 0:
-            raise self.ErrorToException(res)
-
         im.InitIMDelegate()
         ClusterAttribute.Init()
         ClusterCommand.Init()
@@ -321,7 +321,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 +459,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 = []
+            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,

src/controller/python/chip/native/__init__.py

@@ -75,11 +75,6 @@ def GetLibraryHandle() -> ctypes.CDLL:
     global _nativeLibraryHandle
     if _nativeLibraryHandle is None:
         _nativeLibraryHandle = ctypes.CDLL(FindNativeLibraryPath())
-
         setter = NativeLibraryHandleMethodArguments(_nativeLibraryHandle)
 
-        setter.Set("pychip_native_init", None, [])
-
-        _nativeLibraryHandle.pychip_native_init()
-
     return _nativeLibraryHandle