Re: Generic Attribute API race condition

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

 



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.

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 ?

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


[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