2018-10-01 11:13:45

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ] gatt: Fix not cleaning up device state properly

From: Luiz Augusto von Dentz <[email protected]>

If the device is removed locally device_free would end up calling
bt_att_unref which won't trigger any disconnect callback necessary
to remove device states.
---
src/device.c | 3 +++
src/gatt-database.c | 24 ++++++++++++++++++++++++
src/gatt-database.h | 2 ++
3 files changed, 29 insertions(+)

diff --git a/src/device.c b/src/device.c
index ef5b8f86a..5ddfb9266 100644
--- a/src/device.c
+++ b/src/device.c
@@ -580,6 +580,9 @@ static void gatt_server_cleanup(struct btd_device *device)

bt_gatt_server_unref(device->server);
device->server = NULL;
+
+ btd_gatt_database_att_disconnected(
+ btd_adapter_get_database(device->adapter), device->att);
}

static void attio_cleanup(struct btd_device *device)
diff --git a/src/gatt-database.c b/src/gatt-database.c
index abcf7b759..bdabcb4d6 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -3349,6 +3349,30 @@ void btd_gatt_database_att_connected(struct btd_gatt_database *database,
state->pending = NULL;
}

+void btd_gatt_database_att_disconnected(struct btd_gatt_database *database,
+ struct bt_att *att)
+{
+ struct device_state *state;
+ bdaddr_t bdaddr;
+ uint8_t bdaddr_type;
+
+ DBG("");
+
+ if (!get_dst_info(att, &bdaddr, &bdaddr_type))
+ return;
+
+ state = find_device_state(database, &bdaddr, bdaddr_type);
+ if (!state)
+ return;
+
+ if (state->disc_id) {
+ bt_att_unregister_disconnect(att, state->disc_id);
+ state->disc_id = 0;
+ }
+
+ att_disconnected(0, state);
+}
+
static void restore_ccc(struct btd_gatt_database *database,
const bdaddr_t *addr, uint8_t addr_type, uint16_t value)
{
diff --git a/src/gatt-database.h b/src/gatt-database.h
index a6c3139c4..9e7cda8c2 100644
--- a/src/gatt-database.h
+++ b/src/gatt-database.h
@@ -25,5 +25,7 @@ void btd_gatt_database_destroy(struct btd_gatt_database *database);
struct gatt_db *btd_gatt_database_get_db(struct btd_gatt_database *database);
void btd_gatt_database_att_connected(struct btd_gatt_database *database,
struct bt_att *att);
+void btd_gatt_database_att_disconnected(struct btd_gatt_database *database,
+ struct bt_att *att);

void btd_gatt_database_restore_svc_chng_ccc(struct btd_gatt_database *database);
--
2.17.1



2018-10-01 11:44:35

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ] gatt: Fix not cleaning up device state properly

On Mon, Oct 1, 2018 at 2:13 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> If the device is removed locally device_free would end up calling
> bt_att_unref which won't trigger any disconnect callback necessary
> to remove device states.
> ---
> src/device.c | 3 +++
> src/gatt-database.c | 24 ++++++++++++++++++++++++
> src/gatt-database.h | 2 ++
> 3 files changed, 29 insertions(+)
>
> diff --git a/src/device.c b/src/device.c
> index ef5b8f86a..5ddfb9266 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -580,6 +580,9 @@ static void gatt_server_cleanup(struct btd_device *device)
>
> bt_gatt_server_unref(device->server);
> device->server = NULL;
> +
> + btd_gatt_database_att_disconnected(
> + btd_adapter_get_database(device->adapter), device->att);
> }
>
> static void attio_cleanup(struct btd_device *device)
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index abcf7b759..bdabcb4d6 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -3349,6 +3349,30 @@ void btd_gatt_database_att_connected(struct btd_gatt_database *database,
> state->pending = NULL;
> }
>
> +void btd_gatt_database_att_disconnected(struct btd_gatt_database *database,
> + struct bt_att *att)
> +{
> + struct device_state *state;
> + bdaddr_t bdaddr;
> + uint8_t bdaddr_type;
> +
> + DBG("");
> +
> + if (!get_dst_info(att, &bdaddr, &bdaddr_type))
> + return;
> +
> + state = find_device_state(database, &bdaddr, bdaddr_type);
> + if (!state)
> + return;
> +
> + if (state->disc_id) {
> + bt_att_unregister_disconnect(att, state->disc_id);
> + state->disc_id = 0;
> + }
> +
> + att_disconnected(0, state);
> +}
> +
> static void restore_ccc(struct btd_gatt_database *database,
> const bdaddr_t *addr, uint8_t addr_type, uint16_t value)
> {
> diff --git a/src/gatt-database.h b/src/gatt-database.h
> index a6c3139c4..9e7cda8c2 100644
> --- a/src/gatt-database.h
> +++ b/src/gatt-database.h
> @@ -25,5 +25,7 @@ void btd_gatt_database_destroy(struct btd_gatt_database *database);
> struct gatt_db *btd_gatt_database_get_db(struct btd_gatt_database *database);
> void btd_gatt_database_att_connected(struct btd_gatt_database *database,
> struct bt_att *att);
> +void btd_gatt_database_att_disconnected(struct btd_gatt_database *database,
> + struct bt_att *att);
>
> void btd_gatt_database_restore_svc_chng_ccc(struct btd_gatt_database *database);
> --
> 2.17.1
>



--
Luiz Augusto von Dentz

2018-10-01 18:31:27

by Sukesh Srikakula

[permalink] [raw]
Subject: Re: [PATCH BlueZ] gatt: Fix not cleaning up device state properly

Hi Luiz,

Thanks for the patch.
But, I am still seeing the issue even after applying the patch.
btd_gatt_database_att_disconnected() is bailing out at the first
check, get_dst_info(). So, att_disconnected() is not getting called.

Here is the log message
daemon.debug bluetoothd[3913]:
src/gatt-database.c:btd_gatt_database_att_disconnected()
daemon.err bluetoothd[3913]: gatt: bt_io_get: getpeername: Transport
endpoint is not connected (107)
daemon.debug bluetoothd[3913]: attrib/gattrib.c:g_attrib_unref()
0x20669f0: g_attrib_unref=0
daemon.debug bluetoothd[3913]: src/device.c:device_free() 0x2064518
daemon.debug bluetoothd[3913]: plugins/policy.c:disconnect_cb() reason 3
daemon.debug bluetoothd[3913]:
src/adapter.c:bonding_attempt_complete() hci0 bdaddr 7B:01:A4:6B:09:2C
type 2 status 0xe
daemon.debug bluetoothd[3913]: src/adapter.c:resume_discovery()

Regards,
Sukesh.

On Mon, Oct 1, 2018 at 4:44 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> On Mon, Oct 1, 2018 at 2:13 PM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> From: Luiz Augusto von Dentz <[email protected]>
>>
>> If the device is removed locally device_free would end up calling
>> bt_att_unref which won't trigger any disconnect callback necessary
>> to remove device states.
>> ---
>> src/device.c | 3 +++
>> src/gatt-database.c | 24 ++++++++++++++++++++++++
>> src/gatt-database.h | 2 ++
>> 3 files changed, 29 insertions(+)
>>
>> diff --git a/src/device.c b/src/device.c
>> index ef5b8f86a..5ddfb9266 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -580,6 +580,9 @@ static void gatt_server_cleanup(struct btd_device *device)
>>
>> bt_gatt_server_unref(device->server);
>> device->server = NULL;
>> +
>> + btd_gatt_database_att_disconnected(
>> + btd_adapter_get_database(device->adapter), device->att);
>> }
>>
>> static void attio_cleanup(struct btd_device *device)
>> diff --git a/src/gatt-database.c b/src/gatt-database.c
>> index abcf7b759..bdabcb4d6 100644
>> --- a/src/gatt-database.c
>> +++ b/src/gatt-database.c
>> @@ -3349,6 +3349,30 @@ void btd_gatt_database_att_connected(struct btd_gatt_database *database,
>> state->pending = NULL;
>> }
>>
>> +void btd_gatt_database_att_disconnected(struct btd_gatt_database *database,
>> + struct bt_att *att)
>> +{
>> + struct device_state *state;
>> + bdaddr_t bdaddr;
>> + uint8_t bdaddr_type;
>> +
>> + DBG("");
>> +
>> + if (!get_dst_info(att, &bdaddr, &bdaddr_type))
>> + return;
>> +
>> + state = find_device_state(database, &bdaddr, bdaddr_type);
>> + if (!state)
>> + return;
>> +
>> + if (state->disc_id) {
>> + bt_att_unregister_disconnect(att, state->disc_id);
>> + state->disc_id = 0;
>> + }
>> +
>> + att_disconnected(0, state);
>> +}
>> +
>> static void restore_ccc(struct btd_gatt_database *database,
>> const bdaddr_t *addr, uint8_t addr_type, uint16_t value)
>> {
>> diff --git a/src/gatt-database.h b/src/gatt-database.h
>> index a6c3139c4..9e7cda8c2 100644
>> --- a/src/gatt-database.h
>> +++ b/src/gatt-database.h
>> @@ -25,5 +25,7 @@ void btd_gatt_database_destroy(struct btd_gatt_database *database);
>> struct gatt_db *btd_gatt_database_get_db(struct btd_gatt_database *database);
>> void btd_gatt_database_att_connected(struct btd_gatt_database *database,
>> struct bt_att *att);
>> +void btd_gatt_database_att_disconnected(struct btd_gatt_database *database,
>> + struct bt_att *att);
>>
>> void btd_gatt_database_restore_svc_chng_ccc(struct btd_gatt_database *database);
>> --
>> 2.17.1
>>
>
>
>
> --
> Luiz Augusto von Dentz