2014-12-10 08:53:24

by Lukasz Rymanowski

[permalink] [raw]
Subject: Re: [PATCH 00/25] android/gatt: Fix HOG connect/disconnect

Hi Luiz,

On 10 December 2014 at 09:46, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Lukasz,
>
> On Wed, Dec 10, 2014 at 10:38 AM, Lukasz Rymanowski
> <[email protected]> wrote:
>> Hi Luiz,
>>
>> On 10 December 2014 at 08:23, Luiz Augusto von Dentz
>> <[email protected]> wrote:
>>> Hi Lukasz,
>>>
>>> On Tue, Dec 9, 2014 at 5:11 PM, Lukasz Rymanowski
>>> <[email protected]> wrote:
>>>> When testing HOG against PTS we got failing testes if devices were
>>>> not bonded before the test.
>>>> With those patches this issue is not valid anymore.
>>>>
>>>> This set address couple of issues:
>>>> * Some minor fixes found during work on those patches (first 4 patches)
>>>>
>>>> * Changed GATT behaviour during dedicated bonding. Now GATT will not start
>>>> search services until bonding is done. That fixes PTS connection issue
>>>> (patches 5-8)
>>>>
>>>> * Tracking of GATT operations in hog/dis/bas/scpp. After I solved PTS
>>>> connection issue I could very easily observe many of crashes in BfA when
>>>> disconnecting HOG during service search session. This is because HOG
>>>> talks directly to gattrib and clear his attrib instance on HOG disconnect
>>>> even there are ongoing GATT operations. Patches 9 - 25 solve this issue.
>>>
>>> I remember applying a patch from Arman that should have fixed the
>>> problem above: shared/att: cancel_all before calling disconnect cb. In
>>> any if that doesn't really solve the problem the client should cancel
>>> their pending operations before releasing its reference which means no
>>> callback should be called after that.
>>
>> This is problem above bt_att, but It might be worth to consider such
>> scenario in shared/gatt.
>> Anyway, In Android attrib is used by HOG and also GATT with all the
>> applications above. GATT controls registered clients so if one client
>> unregister we should not see the problem. But if HOG and GATT have
>> ongoing gatt operations and in this point of time we disconnect HOG,
>> we should not break ongoing gatt operations for GATT (Android apps).
>> Note that link LE link can be still up because of some other GATT
>> application so no disconnect_cb will be called by shared/add. Thats
>> why attrib client should take care for its operations
>
> Yep, but that why I said you should probably cancel any operations,
> which seems you did, but then why you keep checking for disconnected
> in the callbacks? There looks like there another bug going on.

Canceling operation might not succeed, in that case callback will be
called anyway but unfortunately e.g. hog->attrib will be not valid
anymore and that's we need that additional check.

>
>> \Lukasz
>>>
>>>> Lukasz Rymanowski (25):
>>>> android/bluetooth: Minor typo fix
>>>> android/bluetooth: Minor fix, add missing NULL assignment
>>>> android/gatt: Fix for setting conn timeout
>>>> android/hog: Minor coding style fixes
>>>> android/bluetooth: Add possibility to register for bonded event
>>>> android/gatt: Add paired cb to gatt.c
>>>> android/bluetooth: Add API to check if device is bonding
>>>> android/gatt: Improve LE connection after pair
>>>> android/hog: Add support to track gatt operations
>>>> android/hog: Cancel all GATT operations on disconnect
>>>> android/hog: Add guards to the callbacks
>>>> android/hog: Keep track on primary and include service search
>>>> android/hog: Keep track on discover characteristic and descriptors
>>>> android/hog: Keep track on read/write char and descr
>>>> android/bas: Add queue to keep track on GATT operations
>>>> android/bas: Keep track read/write GATT operations
>>>> android/bas: Keep track on discover characteristic and descriptor
>>>> android/bas: Add guard to GATT callbacks
>>>> android/dis: Add queue to keep track on GATT operations
>>>> android/dis: Keep track on discover and read characteristic
>>>> android/dis: Add guard for GATT callbacks
>>>> android/scpp: Add queue to keep GATT operations
>>>> android/scpp: Keep track on discover characteristic and descriptor
>>>> android/scpp: Keep track on write operation
>>>> android/scpp: Add guard for GATT callbacks
>>>>
>>>> android/bas.c | 192 +++++++++++++++++++++++--
>>>> android/bluetooth.c | 52 ++++++-
>>>> android/bluetooth.h | 5 +
>>>> android/dis.c | 123 +++++++++++++++-
>>>> android/gatt.c | 70 ++++++++-
>>>> android/hog.c | 401 +++++++++++++++++++++++++++++++++++++++++++++-------
>>>> android/scpp.c | 171 ++++++++++++++++++++--
>>>> 7 files changed, 932 insertions(+), 82 deletions(-)
>>>>
>>>> --
>>>> 1.8.4
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>>
>>> --
>>> Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz


2014-12-11 15:19:00

by Lukasz Rymanowski

[permalink] [raw]
Subject: Re: [PATCH 00/25] android/gatt: Fix HOG connect/disconnect

Hi Luiz,

On 10 December 2014 at 10:27, Lukasz Rymanowski
<[email protected]> wrote:
> Hi Luiz,
>
> On 10 December 2014 at 10:25, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Lukasz,
>>
>> On Wed, Dec 10, 2014 at 11:20 AM, Lukasz Rymanowski
>> <[email protected]> wrote:
>>> Hi Luiz,
>>>
>>> On 10 December 2014 at 09:56, Luiz Augusto von Dentz
>>> <[email protected]> wrote:
>>>> Hi Lukasz,
>>>>
>>>> On Wed, Dec 10, 2014 at 10:53 AM, Lukasz Rymanowski
>>>> <[email protected]> wrote:
>>>>> Hi Luiz,
>>>>>
>>>>> On 10 December 2014 at 09:46, Luiz Augusto von Dentz
>>>>> <[email protected]> wrote:
>>>>>> Hi Lukasz,
>>>>>>
>>>>>> On Wed, Dec 10, 2014 at 10:38 AM, Lukasz Rymanowski
>>>>>> <[email protected]> wrote:
>>>>>>> Hi Luiz,
>>>>>>>
>>>>>>> On 10 December 2014 at 08:23, Luiz Augusto von Dentz
>>>>>>> <[email protected]> wrote:
>>>>>>>> Hi Lukasz,
>>>>>>>>
>>>>>>>> On Tue, Dec 9, 2014 at 5:11 PM, Lukasz Rymanowski
>>>>>>>> <[email protected]> wrote:
>>>>>>>>> When testing HOG against PTS we got failing testes if devices were
>>>>>>>>> not bonded before the test.
>>>>>>>>> With those patches this issue is not valid anymore.
>>>>>>>>>
>>>>>>>>> This set address couple of issues:
>>>>>>>>> * Some minor fixes found during work on those patches (first 4 patches)
>>>>>>>>>
>>>>>>>>> * Changed GATT behaviour during dedicated bonding. Now GATT will not start
>>>>>>>>> search services until bonding is done. That fixes PTS connection issue
>>>>>>>>> (patches 5-8)
>>>>>>>>>
>>>>>>>>> * Tracking of GATT operations in hog/dis/bas/scpp. After I solved PTS
>>>>>>>>> connection issue I could very easily observe many of crashes in BfA when
>>>>>>>>> disconnecting HOG during service search session. This is because HOG
>>>>>>>>> talks directly to gattrib and clear his attrib instance on HOG disconnect
>>>>>>>>> even there are ongoing GATT operations. Patches 9 - 25 solve this issue.
>>>>>>>>
>>>>>>>> I remember applying a patch from Arman that should have fixed the
>>>>>>>> problem above: shared/att: cancel_all before calling disconnect cb. In
>>>>>>>> any if that doesn't really solve the problem the client should cancel
>>>>>>>> their pending operations before releasing its reference which means no
>>>>>>>> callback should be called after that.
>>>>>>>
>>>>>>> This is problem above bt_att, but It might be worth to consider such
>>>>>>> scenario in shared/gatt.
>>>>>>> Anyway, In Android attrib is used by HOG and also GATT with all the
>>>>>>> applications above. GATT controls registered clients so if one client
>>>>>>> unregister we should not see the problem. But if HOG and GATT have
>>>>>>> ongoing gatt operations and in this point of time we disconnect HOG,
>>>>>>> we should not break ongoing gatt operations for GATT (Android apps).
>>>>>>> Note that link LE link can be still up because of some other GATT
>>>>>>> application so no disconnect_cb will be called by shared/add. Thats
>>>>>>> why attrib client should take care for its operations
>>>>>>
>>>>>> Yep, but that why I said you should probably cancel any operations,
>>>>>> which seems you did, but then why you keep checking for disconnected
>>>>>> in the callbacks? There looks like there another bug going on.
>>>>>
>>>>> Canceling operation might not succeed, in that case callback will be
>>>>> called anyway but unfortunately e.g. hog->attrib will be not valid
>>>>> anymore and that's we need that additional check.
>>>>
>>>> Not succeed? bt_att_cancel should always succeed except if we cannot
>>>> find the id.
>>>
>>> Just checked bt_att code and indeed, after calling cancel we should be
>>> sure that callback will not be called.
>>> However, I've seen something else during testing. That might be other
>>> bug then. Will look into it probably tomorrow.
>>
>> Yep, it might be another bug or perhaps one operation that was not
>> canceled for some reason.
>
> Agree
>

In the end it seems to be bug.
I had time to look closer into it and the problem is in attrib/gatt.c
Scenario:
When gattrib user (like HOG) calls gatt_discover_char or
gatt_discover_desc it will get valid request_id of sent or queued
request.
If remote response with not contain full range from the request then
attib/gatt.c will send following requests to remote, under the hood.
In this point of time request_id which e.g. HOG has is not valid
anymore, there is new one which is actually ignored by attrib/gatt.
This is why when later HOG is doing cancel, it gets false and in the
end callback is called with a response anyway.

To solve it, HOG needs to have latest req_id so it can cancel the
correct one (solution 1), or attrib/gatt.c has some kind of mapping on
original req_id and ongoing one (solution 2). Any other solutions are
welcome:)
Solution 1) Some callback to gattrib user (like HOG) with user data
and new request id so it can be updated by the user (?)
Solution 2) Here it will be tricky as attrib/gatt.c is only wrraper
so there is no place to keep such map. We could have some table but
since req_id is unsigned int it would be huge. Of course API to cancel
request would have to wrapped in attrib/gatt.c. Maybe we could add
some init to attrib/gatt.c which would init some special queue which
would track original req_id and ongoing req_id (?)

I'm not fan of any of this solutions especially that attrib/* is going
to be removed. I would go with those patches I made and when we start
use shared/gatt, then we can remove not needed code from HOG/DIS etc

What do you think?

\Ɓukasz

> \Lukasz
>>
>> --
>> Luiz Augusto von Dentz

2014-12-10 09:27:27

by Lukasz Rymanowski

[permalink] [raw]
Subject: Re: [PATCH 00/25] android/gatt: Fix HOG connect/disconnect

Hi Luiz,

On 10 December 2014 at 10:25, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Lukasz,
>
> On Wed, Dec 10, 2014 at 11:20 AM, Lukasz Rymanowski
> <[email protected]> wrote:
>> Hi Luiz,
>>
>> On 10 December 2014 at 09:56, Luiz Augusto von Dentz
>> <[email protected]> wrote:
>>> Hi Lukasz,
>>>
>>> On Wed, Dec 10, 2014 at 10:53 AM, Lukasz Rymanowski
>>> <[email protected]> wrote:
>>>> Hi Luiz,
>>>>
>>>> On 10 December 2014 at 09:46, Luiz Augusto von Dentz
>>>> <[email protected]> wrote:
>>>>> Hi Lukasz,
>>>>>
>>>>> On Wed, Dec 10, 2014 at 10:38 AM, Lukasz Rymanowski
>>>>> <[email protected]> wrote:
>>>>>> Hi Luiz,
>>>>>>
>>>>>> On 10 December 2014 at 08:23, Luiz Augusto von Dentz
>>>>>> <[email protected]> wrote:
>>>>>>> Hi Lukasz,
>>>>>>>
>>>>>>> On Tue, Dec 9, 2014 at 5:11 PM, Lukasz Rymanowski
>>>>>>> <[email protected]> wrote:
>>>>>>>> When testing HOG against PTS we got failing testes if devices were
>>>>>>>> not bonded before the test.
>>>>>>>> With those patches this issue is not valid anymore.
>>>>>>>>
>>>>>>>> This set address couple of issues:
>>>>>>>> * Some minor fixes found during work on those patches (first 4 patches)
>>>>>>>>
>>>>>>>> * Changed GATT behaviour during dedicated bonding. Now GATT will not start
>>>>>>>> search services until bonding is done. That fixes PTS connection issue
>>>>>>>> (patches 5-8)
>>>>>>>>
>>>>>>>> * Tracking of GATT operations in hog/dis/bas/scpp. After I solved PTS
>>>>>>>> connection issue I could very easily observe many of crashes in BfA when
>>>>>>>> disconnecting HOG during service search session. This is because HOG
>>>>>>>> talks directly to gattrib and clear his attrib instance on HOG disconnect
>>>>>>>> even there are ongoing GATT operations. Patches 9 - 25 solve this issue.
>>>>>>>
>>>>>>> I remember applying a patch from Arman that should have fixed the
>>>>>>> problem above: shared/att: cancel_all before calling disconnect cb. In
>>>>>>> any if that doesn't really solve the problem the client should cancel
>>>>>>> their pending operations before releasing its reference which means no
>>>>>>> callback should be called after that.
>>>>>>
>>>>>> This is problem above bt_att, but It might be worth to consider such
>>>>>> scenario in shared/gatt.
>>>>>> Anyway, In Android attrib is used by HOG and also GATT with all the
>>>>>> applications above. GATT controls registered clients so if one client
>>>>>> unregister we should not see the problem. But if HOG and GATT have
>>>>>> ongoing gatt operations and in this point of time we disconnect HOG,
>>>>>> we should not break ongoing gatt operations for GATT (Android apps).
>>>>>> Note that link LE link can be still up because of some other GATT
>>>>>> application so no disconnect_cb will be called by shared/add. Thats
>>>>>> why attrib client should take care for its operations
>>>>>
>>>>> Yep, but that why I said you should probably cancel any operations,
>>>>> which seems you did, but then why you keep checking for disconnected
>>>>> in the callbacks? There looks like there another bug going on.
>>>>
>>>> Canceling operation might not succeed, in that case callback will be
>>>> called anyway but unfortunately e.g. hog->attrib will be not valid
>>>> anymore and that's we need that additional check.
>>>
>>> Not succeed? bt_att_cancel should always succeed except if we cannot
>>> find the id.
>>
>> Just checked bt_att code and indeed, after calling cancel we should be
>> sure that callback will not be called.
>> However, I've seen something else during testing. That might be other
>> bug then. Will look into it probably tomorrow.
>
> Yep, it might be another bug or perhaps one operation that was not
> canceled for some reason.

Agree

\Lukasz
>
> --
> Luiz Augusto von Dentz

2014-12-10 09:25:58

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 00/25] android/gatt: Fix HOG connect/disconnect

Hi Lukasz,

On Wed, Dec 10, 2014 at 11:20 AM, Lukasz Rymanowski
<[email protected]> wrote:
> Hi Luiz,
>
> On 10 December 2014 at 09:56, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Lukasz,
>>
>> On Wed, Dec 10, 2014 at 10:53 AM, Lukasz Rymanowski
>> <[email protected]> wrote:
>>> Hi Luiz,
>>>
>>> On 10 December 2014 at 09:46, Luiz Augusto von Dentz
>>> <[email protected]> wrote:
>>>> Hi Lukasz,
>>>>
>>>> On Wed, Dec 10, 2014 at 10:38 AM, Lukasz Rymanowski
>>>> <[email protected]> wrote:
>>>>> Hi Luiz,
>>>>>
>>>>> On 10 December 2014 at 08:23, Luiz Augusto von Dentz
>>>>> <[email protected]> wrote:
>>>>>> Hi Lukasz,
>>>>>>
>>>>>> On Tue, Dec 9, 2014 at 5:11 PM, Lukasz Rymanowski
>>>>>> <[email protected]> wrote:
>>>>>>> When testing HOG against PTS we got failing testes if devices were
>>>>>>> not bonded before the test.
>>>>>>> With those patches this issue is not valid anymore.
>>>>>>>
>>>>>>> This set address couple of issues:
>>>>>>> * Some minor fixes found during work on those patches (first 4 patches)
>>>>>>>
>>>>>>> * Changed GATT behaviour during dedicated bonding. Now GATT will not start
>>>>>>> search services until bonding is done. That fixes PTS connection issue
>>>>>>> (patches 5-8)
>>>>>>>
>>>>>>> * Tracking of GATT operations in hog/dis/bas/scpp. After I solved PTS
>>>>>>> connection issue I could very easily observe many of crashes in BfA when
>>>>>>> disconnecting HOG during service search session. This is because HOG
>>>>>>> talks directly to gattrib and clear his attrib instance on HOG disconnect
>>>>>>> even there are ongoing GATT operations. Patches 9 - 25 solve this issue.
>>>>>>
>>>>>> I remember applying a patch from Arman that should have fixed the
>>>>>> problem above: shared/att: cancel_all before calling disconnect cb. In
>>>>>> any if that doesn't really solve the problem the client should cancel
>>>>>> their pending operations before releasing its reference which means no
>>>>>> callback should be called after that.
>>>>>
>>>>> This is problem above bt_att, but It might be worth to consider such
>>>>> scenario in shared/gatt.
>>>>> Anyway, In Android attrib is used by HOG and also GATT with all the
>>>>> applications above. GATT controls registered clients so if one client
>>>>> unregister we should not see the problem. But if HOG and GATT have
>>>>> ongoing gatt operations and in this point of time we disconnect HOG,
>>>>> we should not break ongoing gatt operations for GATT (Android apps).
>>>>> Note that link LE link can be still up because of some other GATT
>>>>> application so no disconnect_cb will be called by shared/add. Thats
>>>>> why attrib client should take care for its operations
>>>>
>>>> Yep, but that why I said you should probably cancel any operations,
>>>> which seems you did, but then why you keep checking for disconnected
>>>> in the callbacks? There looks like there another bug going on.
>>>
>>> Canceling operation might not succeed, in that case callback will be
>>> called anyway but unfortunately e.g. hog->attrib will be not valid
>>> anymore and that's we need that additional check.
>>
>> Not succeed? bt_att_cancel should always succeed except if we cannot
>> find the id.
>
> Just checked bt_att code and indeed, after calling cancel we should be
> sure that callback will not be called.
> However, I've seen something else during testing. That might be other
> bug then. Will look into it probably tomorrow.

Yep, it might be another bug or perhaps one operation that was not
canceled for some reason.

--
Luiz Augusto von Dentz

2014-12-10 09:20:23

by Lukasz Rymanowski

[permalink] [raw]
Subject: Re: [PATCH 00/25] android/gatt: Fix HOG connect/disconnect

Hi Luiz,

On 10 December 2014 at 09:56, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Lukasz,
>
> On Wed, Dec 10, 2014 at 10:53 AM, Lukasz Rymanowski
> <[email protected]> wrote:
>> Hi Luiz,
>>
>> On 10 December 2014 at 09:46, Luiz Augusto von Dentz
>> <[email protected]> wrote:
>>> Hi Lukasz,
>>>
>>> On Wed, Dec 10, 2014 at 10:38 AM, Lukasz Rymanowski
>>> <[email protected]> wrote:
>>>> Hi Luiz,
>>>>
>>>> On 10 December 2014 at 08:23, Luiz Augusto von Dentz
>>>> <[email protected]> wrote:
>>>>> Hi Lukasz,
>>>>>
>>>>> On Tue, Dec 9, 2014 at 5:11 PM, Lukasz Rymanowski
>>>>> <[email protected]> wrote:
>>>>>> When testing HOG against PTS we got failing testes if devices were
>>>>>> not bonded before the test.
>>>>>> With those patches this issue is not valid anymore.
>>>>>>
>>>>>> This set address couple of issues:
>>>>>> * Some minor fixes found during work on those patches (first 4 patches)
>>>>>>
>>>>>> * Changed GATT behaviour during dedicated bonding. Now GATT will not start
>>>>>> search services until bonding is done. That fixes PTS connection issue
>>>>>> (patches 5-8)
>>>>>>
>>>>>> * Tracking of GATT operations in hog/dis/bas/scpp. After I solved PTS
>>>>>> connection issue I could very easily observe many of crashes in BfA when
>>>>>> disconnecting HOG during service search session. This is because HOG
>>>>>> talks directly to gattrib and clear his attrib instance on HOG disconnect
>>>>>> even there are ongoing GATT operations. Patches 9 - 25 solve this issue.
>>>>>
>>>>> I remember applying a patch from Arman that should have fixed the
>>>>> problem above: shared/att: cancel_all before calling disconnect cb. In
>>>>> any if that doesn't really solve the problem the client should cancel
>>>>> their pending operations before releasing its reference which means no
>>>>> callback should be called after that.
>>>>
>>>> This is problem above bt_att, but It might be worth to consider such
>>>> scenario in shared/gatt.
>>>> Anyway, In Android attrib is used by HOG and also GATT with all the
>>>> applications above. GATT controls registered clients so if one client
>>>> unregister we should not see the problem. But if HOG and GATT have
>>>> ongoing gatt operations and in this point of time we disconnect HOG,
>>>> we should not break ongoing gatt operations for GATT (Android apps).
>>>> Note that link LE link can be still up because of some other GATT
>>>> application so no disconnect_cb will be called by shared/add. Thats
>>>> why attrib client should take care for its operations
>>>
>>> Yep, but that why I said you should probably cancel any operations,
>>> which seems you did, but then why you keep checking for disconnected
>>> in the callbacks? There looks like there another bug going on.
>>
>> Canceling operation might not succeed, in that case callback will be
>> called anyway but unfortunately e.g. hog->attrib will be not valid
>> anymore and that's we need that additional check.
>
> Not succeed? bt_att_cancel should always succeed except if we cannot
> find the id.

Just checked bt_att code and indeed, after calling cancel we should be
sure that callback will not be called.
However, I've seen something else during testing. That might be other
bug then. Will look into it probably tomorrow.

\Lukasz

2014-12-10 08:56:25

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 00/25] android/gatt: Fix HOG connect/disconnect

Hi Lukasz,

On Wed, Dec 10, 2014 at 10:53 AM, Lukasz Rymanowski
<[email protected]> wrote:
> Hi Luiz,
>
> On 10 December 2014 at 09:46, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Lukasz,
>>
>> On Wed, Dec 10, 2014 at 10:38 AM, Lukasz Rymanowski
>> <[email protected]> wrote:
>>> Hi Luiz,
>>>
>>> On 10 December 2014 at 08:23, Luiz Augusto von Dentz
>>> <[email protected]> wrote:
>>>> Hi Lukasz,
>>>>
>>>> On Tue, Dec 9, 2014 at 5:11 PM, Lukasz Rymanowski
>>>> <[email protected]> wrote:
>>>>> When testing HOG against PTS we got failing testes if devices were
>>>>> not bonded before the test.
>>>>> With those patches this issue is not valid anymore.
>>>>>
>>>>> This set address couple of issues:
>>>>> * Some minor fixes found during work on those patches (first 4 patches)
>>>>>
>>>>> * Changed GATT behaviour during dedicated bonding. Now GATT will not start
>>>>> search services until bonding is done. That fixes PTS connection issue
>>>>> (patches 5-8)
>>>>>
>>>>> * Tracking of GATT operations in hog/dis/bas/scpp. After I solved PTS
>>>>> connection issue I could very easily observe many of crashes in BfA when
>>>>> disconnecting HOG during service search session. This is because HOG
>>>>> talks directly to gattrib and clear his attrib instance on HOG disconnect
>>>>> even there are ongoing GATT operations. Patches 9 - 25 solve this issue.
>>>>
>>>> I remember applying a patch from Arman that should have fixed the
>>>> problem above: shared/att: cancel_all before calling disconnect cb. In
>>>> any if that doesn't really solve the problem the client should cancel
>>>> their pending operations before releasing its reference which means no
>>>> callback should be called after that.
>>>
>>> This is problem above bt_att, but It might be worth to consider such
>>> scenario in shared/gatt.
>>> Anyway, In Android attrib is used by HOG and also GATT with all the
>>> applications above. GATT controls registered clients so if one client
>>> unregister we should not see the problem. But if HOG and GATT have
>>> ongoing gatt operations and in this point of time we disconnect HOG,
>>> we should not break ongoing gatt operations for GATT (Android apps).
>>> Note that link LE link can be still up because of some other GATT
>>> application so no disconnect_cb will be called by shared/add. Thats
>>> why attrib client should take care for its operations
>>
>> Yep, but that why I said you should probably cancel any operations,
>> which seems you did, but then why you keep checking for disconnected
>> in the callbacks? There looks like there another bug going on.
>
> Canceling operation might not succeed, in that case callback will be
> called anyway but unfortunately e.g. hog->attrib will be not valid
> anymore and that's we need that additional check.

Not succeed? bt_att_cancel should always succeed except if we cannot
find the id.