Re: Generic Attribute API race condition

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Claudio,

> Hi Jaikumar
>
> On Wed, Jun 8, 2011 at 11:06 PM, Jaikumar Ganesh <jaikumarg@xxxxxxxxx>
> wrote:
>> Hey Claudio:
>>
>> On Thu, Jun 9, 2011 at 2:44 AM, Claudio Takahasi
>> <claudio.takahasi@xxxxxxxxxxxxx> wrote:
>>> Hi Jaikumar,
>>>
>>> On Wed, Jun 8, 2011 at 11:18 AM, Jaikumar Ganesh <jaikumarg@xxxxxxxxx>
>>> wrote:
>>>> Hi Claudio:
>>>>
>>>> On Tue, Jun 7, 2011 at 11:41 PM, Claudio Takahasi
>>>> <claudio.takahasi@xxxxxxxxxxxxx> wrote:
>>>>> Hi All,
>>>>>
>>>>> There is a potential race condition in the Generic Attribute API. I'd
>>>>> like to collect opinions to avoid re-work later.
>>>>>
>>>>> How to reproduce:
>>>>> - call CreateDevice() for a device which exports a GATT based
>>>>> service:
>>>>> CreateDevice will discover all primary services
>>>>>  and the Generic API will register the object paths for all found
>>>>> services
>>>>> - Call DiscoverCharacteristics() of the primary service followed by
>>>>> Characteristic GetProperties(), expected result: read all
>>>>> characteristics properties outcome: partial characteristics
>>>>> properties
>>>>> may appear.
>>>>>
>>>>> This problem happens because DiscoverCharacteristics() doesn't wait
>>>>> for all characteristic VALUES, it returns after discovering all
>>>>> characteristic DECLARATIONS.
>>>>> Wait for characteristic values is a problem, attributes may require
>>>>> authentication or authorization.
>>>>>
>>>>> Suggestions are:
>>>>> 1. Report discovered characteristic values using PropertyChanged:
>>>>> Returning empty properties values (for GetProperties) if the
>>>>> discovery
>>>>> is in progress
>>>>>
>>>>> 2. Block GetProperties() if there is a discovery pending: returning
>>>>> after discovery finishes. Timeout can still happen!
>>>>>
>>>>> 3. Return Busy for GetProperties() if there is a characteristic
>>>>> discovery in progress
>>>>>
>>>>> Comments?
>>>>
>>>> I don't think we should read characteristic value till the application
>>>> asks for it.
>>> Are you suggesting to trigger the characteristic value and descriptors
>>> read when Characteristic GetProperties() is called and block it?
>>> Remember that GATT timeout is 30seconds and the default D-Bus message
>>> reply timeout is smaller.
>>>
>>>> DiscoverCharacteristics() should just return after getting the
>>>> DECLARATIONS.
>>> This is current behaviour.
>>
>> Sorry misread your email a bit.
>>
>> I assume you are referring to the Characteristic Watcher when you are
>> talking about the PropertyChanged signal,
>> since there is no explicit PropertyChanged signal.
>>
>> I don't think we should block GetProperties. But then if we are
>> returning null and depending on PropertyChanged signal when we have to
>> read the values, clients will need to wait for it after calling
>> GetProperties. We need to guarantee that this signal will be sent
>> (with a timeout, unless we want the clients to have the timeout at
>> their end) even if we are unable to get the value - because of some
>> kind of error.
>
> Signal with a timeout? I don't think that it is necessary.
> We can use persistent storage returning "old" values on GetProperties
> calls and use PropertyChanged to notify "new" values asynchronously.
> This approach should be enough.

Just noticed this thread.
I ran into a similar problem. Cannot we just delay the return of
DiscoverCharacteristics method call until after the characteristic
values/properties have been acquired? I tried out this solution and it
seems to work fine. The trick is to implement internal counter for
received responses from the remote device + timeout that is reset on each
successful response. Any thoughts on this?

Also, "Property Changed" on characteristic obj path/interface signal is
mentioned in the attribute API doc, but I cannot seem to find
implementation for the signal in the code. Am I missing something?

>
>>
>> In all other places we use PropertyChanged signal when the property
>> value actually changes, whereas here we are tying it with the reading
>> of the properties.
>>
>> How about providing separate calls for read and writing values and
>> decoupling it from the Properties ?
>
> I don't think it will help, authorization and GATT timeout is still a
> problem.
> We are trying to cover unusual scenarios, IMO change the API to cover
> these cases doesn't look correct.
>
> BR,
> Claudio.
>
>>
>> Thanks
>>
>>>
>>>> A client might not be interested in all the values and thus we should
>>>> read them only
>>>> when needed by the client. This will save both time and power and
>>>> solve this problem too.
>>>
>>> I don't think it is critical, we are not reading them on all
>>> connections/re-connections and the profiles available at the moment
>>> don't expose "complex" characteristics.
>>
>> I am talking about GATT clients written using the attribute DBUS apis.
>> Accessory vendors
>> want to get started on LE and not wait till all the profiles are
>> approved the SIG and developed
>> and they want to develop things based on the GATT clients APIs.
>>
>>>
>>> BR,
>>> Claudio
>>>
>>>>
>>>> Thanks
>>>> Jaikumar
>>>>>
>>>>> BR,
>>>>> Claudio
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Thanks,

Inga Stotland
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum



--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux