[PATCH] inputthread: Fix inputthread not listening if a fd gets re-added immediately after removal

[Hans de Goede]

Hi,

On 10/09/2016 11:52 PM, Mihail Konev wrote:
> Hello.
>
> On Wed,  5 Oct 2016 14:45:20 +0200, Hans de Goede wrote:
>> When a fd is removed dev->state gets set to device_state_removed,
>> if the fd then gets re-added before InputThreadDoWork() deals with
>> the removal, the InputThreadDevice struct gets reused, but its
>> state would stay device_state_removed, so it would still get removed
>> on the first InputThreadDoWork() run, resulting in a non functioning
>> input device.
>>
>> This commit fixes this by (re-)setting dev->state to device_state_running
>> when a InputThreadDevice struct gets reused.
>>
>> This fixes input devices sometimes no longer working after a vt-switch.
>>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>>  os/inputthread.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/os/inputthread.c b/os/inputthread.c
>> index 6aa0a9c..ab1559f 100644
>> --- a/os/inputthread.c
>> +++ b/os/inputthread.c
>> @@ -206,6 +206,8 @@ InputThreadRegisterDev(int fd,
>>      if (dev) {
>>          dev->readInputProc = readInputProc;
>>          dev->readInputArgs = readInputArgs;
>> +        /* Override possible unhandled state == device_state_removed */
>> +        dev->state = device_state_running;
>>      } else {
>>          dev = calloc(1, sizeof(InputThreadDevice));
>>          if (dev == NULL) {
>> --
>> 2.9.3
>>
>
> On 6 Oct 2016, Hans de Goede wrote:
>> On 24-09-16 19:55, Mihail Konev wrote:
>>> <..>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97880
>>
>> Thank you for the patch and you're right that there is an issue here.
>>
>> I've posted a much simpler fix yesterday, and that has been
>> favorably reviewed, so I think we're going to go with that fix
>> instead, <..>
>>
>
> Applied on top of 9e00cf0f75f286ec0e8137d91ba80ef7ba72ab2a , the patch does not
> solve the bug #97880 for me.
>
> Note: ./configure --disable-glamor is broken somewhere after the ade315386
> (Require xproto 7.0.31), so be sure to --enable-glamor.
>
> On 9 Oct 2016, Hans de Goede wrote:
>> So you've tried my patch, with your patch reverted and it
>> does not work for you? Strange as I was hitting the exact
>> same problem and it did work for me.
>>
>
> The difference in problem is, however, that in my case result is
> consistent - keyboard either works or not all the time, not "sometimes".
> Mouse is moving alright.
>
> First, patch was applied on top of fc1c358b9 (1.19 RC1).
> It did not work, so log1.diff was made, applied,
> and log1 produced (both attached).
> Note: log1 is (fully) commented inline.
>
> Rebasing on top of 97a8353ec (Fix id in error) did not help.
>
> At this point (log1), nol_e.diff gives:
> - nol_e.log1
> - bug being present
>
> Applying nol_d.diff to log1 gives:
> - nol_d.log1
> - bug being gone
>
> Manually applied log1.diff to master 97a8353ec (gave log1m.diff).
>
> At this point (log1m) nol_e.diff gives:
> - nol_e.log1m
> - bug being present
> Mistake in bugzilla bug description:
> this shows that not locking EnableDevice is not a sufficient workaround.
>
> Applying nol_d.diff to log1m gives:
> - nol_d.log1m
> - bug being gone
> This is what is right in bugzilla description:
> not locking DisableDevice is a sufficient workaround.
>
> As I see it,
>
> The patch assumes no device state to be maintained except in the input thread dev
> list.

No, the patch assumes that the only device state which gets delayed in processing
because InputThreadDoWork() not running in time is the input thread dev list and
this is correct, all InputThreadDoWork() does on device_state_removed is remove
the fd from the pollfd list, and del the node from the inputhread dev list.

My patch simply makes it correctly re-use the dev node on the list, instead
of removing it even though the device has been re-added.

Hmm, maybe the problem in your case is that the fd is the same before / after
device remove / (re-)add but has been closed, so it needs to be removed from
the poll list and re-added.

I've attached another patch (on top of my previous one) for you to try,
I've good hope that this will fix your issue.

Regards,

Hans

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-inputthread-Re-add-fd-to-the-inputThreadInfo-fds-pol.patch
Type: text/x-patch
Size: 2532 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20161011/280aaa51/attachment.bin>