[Linux] Do not reuse cancellable object (#31828)
* [Linux] Do not reuse cancellable object
Per documentation for g_cancellable_reset():
> Note that it is generally not a good idea to reuse an existing
> cancellable for more operations after it has been cancelled once, as
> this function might tempt you to do. The recommended practice is to
> drop the reference to a cancellable after cancelling it, and let it
> die with the outstanding async operations.
* Update cancellable in ChipDeviceScanner
diff --git a/src/platform/GLibTypeDeleter.h b/src/platform/GLibTypeDeleter.h
index 93ff40f..e6d9bfd 100644
--- a/src/platform/GLibTypeDeleter.h
+++ b/src/platform/GLibTypeDeleter.h
@@ -109,6 +109,12 @@
};
template <>
+struct GAutoPtrDeleter<GCancellable>
+{
+ using deleter = GObjectDeleter;
+};
+
+template <>
struct GAutoPtrDeleter<GDBusConnection>
{
using deleter = GObjectDeleter;
diff --git a/src/platform/Linux/BLEManagerImpl.cpp b/src/platform/Linux/BLEManagerImpl.cpp
index 241c13e..12232bc 100644
--- a/src/platform/Linux/BLEManagerImpl.cpp
+++ b/src/platform/Linux/BLEManagerImpl.cpp
@@ -571,7 +571,7 @@
// Initializes the Bluez BLE layer if needed.
if (mServiceMode == ConnectivityManager::kCHIPoBLEServiceMode_Enabled && !mFlags.Has(Flags::kBluezBLELayerInitialized))
{
- SuccessOrExit(err = mEndpoint.Init(mAdapterId, mIsCentral, nullptr));
+ SuccessOrExit(err = mEndpoint.Init(mIsCentral, mAdapterId));
mFlags.Set(Flags::kBluezBLELayerInitialized);
}
diff --git a/src/platform/Linux/bluez/BluezEndpoint.cpp b/src/platform/Linux/bluez/BluezEndpoint.cpp
index 24614eb..51a418b 100644
--- a/src/platform/Linux/bluez/BluezEndpoint.cpp
+++ b/src/platform/Linux/bluez/BluezEndpoint.cpp
@@ -656,22 +656,14 @@
return err;
}
-CHIP_ERROR BluezEndpoint::Init(uint32_t aAdapterId, bool aIsCentral, const char * apBleAddr)
+CHIP_ERROR BluezEndpoint::Init(bool aIsCentral, uint32_t aAdapterId)
{
- CHIP_ERROR err;
+ VerifyOrReturnError(!mIsInitialized, CHIP_ERROR_INCORRECT_STATE);
mAdapterId = aAdapterId;
mIsCentral = aIsCentral;
- if (apBleAddr != nullptr)
- mpAdapterAddr = g_strdup(apBleAddr);
-
- if (aIsCentral)
- {
- mpConnectCancellable = g_cancellable_new();
- }
-
- err = PlatformMgrImpl().GLibMatterContextInvokeSync(
+ CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(
+[](BluezEndpoint * self) { return self->StartupEndpointBindings(); }, this);
VerifyOrReturnError(err == CHIP_NO_ERROR, err, ChipLogError(DeviceLayer, "Failed to schedule endpoint initialization"));
@@ -681,6 +673,13 @@
return CHIP_NO_ERROR;
}
+CHIP_ERROR BluezEndpoint::Init(bool aIsCentral, const char * apBleAddr)
+{
+ VerifyOrReturnError(!mIsInitialized, CHIP_ERROR_INCORRECT_STATE);
+ mpAdapterAddr = g_strdup(apBleAddr);
+ return Init(aIsCentral, mAdapterId);
+}
+
void BluezEndpoint::Shutdown()
{
VerifyOrReturn(mIsInitialized);
@@ -707,8 +706,6 @@
g_object_unref(self->mpC2);
if (self->mpC3 != nullptr)
g_object_unref(self->mpC3);
- if (self->mpConnectCancellable != nullptr)
- g_object_unref(self->mpConnectCancellable);
return CHIP_NO_ERROR;
},
this);
@@ -744,7 +741,7 @@
g_clear_error(&MakeUniquePointerReceiver(error).Get());
bluez_device1_call_disconnect_sync(device, nullptr, &MakeUniquePointerReceiver(error).Get());
- bluez_device1_call_connect(device, params->mEndpoint.mpConnectCancellable, ConnectDeviceDone, params);
+ bluez_device1_call_connect(device, params->mEndpoint.mConnectCancellable.get(), ConnectDeviceDone, params);
return;
}
@@ -760,8 +757,7 @@
CHIP_ERROR BluezEndpoint::ConnectDeviceImpl(ConnectParams * apParams)
{
- g_cancellable_reset(apParams->mEndpoint.mpConnectCancellable);
- bluez_device1_call_connect(apParams->mpDevice, apParams->mEndpoint.mpConnectCancellable, ConnectDeviceDone, apParams);
+ bluez_device1_call_connect(apParams->mpDevice, apParams->mEndpoint.mConnectCancellable.get(), ConnectDeviceDone, apParams);
return CHIP_NO_ERROR;
}
@@ -770,6 +766,7 @@
auto params = chip::Platform::New<ConnectParams>(*this, &aDevice);
VerifyOrReturnError(params != nullptr, CHIP_ERROR_NO_MEMORY);
+ mConnectCancellable.reset(g_cancellable_new());
if (PlatformMgrImpl().GLibMatterContextInvokeSync(ConnectDeviceImpl, params) != CHIP_NO_ERROR)
{
ChipLogError(Ble, "Failed to schedule ConnectDeviceImpl() on CHIPoBluez thread");
@@ -782,8 +779,8 @@
void BluezEndpoint::CancelConnect()
{
- VerifyOrDie(mpConnectCancellable != nullptr);
- g_cancellable_cancel(mpConnectCancellable);
+ g_cancellable_cancel(mConnectCancellable.get());
+ mConnectCancellable.reset();
}
} // namespace Internal
diff --git a/src/platform/Linux/bluez/BluezEndpoint.h b/src/platform/Linux/bluez/BluezEndpoint.h
index 397b5a1..772b503 100644
--- a/src/platform/Linux/bluez/BluezEndpoint.h
+++ b/src/platform/Linux/bluez/BluezEndpoint.h
@@ -54,6 +54,7 @@
#include <ble/CHIPBleServiceData.h>
#include <lib/core/CHIPError.h>
+#include <platform/GLibTypeDeleter.h>
#include <platform/Linux/dbus/bluez/DbusBluez.h>
#include "BluezConnection.h"
@@ -69,7 +70,8 @@
BluezEndpoint() = default;
~BluezEndpoint() = default;
- CHIP_ERROR Init(uint32_t aAdapterId, bool aIsCentral, const char * apBleAddr);
+ CHIP_ERROR Init(bool aIsCentral, uint32_t aAdapterId);
+ CHIP_ERROR Init(bool aIsCentral, const char * apBleAddr);
void Shutdown();
BluezAdapter1 * GetAdapter() const { return mpAdapter; }
@@ -148,8 +150,8 @@
BluezGattCharacteristic1 * mpC3 = nullptr;
std::unordered_map<std::string, BluezConnection *> mConnMap;
- GCancellable * mpConnectCancellable = nullptr;
- char * mpPeerDevicePath = nullptr;
+ GAutoPtr<GCancellable> mConnectCancellable;
+ char * mpPeerDevicePath = nullptr;
// Allow BluezConnection to access our private members
friend class BluezConnection;
diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.cpp b/src/platform/Linux/bluez/ChipDeviceScanner.cpp
index 3a3115a..5d9cbf2 100644
--- a/src/platform/Linux/bluez/ChipDeviceScanner.cpp
+++ b/src/platform/Linux/bluez/ChipDeviceScanner.cpp
@@ -58,9 +58,8 @@
// Make this function idempotent by shutting down previously initialized state if any.
Shutdown();
- mAdapter = BLUEZ_ADAPTER1(g_object_ref(adapter));
- mCancellable = g_cancellable_new();
- mDelegate = delegate;
+ mAdapter = BLUEZ_ADAPTER1(g_object_ref(adapter));
+ mDelegate = delegate;
// Create the D-Bus object manager client object on the glib thread, so that all D-Bus signals
// will be delivered to the glib thread.
@@ -73,7 +72,7 @@
self->mManager = 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 */, self->mCancellable, &MakeUniquePointerReceiver(err).Get());
+ nullptr /* destroy notify */, nullptr /* cancellable */, &MakeUniquePointerReceiver(err).Get());
VerifyOrReturnError(self->mManager != nullptr, CHIP_ERROR_INTERNAL,
ChipLogError(Ble, "Failed to get D-Bus object manager for device scanning: %s", err->message));
return CHIP_NO_ERROR;
@@ -100,8 +99,6 @@
g_object_unref(self->mManager);
if (self->mAdapter != nullptr)
g_object_unref(self->mAdapter);
- if (self->mCancellable != nullptr)
- g_object_unref(self->mCancellable);
return CHIP_NO_ERROR;
},
this);
@@ -115,6 +112,7 @@
VerifyOrReturnError(mScannerState != ChipDeviceScannerState::SCANNER_SCANNING, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mTimerState == ScannerTimerState::TIMER_CANCELED, CHIP_ERROR_INCORRECT_STATE);
+ mCancellable.reset(g_cancellable_new());
if (PlatformMgrImpl().GLibMatterContextInvokeSync(MainLoopStartScan, this) != CHIP_NO_ERROR)
{
ChipLogError(Ble, "Failed to schedule BLE scan start.");
@@ -191,7 +189,9 @@
{
GAutoPtr<GError> error;
- g_cancellable_cancel(self->mCancellable); // in case we are currently running a scan
+ // In case we are currently running a scan
+ g_cancellable_cancel(self->mCancellable.get());
+ self->mCancellable.reset();
if (self->mObjectAddedSignal)
{
@@ -303,7 +303,7 @@
g_variant_builder_add(&filterBuilder, "{sv}", "Transport", g_variant_new_string("le"));
GVariant * filter = g_variant_builder_end(&filterBuilder);
- if (!bluez_adapter1_call_set_discovery_filter_sync(self->mAdapter, filter, self->mCancellable,
+ if (!bluez_adapter1_call_set_discovery_filter_sync(self->mAdapter, filter, self->mCancellable.get(),
&MakeUniquePointerReceiver(error).Get()))
{
// Not critical: ignore if fails
@@ -312,7 +312,8 @@
}
ChipLogProgress(Ble, "BLE initiating scan.");
- if (!bluez_adapter1_call_start_discovery_sync(self->mAdapter, self->mCancellable, &MakeUniquePointerReceiver(error).Get()))
+ if (!bluez_adapter1_call_start_discovery_sync(self->mAdapter, self->mCancellable.get(),
+ &MakeUniquePointerReceiver(error).Get()))
{
ChipLogError(Ble, "Failed to start discovery: %s", error->message);
return CHIP_ERROR_INTERNAL;
diff --git a/src/platform/Linux/bluez/ChipDeviceScanner.h b/src/platform/Linux/bluez/ChipDeviceScanner.h
index 4785a73..1271ec3 100644
--- a/src/platform/Linux/bluez/ChipDeviceScanner.h
+++ b/src/platform/Linux/bluez/ChipDeviceScanner.h
@@ -23,6 +23,7 @@
#include <ble/CHIPBleServiceData.h>
#include <lib/core/CHIPError.h>
+#include <platform/GLibTypeDeleter.h>
#include <platform/Linux/dbus/bluez/DbusBluez.h>
#include <system/SystemLayer.h>
@@ -106,13 +107,13 @@
GDBusObjectManager * mManager = nullptr;
BluezAdapter1 * mAdapter = nullptr;
- GCancellable * mCancellable = nullptr;
ChipDeviceScannerDelegate * mDelegate = nullptr;
gulong mObjectAddedSignal = 0;
gulong mInterfaceChangedSignal = 0;
ChipDeviceScannerState mScannerState = ChipDeviceScannerState::SCANNER_UNINITIALIZED;
/// Used to track if timer has already expired and doesn't need to be canceled.
ScannerTimerState mTimerState = ScannerTimerState::TIMER_CANCELED;
+ GAutoPtr<GCancellable> mCancellable;
};
} // namespace Internal
