On Thu, Mar 3, 2011 at 2:35 AM, Peter Hutterer <peter.hutte...@who-t.net>wrote:
> On 3/03/11 17:07 , Ping Cheng wrote:
>
>> On Wed, Mar 2, 2011 at 10:06 PM, Peter Hutterer
>> <peter.hutte...@who-t.net <mailto:peter.hutte...@who-t.net>> wrote:
>>
>> There is a small time window where a device may try to send an event
>> even
>> though it is not fully setup to send events yet, causing a server
>> crash.
>>
>> This window opens when the tool is added to the list of devices in
>> wcmParseOptions() and closes with the server calling
>> xf86ActivateDevice().
>> If an event for a dependent device is processed during that time,
>> the tool
>> will be available but the device pointer is still invalid.
>>
>> Crash can be reproduced by putting a breakpoint after
>> wcmParseOptions() for
>> the eraser, then generating events with the eraser. These will cause
>> the
>> tool to dereference tool->device->dev, which is uninitialized.
>>
>> Work around this with a simple "enabled" flag that is set whenever
>> the tool
>> is actually enabled.
>>
>> I didn't test anything yet. As I've shared with you before, I don't
>> think a local enabled flag is enough. Something common to all the tools
>> associated with the same port is needed since we should not access the
>> fd before all tools are ready.
>> + if (!tool->enabled) {
>> need to be changed to something like
>> + if (!common->enabled) {
>> And commmon->enabled is set to true oly when the last hotplugged tool is
>> enabled. It will be set to false as soon as one tool is disabled.
>> Let me know if I've lost you.
>>
>
> is this just a hunch or have you found some other race condition?
>
It's from testing result. I didn't get what exactly patch 2/4 is doing yet.
You might have fixed the issue by that patch. Plus, the testing was using
linuxwacom instead of xf86-input-wacom. So, maybe the testing result does
not apply here any more.
But, it won't hurt to repeat the result to see if we can get anything out of
it or not.
The testing was based on X server 1.6.5, with hot-plugging support based on
HAL.
Adding "if (!pInfo || !pInfo->dev || !pInfo->dev->enabled)" did reduce the
crash rate a lot, from one crash every 2-300 hot plugging to 2500;
But, 2500 isn't an acceptable number for the user. So, we had to figure out
where else was causing the crash.
After tracing into the code, we found guarding at the if-statement was too
late in the game since crashes actually happened when a xf86ReadSerial error
occurred before all tools were added. That tells us we should not access the
fd before all tools are added. So, I added a check in wcmDevReadInput to
make sure all tools are added before getting into the for loop. With this
change, no crashes have showed up so far.
That's why I think checking on individual tool status won't prevent the
above issue happening. The race condition is a combined scenario, which
links all tools on the same port together. Treating one at a time makes
accessing the device before all tools are ready possible. That, at least,
was one of the cause of a race condition.
Ping
------------------------------------------------------------------------------
What You Don't Know About Data Connectivity CAN Hurt You
This paper provides an overview of data connectivity, details
its effect on application quality, and explores various alternative
solutions. http://p.sf.net/sfu/progress-d2d
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
