From 6aade96108f48d20382950aff5610e0df24e5616 Mon Sep 17 00:00:00 2001 From: Oliver Wolff Date: Wed, 10 Apr 2019 15:06:30 +0200 Subject: winrt: Fix reading of descriptor values We have to access the service data instead of relying on passed references as these references might have run out of scope and thus might not be valid any more. Fixes: QTBUG-75070 Change-Id: I02ad0fef2337488c926fb950ddf2da6eda56a396 Reviewed-by: Alex Blasche --- src/bluetooth/qlowenergycontroller_winrt.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'src/bluetooth') diff --git a/src/bluetooth/qlowenergycontroller_winrt.cpp b/src/bluetooth/qlowenergycontroller_winrt.cpp index 6efa2231..f946b541 100644 --- a/src/bluetooth/qlowenergycontroller_winrt.cpp +++ b/src/bluetooth/qlowenergycontroller_winrt.cpp @@ -779,7 +779,7 @@ void QLowEnergyControllerPrivateWinRT::readDescriptor(const QSharedPointercharacteristicList.value(charHandle); + const QLowEnergyServicePrivate::CharData charData = service->characteristicList.value(charHandle); ComPtr characteristic = getNativeCharacteristic(service->uuid, charData.uuid); if (!characteristic) { qCDebug(QT_BT_WINRT) << "Could not obtain native characteristic" << charData.uuid @@ -791,12 +791,13 @@ void QLowEnergyControllerPrivateWinRT::readDescriptor(const QSharedPointer> readOp; HRESULT hr = characteristic->ReadClientCharacteristicConfigurationDescriptorAsync(&readOp); Q_ASSERT_SUCCEEDED(hr); - auto readCompletedLambda = [&charData, charHandle, &descData, descHandle, service] + auto readCompletedLambda = [charHandle, descHandle, service] (IAsyncOperation *op, AsyncStatus status) { if (status == AsyncStatus::Canceled || status == AsyncStatus::Error) { @@ -837,9 +838,11 @@ void QLowEnergyControllerPrivateWinRT::readDescriptor(const QSharedPointersetError(QLowEnergyService::DescriptorReadError); return S_OK; } + QLowEnergyServicePrivate::DescData descData; + descData.uuid = QBluetoothUuid::ClientCharacteristicConfiguration; descData.value = QByteArray(2, Qt::Uninitialized); qToLittleEndian(result, descData.value.data()); - charData.descriptorList.insert(descHandle, descData); + service->characteristicList[charHandle].descriptorList[descHandle] = descData; emit service->descriptorRead(QLowEnergyDescriptor(service, charHandle, descHandle), descData.value); return S_OK; @@ -857,7 +860,7 @@ void QLowEnergyControllerPrivateWinRT::readDescriptor(const QSharedPointer> readOp; hr = descriptor->ReadValueWithCacheModeAsync(BluetoothCacheMode_Uncached, &readOp); Q_ASSERT_SUCCEEDED(hr); - auto readCompletedLambda = [&charData, charHandle, &descData, descHandle, service] + auto readCompletedLambda = [charHandle, descHandle, descUuid, service] (IAsyncOperation *op, AsyncStatus status) { if (status == AsyncStatus::Canceled || status == AsyncStatus::Error) { @@ -873,11 +876,12 @@ void QLowEnergyControllerPrivateWinRT::readDescriptor(const QSharedPointersetError(QLowEnergyService::DescriptorReadError); return S_OK; } - if (descData.uuid == QBluetoothUuid::CharacteristicUserDescription) + QLowEnergyServicePrivate::DescData descData; + if (descUuid == QBluetoothUuid::CharacteristicUserDescription) descData.value = byteArrayFromGattResult(descriptorValue, true); else descData.value = byteArrayFromGattResult(descriptorValue); - charData.descriptorList.insert(descHandle, descData); + service->characteristicList[charHandle].descriptorList[descHandle] = descData; emit service->descriptorRead(QLowEnergyDescriptor(service, charHandle, descHandle), descData.value); return S_OK; -- cgit v1.2.3 From 717b95c7863278332ec4eb4dfbbfb6147a5ac9ed Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Wed, 10 Apr 2019 13:02:00 +0200 Subject: CoreBluetooth: add a missing -peripheral:didModifyServices: method With some peculiar device we suddenly (during the service details discovery) got a crash with CBDescriptor suddenly becoming something else - NSString, NSMutableArray etc. - meaning the object was deleted and its memory re-used. It would appear, CBPeripheral can suddenly change it's services tree and it informs its delegate (aka 'us') about this change using the (previously) missing method. In this method we cannot do much, due to the specificity of our public API that allows concurrent discoveries, it's 'non-monolitic' (in several steps) discoveries etc. etc. So the only thing we can do - stop everything, remove all services, transition to QLowEnergyController::ConnectedState and wait for a user to re-discover services. Fixes: QTBUG-75043 Change-Id: Ie98d90aea112e40b4c6771e3f7315772dfd92b39 Reviewed-by: Alex Blasche --- src/bluetooth/osx/osxbtcentralmanager.mm | 24 ++++++++++++++++++++++++ src/bluetooth/osx/osxbtnotifier_p.h | 1 + src/bluetooth/qlowenergycontroller_osx.mm | 17 +++++++++++++++++ src/bluetooth/qlowenergycontroller_osx_p.h | 1 + 4 files changed, 43 insertions(+) (limited to 'src/bluetooth') diff --git a/src/bluetooth/osx/osxbtcentralmanager.mm b/src/bluetooth/osx/osxbtcentralmanager.mm index 41713909..6b742684 100644 --- a/src/bluetooth/osx/osxbtcentralmanager.mm +++ b/src/bluetooth/osx/osxbtcentralmanager.mm @@ -1373,6 +1373,30 @@ QT_USE_NAMESPACE [self discoverIncludedServices]; } +- (void)peripheral:(CBPeripheral *)aPeripheral + didModifyServices:(NSArray *)invalidatedServices +{ + Q_UNUSED(aPeripheral) + Q_UNUSED(invalidatedServices) + + qCWarning(QT_BT_OSX) << "The peripheral has modified its services."; + // "This method is invoked whenever one or more services of a peripheral have changed. + // A peripheral’s services have changed if: + // * A service is removed from the peripheral’s database + // * A new service is added to the peripheral’s database + // * A service that was previously removed from the peripheral’s + // database is readded to the database at a different location" + + // In case new services were added - we have to discover them. + // In case some were removed - we can end up with dangling pointers + // (see our 'watchdogs', for example). To handle the situation + // we stop all current operations here, report to QLowEnergyController + // so that it can trigger re-discovery. + [self reset]; + managerState = OSXBluetooth::CentralManagerIdle; + if (notifier) + emit notifier->servicesWereModified(); +} - (void)peripheral:(CBPeripheral *)aPeripheral didDiscoverIncludedServicesForService:(CBService *)service error:(NSError *)error diff --git a/src/bluetooth/osx/osxbtnotifier_p.h b/src/bluetooth/osx/osxbtnotifier_p.h index 47ee6ba1..dca6c268 100644 --- a/src/bluetooth/osx/osxbtnotifier_p.h +++ b/src/bluetooth/osx/osxbtnotifier_p.h @@ -89,6 +89,7 @@ Q_SIGNALS: void descriptorRead(QLowEnergyHandle descHandle, const QByteArray &value); void descriptorWritten(QLowEnergyHandle descHandle, const QByteArray &value); void notificationEnabled(QLowEnergyHandle charHandle, bool enabled); + void servicesWereModified(); void LEnotSupported(); void CBManagerError(QBluetoothDeviceDiscoveryAgent::Error error); diff --git a/src/bluetooth/qlowenergycontroller_osx.mm b/src/bluetooth/qlowenergycontroller_osx.mm index 8cef621c..8bcdc22e 100644 --- a/src/bluetooth/qlowenergycontroller_osx.mm +++ b/src/bluetooth/qlowenergycontroller_osx.mm @@ -339,6 +339,21 @@ void QLowEnergyControllerPrivateOSX::_q_serviceDetailsDiscoveryFinished(QSharedP qtService->setState(QLowEnergyService::ServiceDiscovered); } +void QLowEnergyControllerPrivateOSX::_q_servicesWereModified() +{ + if (!(controllerState == QLowEnergyController::DiscoveringState + || controllerState == QLowEnergyController::DiscoveredState)) { + qCWarning(QT_BT_OSX) << "services were modified while controller is not in Discovered/Discovering state"; + return; + } + + if (controllerState == QLowEnergyController::DiscoveredState) + invalidateServices(); + + controllerState = QLowEnergyController::ConnectedState; + q_ptr->discoverServices(); +} + void QLowEnergyControllerPrivateOSX::_q_characteristicRead(QLowEnergyHandle charHandle, const QByteArray &value) { @@ -989,6 +1004,8 @@ bool QLowEnergyControllerPrivateOSX::connectSlots(OSXBluetooth::LECBManagerNotif this, &QLowEnergyControllerPrivateOSX::_q_serviceDiscoveryFinished); ok = ok && connect(notifier, &LECBManagerNotifier::serviceDetailsDiscoveryFinished, this, &QLowEnergyControllerPrivateOSX::_q_serviceDetailsDiscoveryFinished); + ok = ok && connect(notifier, &LECBManagerNotifier::servicesWereModified, + this, &QLowEnergyControllerPrivateOSX::_q_servicesWereModified); ok = ok && connect(notifier, &LECBManagerNotifier::characteristicRead, this, &QLowEnergyControllerPrivateOSX::_q_characteristicRead); ok = ok && connect(notifier, &LECBManagerNotifier::characteristicWritten, diff --git a/src/bluetooth/qlowenergycontroller_osx_p.h b/src/bluetooth/qlowenergycontroller_osx_p.h index 24b7c6e9..da959895 100644 --- a/src/bluetooth/qlowenergycontroller_osx_p.h +++ b/src/bluetooth/qlowenergycontroller_osx_p.h @@ -98,6 +98,7 @@ private Q_SLOTS: void _q_serviceDiscoveryFinished(); void _q_serviceDetailsDiscoveryFinished(QSharedPointer service); + void _q_servicesWereModified(); void _q_characteristicRead(QLowEnergyHandle charHandle, const QByteArray &value); void _q_characteristicWritten(QLowEnergyHandle charHandle, const QByteArray &value); -- cgit v1.2.3