From de3bafe72fe8d60f521da5d3ef94139f50eb6de2 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Mon, 27 Mar 2023 09:26:29 +0200 Subject: [PATCH 01/12] Run Glib thread on a dedicated context --- src/platform/Linux/PlatformManagerImpl.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/platform/Linux/PlatformManagerImpl.cpp b/src/platform/Linux/PlatformManagerImpl.cpp index 8001ae6677d7de..16640a4aef8708 100644 --- a/src/platform/Linux/PlatformManagerImpl.cpp +++ b/src/platform/Linux/PlatformManagerImpl.cpp @@ -56,9 +56,14 @@ PlatformManagerImpl PlatformManagerImpl::sInstance; namespace { #if CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP -void * GLibMainLoopThread(void * loop) +void * GLibMainLoopThread(void * userData) { - g_main_loop_run(static_cast(loop)); + GMainLoop * loop = static_cast(userData); + GMainContext * context = g_main_loop_get_context(loop); + + g_main_context_push_thread_default(context); + g_main_loop_run(loop); + return nullptr; } #endif @@ -190,8 +195,10 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack() { #if CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP - mGLibMainLoop = g_main_loop_new(nullptr, FALSE); + auto * context = g_main_context_new(); + mGLibMainLoop = g_main_loop_new(context, FALSE); mGLibMainLoopThread = g_thread_new("gmain-matter", GLibMainLoopThread, mGLibMainLoop); + g_main_context_unref(context); { // Wait for the GLib main loop to start. It is required that the context used From 77e5733e573d81dd8b0f0ab7069f330e4863fdf9 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Mon, 27 Mar 2023 11:22:49 +0200 Subject: [PATCH 02/12] Run WiFiIPChangeListener on dedicated context --- src/platform/Linux/PlatformManagerImpl.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/platform/Linux/PlatformManagerImpl.cpp b/src/platform/Linux/PlatformManagerImpl.cpp index 16640a4aef8708..88f067fbde754e 100644 --- a/src/platform/Linux/PlatformManagerImpl.cpp +++ b/src/platform/Linux/PlatformManagerImpl.cpp @@ -156,7 +156,7 @@ gboolean WiFiIPChangeListener(GIOChannel * ch, GIOCondition /* condition */, voi // The temporary hack for getting IP address change on linux for network provisioning in the rendezvous session. // This should be removed or find a better place once we deprecate the rendezvous session. -CHIP_ERROR RunWiFiIPChangeListener() +CHIP_ERROR RunWiFiIPChangeListener(GMainContext * context) { int sock; if ((sock = socket(PF_NETLINK, SOCK_RAW, NETLINK_ROUTE)) == -1) @@ -177,11 +177,15 @@ CHIP_ERROR RunWiFiIPChangeListener() return CHIP_ERROR_INTERNAL; } - GIOChannel * ch = g_io_channel_unix_new(sock); - g_io_add_watch_full(ch, G_PRIORITY_DEFAULT, G_IO_IN, WiFiIPChangeListener, nullptr, nullptr); - + GIOChannel * ch = g_io_channel_unix_new(sock); + GSource * watchSource = g_io_create_watch(ch, G_IO_IN); + g_source_set_callback(watchSource, reinterpret_cast(WiFiIPChangeListener), nullptr, nullptr); g_io_channel_set_close_on_unref(ch, TRUE); g_io_channel_set_encoding(ch, nullptr, nullptr); + + g_source_attach(watchSource, context); + + g_source_unref(watchSource); g_io_channel_unref(ch); return CHIP_NO_ERROR; @@ -228,7 +232,7 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack() #endif #if CHIP_DEVICE_CONFIG_ENABLE_WIFI - ReturnErrorOnFailure(RunWiFiIPChangeListener()); + ReturnErrorOnFailure(RunWiFiIPChangeListener(g_main_loop_get_context(mGLibMainLoop))); #endif // Initialize the configuration system. From e78dd2f548bc359e4296a53cabb8219ab3078504 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Mon, 27 Mar 2023 12:26:27 +0200 Subject: [PATCH 03/12] Do not use g_io_add_watch when adding IO watch --- src/platform/Linux/PlatformManagerImpl.cpp | 10 +++++----- src/platform/Linux/PlatformManagerImpl.h | 15 ++++++++++++--- src/platform/Linux/bluez/Helper.cpp | 19 +++++++++++++------ 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/platform/Linux/PlatformManagerImpl.cpp b/src/platform/Linux/PlatformManagerImpl.cpp index 88f067fbde754e..a1c83faf299389 100644 --- a/src/platform/Linux/PlatformManagerImpl.cpp +++ b/src/platform/Linux/PlatformManagerImpl.cpp @@ -156,7 +156,7 @@ gboolean WiFiIPChangeListener(GIOChannel * ch, GIOCondition /* condition */, voi // The temporary hack for getting IP address change on linux for network provisioning in the rendezvous session. // This should be removed or find a better place once we deprecate the rendezvous session. -CHIP_ERROR RunWiFiIPChangeListener(GMainContext * context) +CHIP_ERROR RunWiFiIPChangeListener() { int sock; if ((sock = socket(PF_NETLINK, SOCK_RAW, NETLINK_ROUTE)) == -1) @@ -179,11 +179,11 @@ CHIP_ERROR RunWiFiIPChangeListener(GMainContext * context) GIOChannel * ch = g_io_channel_unix_new(sock); GSource * watchSource = g_io_create_watch(ch, G_IO_IN); - g_source_set_callback(watchSource, reinterpret_cast(WiFiIPChangeListener), nullptr, nullptr); + g_source_set_callback(watchSource, G_SOURCE_FUNC(WiFiIPChangeListener), nullptr, nullptr); g_io_channel_set_close_on_unref(ch, TRUE); g_io_channel_set_encoding(ch, nullptr, nullptr); - g_source_attach(watchSource, context); + PlatformMgrImpl().GLibMatterContextAttachSource(watchSource); g_source_unref(watchSource); g_io_channel_unref(ch); @@ -223,7 +223,7 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack() return G_SOURCE_REMOVE; }, &invokeData, nullptr); - g_source_attach(idleSource, g_main_loop_get_context(mGLibMainLoop)); + GLibMatterContextAttachSource(idleSource); g_source_unref(idleSource); invokeData.mDoneCond.wait(lock, [&invokeData]() { return invokeData.mDone; }); @@ -232,7 +232,7 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack() #endif #if CHIP_DEVICE_CONFIG_ENABLE_WIFI - ReturnErrorOnFailure(RunWiFiIPChangeListener(g_main_loop_get_context(mGLibMainLoop))); + ReturnErrorOnFailure(RunWiFiIPChangeListener()); #endif // Initialize the configuration system. diff --git a/src/platform/Linux/PlatformManagerImpl.h b/src/platform/Linux/PlatformManagerImpl.h index aa50cce59e2998..c67b106114683f 100644 --- a/src/platform/Linux/PlatformManagerImpl.h +++ b/src/platform/Linux/PlatformManagerImpl.h @@ -72,9 +72,18 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener return _GLibMatterContextInvokeSync((CHIP_ERROR(*)(void *)) func, (void *) userData); } + unsigned int GLibMatterContextAttachSource(GSource * source) + { + VerifyOrDie(mGLibMainLoop != nullptr); + return g_source_attach(source, g_main_loop_get_context(mGLibMainLoop)); + } + #endif - System::Clock::Timestamp GetStartTime() { return mStartTime; } + System::Clock::Timestamp GetStartTime() + { + return mStartTime; + } private: // ===== Methods that implement the PlatformManager abstract interface. @@ -121,8 +130,8 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener // event loop thread before the call to g_source_attach(). std::mutex mGLibMainLoopCallbackIndirectionMutex; - GMainLoop * mGLibMainLoop; - GThread * mGLibMainLoopThread; + GMainLoop * mGLibMainLoop = nullptr; + GThread * mGLibMainLoopThread = nullptr; #endif // CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP }; diff --git a/src/platform/Linux/bluez/Helper.cpp b/src/platform/Linux/bluez/Helper.cpp index f73d4ccb392ba9..6a0b658c018e0b 100644 --- a/src/platform/Linux/bluez/Helper.cpp +++ b/src/platform/Linux/bluez/Helper.cpp @@ -442,6 +442,7 @@ static gboolean BluezCharacteristicAcquireWrite(BluezGattCharacteristic1 * aChar { int fds[2] = { -1, -1 }; GIOChannel * channel; + GSource * watchSource; #if CHIP_ERROR_LOGGING char * errStr; #endif // CHIP_ERROR_LOGGING @@ -485,10 +486,12 @@ static gboolean BluezCharacteristicAcquireWrite(BluezGattCharacteristic1 * aChar g_io_channel_set_encoding(channel, nullptr, nullptr); g_io_channel_set_close_on_unref(channel, TRUE); g_io_channel_set_buffered(channel, FALSE); - conn->mC1Channel.mpChannel = channel; - conn->mC1Channel.mWatch = g_io_add_watch(channel, static_cast(G_IO_HUP | G_IO_IN | G_IO_ERR | G_IO_NVAL), - BluezCharacteristicWriteFD, conn); + + watchSource = g_io_create_watch(channel, static_cast(G_IO_HUP | G_IO_IN | G_IO_ERR | G_IO_NVAL)); + g_source_set_callback(watchSource, G_SOURCE_FUNC(BluezCharacteristicWriteFD), conn, nullptr); + conn->mC1Channel.mWatch = PlatformMgrImpl().GLibMatterContextAttachSource(watchSource); + g_source_unref(watchSource); bluez_gatt_characteristic1_set_write_acquired(aChar, TRUE); @@ -514,6 +517,7 @@ static gboolean BluezCharacteristicAcquireNotify(BluezGattCharacteristic1 * aCha { int fds[2] = { -1, -1 }; GIOChannel * channel; + GSource * watchSource; #if CHIP_ERROR_LOGGING char * errStr; #endif // CHIP_ERROR_LOGGING @@ -548,14 +552,17 @@ static gboolean BluezCharacteristicAcquireNotify(BluezGattCharacteristic1 * aCha g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.Failed", "FD creation failed"); goto exit; } + channel = g_io_channel_unix_new(fds[0]); g_io_channel_set_encoding(channel, nullptr, nullptr); g_io_channel_set_close_on_unref(channel, TRUE); g_io_channel_set_buffered(channel, FALSE); conn->mC2Channel.mpChannel = channel; - conn->mC2Channel.mWatch = - g_io_add_watch_full(channel, G_PRIORITY_DEFAULT_IDLE, static_cast(G_IO_HUP | G_IO_ERR | G_IO_NVAL), - bluezCharacteristicDestroyFD, conn, nullptr); + + watchSource = g_io_create_watch(channel, static_cast(G_IO_HUP | G_IO_ERR | G_IO_NVAL)); + g_source_set_callback(watchSource, G_SOURCE_FUNC(bluezCharacteristicDestroyFD), conn, nullptr); + conn->mC2Channel.mWatch = PlatformMgrImpl().GLibMatterContextAttachSource(watchSource); + g_source_unref(watchSource); bluez_gatt_characteristic1_set_notify_acquired(aChar, TRUE); From 97bbf414aa7c2718c6c57090cfec4b3271512871 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Thu, 30 Mar 2023 10:25:43 +0200 Subject: [PATCH 04/12] Check if D-Bus object proxies are created correctly All D-Bus proxy objects created with GLib *_new_for_bus and *_new_for_bus_sync functions have to be created with thread default main context set. Otherwise, proxy object internal signals will be scheduled on GLib global main context. We do not want that, because we do not know whether Matter SDK will be used in GLib-based application or not, so we can not run global main even loop on our own - all Matter-related GLib signals shell be run on a dedicated Matter GLib context. --- .../Linux/ConnectivityManagerImpl.cpp | 32 +++++++++++++++++++ src/platform/Linux/ThreadStackManagerImpl.cpp | 4 +++ src/platform/Linux/bluez/AdapterIterator.cpp | 4 +++ .../Linux/bluez/ChipDeviceScanner.cpp | 4 +++ 4 files changed, 44 insertions(+) diff --git a/src/platform/Linux/ConnectivityManagerImpl.cpp b/src/platform/Linux/ConnectivityManagerImpl.cpp index 57bba172fe3590..e922758822681a 100644 --- a/src/platform/Linux/ConnectivityManagerImpl.cpp +++ b/src/platform/Linux/ConnectivityManagerImpl.cpp @@ -486,6 +486,10 @@ void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaFiW1Wpa_supplicant1Inte void ConnectivityManagerImpl::_OnWpaInterfaceProxyReady(GObject * source_object, GAsyncResult * res, gpointer user_data) { + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + GError * err = nullptr; std::lock_guard lock(mWpaSupplicantMutex); @@ -530,6 +534,10 @@ void ConnectivityManagerImpl::_OnWpaInterfaceProxyReady(GObject * source_object, void ConnectivityManagerImpl::_OnWpaBssProxyReady(GObject * source_object, GAsyncResult * res, gpointer user_data) { + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + GError * err = nullptr; std::lock_guard lock(mWpaSupplicantMutex); @@ -559,6 +567,10 @@ void ConnectivityManagerImpl::_OnWpaBssProxyReady(GObject * source_object, GAsyn void ConnectivityManagerImpl::_OnWpaInterfaceReady(GObject * source_object, GAsyncResult * res, gpointer user_data) { + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + GError * err = nullptr; std::lock_guard lock(mWpaSupplicantMutex); @@ -634,6 +646,10 @@ void ConnectivityManagerImpl::_OnWpaInterfaceReady(GObject * source_object, GAsy void ConnectivityManagerImpl::_OnWpaInterfaceAdded(WpaFiW1Wpa_supplicant1 * proxy, const gchar * path, GVariant * properties, gpointer user_data) { + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + std::lock_guard lock(mWpaSupplicantMutex); if (mWpaSupplicant.interfacePath) @@ -696,6 +712,10 @@ void ConnectivityManagerImpl::_OnWpaInterfaceRemoved(WpaFiW1Wpa_supplicant1 * pr void ConnectivityManagerImpl::_OnWpaProxyReady(GObject * source_object, GAsyncResult * res, gpointer user_data) { + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + GError * err = nullptr; std::lock_guard lock(mWpaSupplicantMutex); @@ -725,6 +745,10 @@ void ConnectivityManagerImpl::_OnWpaProxyReady(GObject * source_object, GAsyncRe void ConnectivityManagerImpl::StartWiFiManagement() { + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + std::lock_guard lock(mWpaSupplicantMutex); mConnectivityFlag.ClearAll(); @@ -1276,6 +1300,10 @@ int32_t ConnectivityManagerImpl::GetDisconnectReason() CHIP_ERROR ConnectivityManagerImpl::GetConfiguredNetwork(NetworkCommissioning::Network & network) { + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + std::lock_guard lock(mWpaSupplicantMutex); std::unique_ptr err; @@ -1460,6 +1488,10 @@ std::pair GetBandAndChannelFromFrequency(uint32_t freq) bool ConnectivityManagerImpl::_GetBssInfo(const gchar * bssPath, NetworkCommissioning::WiFiScanResponse & result) { + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + std::unique_ptr err; std::unique_ptr bss( wpa_fi_w1_wpa_supplicant1_bss_proxy_new_for_bus_sync(G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE, kWpaSupplicantServiceName, diff --git a/src/platform/Linux/ThreadStackManagerImpl.cpp b/src/platform/Linux/ThreadStackManagerImpl.cpp index 9b27f7d6c68e0f..a8d52e3b355ae2 100755 --- a/src/platform/Linux/ThreadStackManagerImpl.cpp +++ b/src/platform/Linux/ThreadStackManagerImpl.cpp @@ -53,6 +53,10 @@ ThreadStackManagerImpl::ThreadStackManagerImpl() : mAttached(false) {} CHIP_ERROR ThreadStackManagerImpl::_InitThreadStack() { + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + std::unique_ptr err; mProxy.reset(openthread_io_openthread_border_router_proxy_new_for_bus_sync(G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE, kDBusOpenThreadService, kDBusOpenThreadObjectPath, diff --git a/src/platform/Linux/bluez/AdapterIterator.cpp b/src/platform/Linux/bluez/AdapterIterator.cpp index ec2b58b6bfe6de..30825b6c372040 100644 --- a/src/platform/Linux/bluez/AdapterIterator.cpp +++ b/src/platform/Linux/bluez/AdapterIterator.cpp @@ -47,6 +47,10 @@ AdapterIterator::~AdapterIterator() void AdapterIterator::Initialize() { + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + GError * error = nullptr; mManager = g_dbus_object_manager_client_new_for_bus_sync(G_BUS_TYPE_SYSTEM, G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_NONE, diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp index 0172c820c5a0cc..7698e8e313ce53 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp +++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp @@ -93,6 +93,10 @@ ChipDeviceScanner::~ChipDeviceScanner() std::unique_ptr ChipDeviceScanner::Create(BluezAdapter1 * adapter, ChipDeviceScannerDelegate * delegate) { + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + GError * error = nullptr; GCancellableUniquePtr cancellable(g_cancellable_new(), GObjectUnref()); From a661fdebe53a78720b1eb30cadc0af5f6ab23df8 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Tue, 28 Mar 2023 15:22:37 +0200 Subject: [PATCH 05/12] Initialize BlueZ D-Bus object proxy on Matter GLib event loop --- .../Linux/bluez/ChipDeviceScanner.cpp | 69 +++++++++++-------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp index 7698e8e313ce53..62e4c1d03e093b 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp +++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp @@ -31,19 +31,44 @@ namespace chip { namespace DeviceLayer { namespace Internal { + namespace { -struct GObjectUnref +struct GDBusCancellableObjectManager { - template - void operator()(T * value) + GCancellable * cancellable = nullptr; + GDBusObjectManager * object = nullptr; + + GDBusCancellableObjectManager() : cancellable(g_cancellable_new()) {} + ~GDBusCancellableObjectManager() { - g_object_unref(value); + if (cancellable != nullptr) + { + g_object_unref(cancellable); + } + if (object != nullptr) + { + g_object_unref(object); + } } }; -using GCancellableUniquePtr = std::unique_ptr; -using GDBusObjectManagerUniquePtr = std::unique_ptr; +CHIP_ERROR MainLoopCreateObjectManager(GDBusCancellableObjectManager * cancellableObjectManager) +{ + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + + std::unique_ptr err; + cancellableObjectManager->object = g_dbus_object_manager_client_new_for_bus_sync( + G_BUS_TYPE_SYSTEM, G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_NONE, BLUEZ_INTERFACE, "/", + bluez_object_manager_client_get_proxy_type, nullptr /* unused user data in the Proxy Type Func */, + nullptr /* destroy notify */, cancellableObjectManager->cancellable, &MakeUniquePointerReceiver(err).Get()); + VerifyOrReturnError(cancellableObjectManager->object != nullptr, CHIP_ERROR_INTERNAL, + ChipLogError(Ble, "Failed to get DBUS object manager for device scanning: %s", err->message)); + + return CHIP_NO_ERROR; +} /// Retrieve CHIP device identification info from the device advertising data bool BluezGetChipDeviceInfo(BluezDevice1 & aDevice, chip::Ble::ChipBLEDeviceIdentificationInfo & aDeviceInfo) @@ -93,33 +118,17 @@ ChipDeviceScanner::~ChipDeviceScanner() std::unique_ptr ChipDeviceScanner::Create(BluezAdapter1 * adapter, ChipDeviceScannerDelegate * delegate) { - // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, - // all D-Bus signals will be delivered to the GLib global default main context. - VerifyOrDie(g_main_context_get_thread_default() != nullptr); - - GError * error = nullptr; - - GCancellableUniquePtr cancellable(g_cancellable_new(), GObjectUnref()); + GDBusCancellableObjectManager manager; + CHIP_ERROR err; - if (!cancellable) - { - return std::unique_ptr(); - } + VerifyOrExit(manager.cancellable != nullptr, ); + err = PlatformMgrImpl().GLibMatterContextInvokeSync(MainLoopCreateObjectManager, &manager); + VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(Ble, "Failed to create BLE object manager")); - GDBusObjectManagerUniquePtr manager( - g_dbus_object_manager_client_new_for_bus_sync(G_BUS_TYPE_SYSTEM, G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_NONE, BLUEZ_INTERFACE, - "/", bluez_object_manager_client_get_proxy_type, - nullptr /* unused user data in the Proxy Type Func */, - nullptr /*destroy notify */, cancellable.get(), &error), - GObjectUnref()); - if (!manager) - { - ChipLogError(Ble, "Failed to get DBUS object manager for device scanning: %s", error->message); - g_error_free(error); - return std::unique_ptr(); - } + return std::make_unique(manager.object, adapter, manager.cancellable, delegate); - return std::make_unique(manager.get(), adapter, cancellable.get(), delegate); +exit: + return std::unique_ptr(); } CHIP_ERROR ChipDeviceScanner::StartScan(System::Clock::Timeout timeout) From 2ab13cb69935666fd836a12baa4787abcd2b83c2 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Thu, 30 Mar 2023 13:03:51 +0200 Subject: [PATCH 06/12] Initialize WPA D-Bus object proxy on Matter GLib event loop --- .../Linux/ConnectivityManagerImpl.cpp | 48 +++++++++++-------- src/platform/Linux/ConnectivityManagerImpl.h | 2 + 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/platform/Linux/ConnectivityManagerImpl.cpp b/src/platform/Linux/ConnectivityManagerImpl.cpp index e922758822681a..69f99697fb281a 100644 --- a/src/platform/Linux/ConnectivityManagerImpl.cpp +++ b/src/platform/Linux/ConnectivityManagerImpl.cpp @@ -745,19 +745,8 @@ void ConnectivityManagerImpl::_OnWpaProxyReady(GObject * source_object, GAsyncRe void ConnectivityManagerImpl::StartWiFiManagement() { - // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, - // all D-Bus signals will be delivered to the GLib global default main context. - VerifyOrDie(g_main_context_get_thread_default() != nullptr); - - std::lock_guard lock(mWpaSupplicantMutex); - - mConnectivityFlag.ClearAll(); - mWpaSupplicant = GDBusWpaSupplicant{}; - - ChipLogProgress(DeviceLayer, "wpa_supplicant: Start WiFi management"); - - wpa_fi_w1_wpa_supplicant1_proxy_new_for_bus(G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE, kWpaSupplicantServiceName, - kWpaSupplicantObjectPath, nullptr, _OnWpaProxyReady, nullptr); + CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(_StartWiFiManagement, this); + VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(DeviceLayer, "Failed to start WiFi management")); } bool ConnectivityManagerImpl::IsWiFiManagementStarted() @@ -1300,9 +1289,10 @@ int32_t ConnectivityManagerImpl::GetDisconnectReason() CHIP_ERROR ConnectivityManagerImpl::GetConfiguredNetwork(NetworkCommissioning::Network & network) { - // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, - // all D-Bus signals will be delivered to the GLib global default main context. - VerifyOrDie(g_main_context_get_thread_default() != nullptr); + // This function can be called without g_main_context_get_thread_default() being set. + // The network proxy object is created in a synchronous manner, so the D-Bus call will + // be completed before this function returns. Also, no external callbacks are registered + // with the proxy object. std::lock_guard lock(mWpaSupplicantMutex); std::unique_ptr err; @@ -1488,9 +1478,10 @@ std::pair GetBandAndChannelFromFrequency(uint32_t freq) bool ConnectivityManagerImpl::_GetBssInfo(const gchar * bssPath, NetworkCommissioning::WiFiScanResponse & result) { - // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, - // all D-Bus signals will be delivered to the GLib global default main context. - VerifyOrDie(g_main_context_get_thread_default() != nullptr); + // This function can be called without g_main_context_get_thread_default() being set. + // The BSS proxy object is created in a synchronous manner, so the D-Bus call will be + // completed before this function returns. Also, no external callbacks are registered + // with the proxy object. std::unique_ptr err; std::unique_ptr bss( @@ -1705,6 +1696,25 @@ void ConnectivityManagerImpl::_OnWpaInterfaceScanDone(GObject * source_object, G g_strfreev(oldBsss); } +CHIP_ERROR ConnectivityManagerImpl::_StartWiFiManagement(ConnectivityManagerImpl * self) +{ + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + + std::lock_guard lock(self->mWpaSupplicantMutex); + + self->mConnectivityFlag.ClearAll(); + self->mWpaSupplicant = GDBusWpaSupplicant{}; + + ChipLogProgress(DeviceLayer, "wpa_supplicant: Start WiFi management"); + + wpa_fi_w1_wpa_supplicant1_proxy_new_for_bus(G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE, kWpaSupplicantServiceName, + kWpaSupplicantObjectPath, nullptr, self->_OnWpaProxyReady, nullptr); + + return CHIP_NO_ERROR; +} + #endif // CHIP_DEVICE_CONFIG_ENABLE_WPA } // namespace DeviceLayer diff --git a/src/platform/Linux/ConnectivityManagerImpl.h b/src/platform/Linux/ConnectivityManagerImpl.h index b3a87ea1f6735b..5a0c45c8d3f307 100644 --- a/src/platform/Linux/ConnectivityManagerImpl.h +++ b/src/platform/Linux/ConnectivityManagerImpl.h @@ -205,6 +205,8 @@ class ConnectivityManagerImpl final : public ConnectivityManager, static bool _GetBssInfo(const gchar * bssPath, NetworkCommissioning::WiFiScanResponse & result); + static CHIP_ERROR _StartWiFiManagement(ConnectivityManagerImpl * self); + static bool mAssociationStarted; static BitFlags mConnectivityFlag; static GDBusWpaSupplicant mWpaSupplicant; From 06cc96fbefe90993d7fc41837312d961dbe6417b Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Thu, 30 Mar 2023 13:09:15 +0200 Subject: [PATCH 07/12] Fix typo: devcie -> device --- src/platform/Linux/ThreadStackManagerImpl.cpp | 6 +++--- src/platform/Linux/ThreadStackManagerImpl.h | 2 +- src/platform/webos/ThreadStackManagerImpl.cpp | 6 +++--- src/platform/webos/ThreadStackManagerImpl.h | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/platform/Linux/ThreadStackManagerImpl.cpp b/src/platform/Linux/ThreadStackManagerImpl.cpp index a8d52e3b355ae2..7e7a3e2b4421a6 100755 --- a/src/platform/Linux/ThreadStackManagerImpl.cpp +++ b/src/platform/Linux/ThreadStackManagerImpl.cpp @@ -74,7 +74,7 @@ CHIP_ERROR ThreadStackManagerImpl::_InitThreadStack() std::unique_ptr role(openthread_io_openthread_border_router_dup_device_role(mProxy.get())); if (role) { - ThreadDevcieRoleChangedHandler(role.get()); + ThreadDeviceRoleChangedHandler(role.get()); } return CHIP_NO_ERROR; @@ -106,13 +106,13 @@ void ThreadStackManagerImpl::OnDbusPropertiesChanged(OpenthreadIoOpenthreadBorde if (value_str == nullptr) continue; ChipLogProgress(DeviceLayer, "Thread role changed to: %s", StringOrNullMarker(value_str)); - me->ThreadDevcieRoleChangedHandler(value_str); + me->ThreadDeviceRoleChangedHandler(value_str); } } } } -void ThreadStackManagerImpl::ThreadDevcieRoleChangedHandler(const gchar * role) +void ThreadStackManagerImpl::ThreadDeviceRoleChangedHandler(const gchar * role) { bool attached = strcmp(role, kOpenthreadDeviceRoleDetached) != 0 && strcmp(role, kOpenthreadDeviceRoleDisabled) != 0; diff --git a/src/platform/Linux/ThreadStackManagerImpl.h b/src/platform/Linux/ThreadStackManagerImpl.h index 37e743fbe2e64b..03398856e9ab9a 100755 --- a/src/platform/Linux/ThreadStackManagerImpl.h +++ b/src/platform/Linux/ThreadStackManagerImpl.h @@ -149,7 +149,7 @@ class ThreadStackManagerImpl : public ThreadStackManager static void OnDbusPropertiesChanged(OpenthreadIoOpenthreadBorderRouter * proxy, GVariant * changed_properties, const gchar * const * invalidated_properties, gpointer user_data); - void ThreadDevcieRoleChangedHandler(const gchar * role); + void ThreadDeviceRoleChangedHandler(const gchar * role); Thread::OperationalDataset mDataset = {}; diff --git a/src/platform/webos/ThreadStackManagerImpl.cpp b/src/platform/webos/ThreadStackManagerImpl.cpp index eba2d0d9033924..3b0bea0eeec628 100644 --- a/src/platform/webos/ThreadStackManagerImpl.cpp +++ b/src/platform/webos/ThreadStackManagerImpl.cpp @@ -70,7 +70,7 @@ CHIP_ERROR ThreadStackManagerImpl::_InitThreadStack() std::unique_ptr role(openthread_io_openthread_border_router_dup_device_role(mProxy.get())); if (role) { - ThreadDevcieRoleChangedHandler(role.get()); + ThreadDeviceRoleChangedHandler(role.get()); } return CHIP_NO_ERROR; @@ -102,13 +102,13 @@ void ThreadStackManagerImpl::OnDbusPropertiesChanged(OpenthreadIoOpenthreadBorde if (value_str == nullptr) continue; ChipLogProgress(DeviceLayer, "Thread role changed to: %s", StringOrNullMarker(value_str)); - me->ThreadDevcieRoleChangedHandler(value_str); + me->ThreadDeviceRoleChangedHandler(value_str); } } } } -void ThreadStackManagerImpl::ThreadDevcieRoleChangedHandler(const gchar * role) +void ThreadStackManagerImpl::ThreadDeviceRoleChangedHandler(const gchar * role) { bool attached = strcmp(role, kOpenthreadDeviceRoleDetached) != 0 && strcmp(role, kOpenthreadDeviceRoleDisabled) != 0; diff --git a/src/platform/webos/ThreadStackManagerImpl.h b/src/platform/webos/ThreadStackManagerImpl.h index ce7bf3b0e56b52..e2391dffd78aba 100644 --- a/src/platform/webos/ThreadStackManagerImpl.h +++ b/src/platform/webos/ThreadStackManagerImpl.h @@ -147,7 +147,7 @@ class ThreadStackManagerImpl : public ThreadStackManager static void OnDbusPropertiesChanged(OpenthreadIoOpenthreadBorderRouter * proxy, GVariant * changed_properties, const gchar * const * invalidated_properties, gpointer user_data); - void ThreadDevcieRoleChangedHandler(const gchar * role); + void ThreadDeviceRoleChangedHandler(const gchar * role); Thread::OperationalDataset mDataset = {}; From e19cad111f54c732746779d1a5c6710d682899bd Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Tue, 4 Apr 2023 09:29:29 +0200 Subject: [PATCH 08/12] Initialize OTBR D-Bus object proxy on Matter GLib event loop --- src/platform/Linux/ThreadStackManagerImpl.cpp | 28 ++++++++++++------- src/platform/Linux/ThreadStackManagerImpl.h | 1 + 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/platform/Linux/ThreadStackManagerImpl.cpp b/src/platform/Linux/ThreadStackManagerImpl.cpp index 7e7a3e2b4421a6..fd59b0bf743602 100755 --- a/src/platform/Linux/ThreadStackManagerImpl.cpp +++ b/src/platform/Linux/ThreadStackManagerImpl.cpp @@ -51,23 +51,31 @@ constexpr char ThreadStackManagerImpl::kPropertyDeviceRole[]; ThreadStackManagerImpl::ThreadStackManagerImpl() : mAttached(false) {} -CHIP_ERROR ThreadStackManagerImpl::_InitThreadStack() +CHIP_ERROR ThreadStackManagerImpl::GLibMatterContextInitThreadStack(ThreadStackManagerImpl * self) { // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, // all D-Bus signals will be delivered to the GLib global default main context. VerifyOrDie(g_main_context_get_thread_default() != nullptr); std::unique_ptr err; - mProxy.reset(openthread_io_openthread_border_router_proxy_new_for_bus_sync(G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE, - kDBusOpenThreadService, kDBusOpenThreadObjectPath, - nullptr, &MakeUniquePointerReceiver(err).Get())); - if (!mProxy) - { - ChipLogError(DeviceLayer, "openthread: failed to create openthread dbus proxy %s", err ? err->message : "unknown error"); - return CHIP_ERROR_INTERNAL; - } + self->mProxy.reset(openthread_io_openthread_border_router_proxy_new_for_bus_sync( + G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE, kDBusOpenThreadService, kDBusOpenThreadObjectPath, nullptr, + &MakeUniquePointerReceiver(err).Get())); + VerifyOrReturnError( + self->mProxy != nullptr, CHIP_ERROR_INTERNAL, + ChipLogError(DeviceLayer, "openthread: failed to create openthread dbus proxy %s", err ? err->message : "unknown error")); + + g_signal_connect(self->mProxy.get(), "g-properties-changed", G_CALLBACK(OnDbusPropertiesChanged), self); + + return CHIP_NO_ERROR; +} + +CHIP_ERROR ThreadStackManagerImpl::_InitThreadStack() +{ + CHIP_ERROR err; - g_signal_connect(mProxy.get(), "g-properties-changed", G_CALLBACK(OnDbusPropertiesChanged), this); + err = PlatformMgrImpl().GLibMatterContextInvokeSync(GLibMatterContextInitThreadStack, this); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "openthread: failed to init dbus proxy")); // If get property is called inside dbus thread (we are going to make it so), XXX_get_XXX can be used instead of XXX_dup_XXX // which is a little bit faster and the returned object doesn't need to be freed. Same for all following get properties. diff --git a/src/platform/Linux/ThreadStackManagerImpl.h b/src/platform/Linux/ThreadStackManagerImpl.h index 03398856e9ab9a..3f168666b3e5ee 100755 --- a/src/platform/Linux/ThreadStackManagerImpl.h +++ b/src/platform/Linux/ThreadStackManagerImpl.h @@ -147,6 +147,7 @@ class ThreadStackManagerImpl : public ThreadStackManager std::unique_ptr mProxy; + static CHIP_ERROR GLibMatterContextInitThreadStack(ThreadStackManagerImpl * self); static void OnDbusPropertiesChanged(OpenthreadIoOpenthreadBorderRouter * proxy, GVariant * changed_properties, const gchar * const * invalidated_properties, gpointer user_data); void ThreadDeviceRoleChangedHandler(const gchar * role); From 029c1244acf73d316786954ab0ea49bba1943296 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Tue, 4 Apr 2023 13:31:31 +0200 Subject: [PATCH 09/12] Fix glib source watch removal in BLE module --- src/platform/Linux/PlatformManagerImpl.h | 5 +-- src/platform/Linux/bluez/Helper.cpp | 41 +++++++++--------------- src/platform/Linux/bluez/Types.h | 2 +- 3 files changed, 17 insertions(+), 31 deletions(-) diff --git a/src/platform/Linux/PlatformManagerImpl.h b/src/platform/Linux/PlatformManagerImpl.h index c67b106114683f..5669ce5dd28d62 100644 --- a/src/platform/Linux/PlatformManagerImpl.h +++ b/src/platform/Linux/PlatformManagerImpl.h @@ -80,10 +80,7 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener #endif - System::Clock::Timestamp GetStartTime() - { - return mStartTime; - } + System::Clock::Timestamp GetStartTime() { return mStartTime; } private: // ===== Methods that implement the PlatformManager abstract interface. diff --git a/src/platform/Linux/bluez/Helper.cpp b/src/platform/Linux/bluez/Helper.cpp index 6a0b658c018e0b..56513651cbe8c3 100644 --- a/src/platform/Linux/bluez/Helper.cpp +++ b/src/platform/Linux/bluez/Helper.cpp @@ -401,16 +401,6 @@ static gboolean BluezCharacteristicWriteFD(GIOChannel * aChannel, GIOCondition a isSuccess = true; exit: - if (!isSuccess && (conn != nullptr)) - { - // Returning G_SOURCE_REMOVE from the source callback removes the source object - // from the context. Unset self source ID tag, so we will not call g_source_remove() - // in BluezOTConnectionDestroy() on already removed source. - // - // TODO: Investigate whether there is a batter way to handle this. - conn->mC1Channel.mWatch = 0; - } - return isSuccess ? G_SOURCE_CONTINUE : G_SOURCE_REMOVE; } @@ -425,15 +415,8 @@ static void Bluez_gatt_characteristic1_complete_acquire_write_with_fd(GDBusMetho fd_list); } -static gboolean bluezCharacteristicDestroyFD(GIOChannel * aChannel, GIOCondition aCond, gpointer apClosure) +static gboolean bluezCharacteristicDestroyFD(GIOChannel * aChannel, GIOCondition aCond, gpointer apEndpoint) { - BluezConnection * conn = static_cast(apClosure); - // Returning G_SOURCE_REMOVE from the source callback removes the source object - // from the context. Unset self source ID tag, so we will not call g_source_remove() - // in BluezOTConnectionDestroy() on already removed source. - // - // TODO: Investigate whether there is a batter way to handle this. - conn->mC2Channel.mWatch = 0; return G_SOURCE_REMOVE; } @@ -490,8 +473,8 @@ static gboolean BluezCharacteristicAcquireWrite(BluezGattCharacteristic1 * aChar watchSource = g_io_create_watch(channel, static_cast(G_IO_HUP | G_IO_IN | G_IO_ERR | G_IO_NVAL)); g_source_set_callback(watchSource, G_SOURCE_FUNC(BluezCharacteristicWriteFD), conn, nullptr); - conn->mC1Channel.mWatch = PlatformMgrImpl().GLibMatterContextAttachSource(watchSource); - g_source_unref(watchSource); + PlatformMgrImpl().GLibMatterContextAttachSource(watchSource); + conn->mC1Channel.mWatchSource = watchSource; bluez_gatt_characteristic1_set_write_acquired(aChar, TRUE); @@ -561,8 +544,8 @@ static gboolean BluezCharacteristicAcquireNotify(BluezGattCharacteristic1 * aCha watchSource = g_io_create_watch(channel, static_cast(G_IO_HUP | G_IO_ERR | G_IO_NVAL)); g_source_set_callback(watchSource, G_SOURCE_FUNC(bluezCharacteristicDestroyFD), conn, nullptr); - conn->mC2Channel.mWatch = PlatformMgrImpl().GLibMatterContextAttachSource(watchSource); - g_source_unref(watchSource); + PlatformMgrImpl().GLibMatterContextAttachSource(watchSource); + conn->mC1Channel.mWatchSource = watchSource; bluez_gatt_characteristic1_set_notify_acquired(aChar, TRUE); @@ -795,12 +778,18 @@ static void BluezOTConnectionDestroy(BluezConnection * aConn) g_object_unref(aConn->mpC2); if (aConn->mpPeerAddress) g_free(aConn->mpPeerAddress); - if (aConn->mC1Channel.mWatch > 0) - g_source_remove(aConn->mC1Channel.mWatch); + if (aConn->mC1Channel.mWatchSource) + { + g_source_destroy(aConn->mC1Channel.mWatchSource); + g_source_unref(aConn->mC1Channel.mWatchSource); + } if (aConn->mC1Channel.mpChannel) g_io_channel_unref(aConn->mC1Channel.mpChannel); - if (aConn->mC2Channel.mWatch > 0) - g_source_remove(aConn->mC2Channel.mWatch); + if (aConn->mC2Channel.mWatchSource) + { + g_source_destroy(aConn->mC2Channel.mWatchSource); + g_source_unref(aConn->mC2Channel.mWatchSource); + } if (aConn->mC2Channel.mpChannel) g_io_channel_unref(aConn->mC2Channel.mpChannel); diff --git a/src/platform/Linux/bluez/Types.h b/src/platform/Linux/bluez/Types.h index b96070e9e94430..3f61c430a5f7a2 100644 --- a/src/platform/Linux/bluez/Types.h +++ b/src/platform/Linux/bluez/Types.h @@ -125,7 +125,7 @@ struct BluezAddress struct IOChannel { GIOChannel * mpChannel; - guint mWatch; + GSource * mWatchSource; }; struct BluezEndpoint From cc44d011939f89bd9b6199bd4b223604e242b9d8 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Tue, 4 Apr 2023 15:51:53 +0200 Subject: [PATCH 10/12] Enable glib event loop in Thread-only configuration --- src/platform/Linux/CHIPDevicePlatformConfig.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/platform/Linux/CHIPDevicePlatformConfig.h b/src/platform/Linux/CHIPDevicePlatformConfig.h index 9a316d4832348b..530e2716308842 100644 --- a/src/platform/Linux/CHIPDevicePlatformConfig.h +++ b/src/platform/Linux/CHIPDevicePlatformConfig.h @@ -41,9 +41,9 @@ #define CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE 0 #endif -// Start GLib main event loop if BLE or WiFi is enabled. This is needed to handle -// D-Bus communication with BlueZ or wpa_supplicant. -#if CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE || CHIP_DEVICE_CONFIG_ENABLE_WIFI +// Start GLib main event loop if BLE, Thread or WiFi is enabled. This is needed +// to handle D-Bus communication with BlueZ or wpa_supplicant. +#if CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE || CHIP_DEVICE_CONFIG_ENABLE_THREAD || CHIP_DEVICE_CONFIG_ENABLE_WIFI #define CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP 1 #else #define CHIP_DEVICE_CONFIG_WITH_GLIB_MAIN_LOOP 0 From ec685cd7a6c2d7dfa14fe1909a6ccb359f305312 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Wed, 12 Apr 2023 09:12:09 +0200 Subject: [PATCH 11/12] Run OTBR async calls on Matter glib context --- src/platform/Linux/ThreadStackManagerImpl.cpp | 59 +++++++++++++++---- src/platform/Linux/ThreadStackManagerImpl.h | 2 + 2 files changed, 49 insertions(+), 12 deletions(-) diff --git a/src/platform/Linux/ThreadStackManagerImpl.cpp b/src/platform/Linux/ThreadStackManagerImpl.cpp index fd59b0bf743602..fd89be709e1a20 100755 --- a/src/platform/Linux/ThreadStackManagerImpl.cpp +++ b/src/platform/Linux/ThreadStackManagerImpl.cpp @@ -49,6 +49,32 @@ constexpr char ThreadStackManagerImpl::kOpenthreadDeviceRoleLeader[]; constexpr char ThreadStackManagerImpl::kPropertyDeviceRole[]; +namespace { + +struct SetActiveDatasetContext +{ + OpenthreadIoOpenthreadBorderRouter * proxy; + ByteSpan netInfo; +}; + +CHIP_ERROR GLibMatterContextSetActiveDataset(SetActiveDatasetContext * context) +{ + // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, + // all D-Bus signals will be delivered to the GLib global default main context. + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + + std::unique_ptr bytes(g_bytes_new(context->netInfo.data(), context->netInfo.size())); + if (!bytes) + return CHIP_ERROR_NO_MEMORY; + std::unique_ptr value(g_variant_new_from_bytes(G_VARIANT_TYPE_BYTESTRING, bytes.release(), true)); + if (!value) + return CHIP_ERROR_NO_MEMORY; + openthread_io_openthread_border_router_set_active_dataset_tlvs(context->proxy, value.release()); + return CHIP_NO_ERROR; +} + +} // namespace + ThreadStackManagerImpl::ThreadStackManagerImpl() : mAttached(false) {} CHIP_ERROR ThreadStackManagerImpl::GLibMatterContextInitThreadStack(ThreadStackManagerImpl * self) @@ -229,16 +255,9 @@ CHIP_ERROR ThreadStackManagerImpl::_SetThreadProvision(ByteSpan netInfo) VerifyOrReturnError(mProxy, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(Thread::OperationalDataset::IsValid(netInfo), CHIP_ERROR_INVALID_ARGUMENT); - { - std::unique_ptr bytes(g_bytes_new(netInfo.data(), netInfo.size())); - if (!bytes) - return CHIP_ERROR_NO_MEMORY; - std::unique_ptr value( - g_variant_new_from_bytes(G_VARIANT_TYPE_BYTESTRING, bytes.release(), true)); - if (!value) - return CHIP_ERROR_NO_MEMORY; - openthread_io_openthread_border_router_set_active_dataset_tlvs(mProxy.get(), value.release()); - } + SetActiveDatasetContext context = { mProxy.get(), netInfo }; + CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(GLibMatterContextSetActiveDataset, &context); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "openthread: failed to set active dataset")); // post an event alerting other subsystems about change in provisioning state ChipDeviceEvent event; @@ -357,12 +376,20 @@ bool ThreadStackManagerImpl::_IsThreadAttached() const return mAttached; } +CHIP_ERROR ThreadStackManagerImpl::GLibMatterContextCallAttach(ThreadStackManagerImpl * self) +{ + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + openthread_io_openthread_border_router_call_attach(self->mProxy.get(), nullptr, _OnThreadBrAttachFinished, self); + return CHIP_NO_ERROR; +} + CHIP_ERROR ThreadStackManagerImpl::_SetThreadEnabled(bool val) { VerifyOrReturnError(mProxy, CHIP_ERROR_INCORRECT_STATE); if (val) { - openthread_io_openthread_border_router_call_attach(mProxy.get(), nullptr, _OnThreadBrAttachFinished, this); + CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(GLibMatterContextCallAttach, this); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "openthread: failed to attach")); } else { @@ -584,12 +611,20 @@ void ThreadStackManagerImpl::_SetRouterPromotion(bool val) // Set Router Promotion is not supported on linux } +CHIP_ERROR ThreadStackManagerImpl::GLibMatterContextCallScan(ThreadStackManagerImpl * self) +{ + VerifyOrDie(g_main_context_get_thread_default() != nullptr); + openthread_io_openthread_border_router_call_scan(self->mProxy.get(), nullptr, _OnNetworkScanFinished, self); + return CHIP_NO_ERROR; +} + CHIP_ERROR ThreadStackManagerImpl::_StartThreadScan(ThreadDriver::ScanCallback * callback) { // There is another ongoing scan request, reject the new one. VerifyOrReturnError(mpScanCallback == nullptr, CHIP_ERROR_INCORRECT_STATE); mpScanCallback = callback; - openthread_io_openthread_border_router_call_scan(mProxy.get(), nullptr, _OnNetworkScanFinished, this); + CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(GLibMatterContextCallScan, this); + VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "openthread: failed to start scan")); return CHIP_NO_ERROR; } diff --git a/src/platform/Linux/ThreadStackManagerImpl.h b/src/platform/Linux/ThreadStackManagerImpl.h index 3f168666b3e5ee..2c4a47a43a0c62 100755 --- a/src/platform/Linux/ThreadStackManagerImpl.h +++ b/src/platform/Linux/ThreadStackManagerImpl.h @@ -148,6 +148,8 @@ class ThreadStackManagerImpl : public ThreadStackManager std::unique_ptr mProxy; static CHIP_ERROR GLibMatterContextInitThreadStack(ThreadStackManagerImpl * self); + static CHIP_ERROR GLibMatterContextCallAttach(ThreadStackManagerImpl * self); + static CHIP_ERROR GLibMatterContextCallScan(ThreadStackManagerImpl * self); static void OnDbusPropertiesChanged(OpenthreadIoOpenthreadBorderRouter * proxy, GVariant * changed_properties, const gchar * const * invalidated_properties, gpointer user_data); void ThreadDeviceRoleChangedHandler(const gchar * role); From 2b3664c09f899d383b1ec2574878dc8c1ac79679 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Mon, 24 Apr 2023 11:09:36 +0200 Subject: [PATCH 12/12] Document GDBusCreateObjectManagerContext usage --- .../Linux/bluez/ChipDeviceScanner.cpp | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp index 62e4c1d03e093b..278ac2e623203d 100644 --- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp +++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp @@ -34,18 +34,19 @@ namespace Internal { namespace { -struct GDBusCancellableObjectManager +// Helper context for creating GDBusObjectManager with +// chip::DeviceLayer::GLibMatterContextInvokeSync() +struct GDBusCreateObjectManagerContext { - GCancellable * cancellable = nullptr; GDBusObjectManager * object = nullptr; + // Cancellable passed to g_dbus_object_manager_client_new_for_bus_sync() + // which later can be used to cancel the scan operation. + GCancellable * cancellable = nullptr; - GDBusCancellableObjectManager() : cancellable(g_cancellable_new()) {} - ~GDBusCancellableObjectManager() + GDBusCreateObjectManagerContext() : cancellable(g_cancellable_new()) {} + ~GDBusCreateObjectManagerContext() { - if (cancellable != nullptr) - { - g_object_unref(cancellable); - } + g_object_unref(cancellable); if (object != nullptr) { g_object_unref(object); @@ -53,18 +54,18 @@ struct GDBusCancellableObjectManager } }; -CHIP_ERROR MainLoopCreateObjectManager(GDBusCancellableObjectManager * cancellableObjectManager) +CHIP_ERROR MainLoopCreateObjectManager(GDBusCreateObjectManagerContext * context) { // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise, // all D-Bus signals will be delivered to the GLib global default main context. VerifyOrDie(g_main_context_get_thread_default() != nullptr); std::unique_ptr err; - cancellableObjectManager->object = g_dbus_object_manager_client_new_for_bus_sync( + context->object = g_dbus_object_manager_client_new_for_bus_sync( G_BUS_TYPE_SYSTEM, G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_NONE, BLUEZ_INTERFACE, "/", bluez_object_manager_client_get_proxy_type, nullptr /* unused user data in the Proxy Type Func */, - nullptr /* destroy notify */, cancellableObjectManager->cancellable, &MakeUniquePointerReceiver(err).Get()); - VerifyOrReturnError(cancellableObjectManager->object != nullptr, CHIP_ERROR_INTERNAL, + nullptr /* destroy notify */, context->cancellable, &MakeUniquePointerReceiver(err).Get()); + VerifyOrReturnError(context->object != nullptr, CHIP_ERROR_INTERNAL, ChipLogError(Ble, "Failed to get DBUS object manager for device scanning: %s", err->message)); return CHIP_NO_ERROR; @@ -118,14 +119,13 @@ ChipDeviceScanner::~ChipDeviceScanner() std::unique_ptr ChipDeviceScanner::Create(BluezAdapter1 * adapter, ChipDeviceScannerDelegate * delegate) { - GDBusCancellableObjectManager manager; + GDBusCreateObjectManagerContext context; CHIP_ERROR err; - VerifyOrExit(manager.cancellable != nullptr, ); - err = PlatformMgrImpl().GLibMatterContextInvokeSync(MainLoopCreateObjectManager, &manager); + err = PlatformMgrImpl().GLibMatterContextInvokeSync(MainLoopCreateObjectManager, &context); VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(Ble, "Failed to create BLE object manager")); - return std::make_unique(manager.object, adapter, manager.cancellable, delegate); + return std::make_unique(context.object, adapter, context.cancellable, delegate); exit: return std::unique_ptr();