DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0B597208AE
MIME-Version: 1.0
In-Reply-To: <CABBYNZKOij42rhG59VPyLTcdzA9se1BUX_RVhG7n4u6hvNn=HA@mail.gmail.com>
References: <20181001111338.11634-1-luiz.dentz@gmail.com> <CABBYNZKOij42rhG59VPyLTcdzA9se1BUX_RVhG7n4u6hvNn=HA@mail.gmail.com>
From:   Sukesh Srikakula <sukeshs@google.com>
Date:   Mon, 1 Oct 2018 11:31:24 -0700
Message-ID: <CAE+ULpCT0uW607yjg1D2dvkVXrmufa2_hwoAfDf2MPpM8c80WQ@mail.gmail.com>
Subject: Re: [PATCH BlueZ] gatt: Fix not cleaning up device state properly
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        Yunhan Wang <yunhanw@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

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
<luiz.dentz@gmail.com> wrote:
> On Mon, Oct 1, 2018 at 2:13 PM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> 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