acpid hangs with full cpu usage when removing USB input devices

Natanael Copa natanael.copa at gmail.com
Mon Jan 9 15:58:10 UTC 2012


Hi,

On Mon, Jan 9, 2012 at 5:02 AM, Denys Vlasenko <vda.linux at googlemail.com> wrote:
> On Friday 06 January 2012 14:52, Natanael Copa wrote:
>> Looking a bit at the code, it appears that it never checks for POLLERR
>> or POLLHUP and thus enters an endless loop.
>>
>> Doh... it just hit me.
>>
>> What happens is, I have an USB keyboard and USB mouse attached to the
>> screen. When I power off the screen the keyboard and mouse input
>> devices disappear and the file handle in acpid becomes invalid. Since
>> nothing checks for POLLERR|POLLHUP it ends up in an endless loop since
>> POLLIN was not set.
>
> This is weird, because usually POLLIN would be set too in such a case -
> since read() would usually be possible (in a sense that it would not block,
> but return EOF or error _immediately_), and therefore poll must indicate that.
>
> Oh well...
>
>> I wonder what the proper way to handle this is. Probably do some
>> netlink stuff to detect new devices for acpid?
>
> Another solution is just to restart acpid.
> Best to do it automatically from hotplug helper...

That'd be a simpler solution yes, and more than good enough.

>> Quick workaround would be to just close fd on POLLHUP or POLLERR:
>
>> diff --git a/util-linux/acpid.c b/util-linux/acpid.c
>> index 6e7321b..3d723c0 100644
>> --- a/util-linux/acpid.c
>> +++ b/util-linux/acpid.c
>> @@ -292,6 +292,11 @@ int acpid_main(int argc UNUSED_PARAM, char **argv)
>>                 for (i = 0; i < nfd; i++) {
>>                         const char *event = NULL;
>>
>> +                       if (pfd[i].revents & (POLLERR | POLLHUP)) {
>> +                               close(pfd[i].fd);
>> +                               pfd[i].fd = -1;
>> +                       }
>> +
>
> I'm a bit surprised this would work: I don't remember any special case
> in poll() to ignore fds < 0, so it _might_ start returning POLLNVAL
> on this pfd[i] from now on.

>From http://pubs.opengroup.org/onlinepubs/009695399/functions/poll.html:

"If the value of fd is less than 0, events shall be ignored, and
revents shall be set to 0 in that entry on return from poll()."

I tested adding a
+                       if (pfd[i].revents & (POLLNVAL))
+                               printf("pollnval\n");

The POLLNVAL never happens after fd is set to -1.

> Does it work if instead of closing pfd[i].fd, you set pfd[i].events = 0?

No. Still hangs with full CPU usage - and I get lots of POLLNVAL.

> A cleaner solution is to shift all pfd[] elements one element down
> starting from [i], and do nfd--. Something like this:
>
>                        if (!(pfd[i].revents & POLLIN)) {
>                                if (pfd[i].revents == 0)
>                                        continue; /* this fd has nothing */
>
>                                /* Likely POLLERR, POLLHUP, POLLNVAL.
>                                 * Do not listen on this fd anymore.
>                                 */
>                                close(pfd[i].fd);
>                                nfd--;
>                                for (; i < nfd; i++)
>                                        pfd[i].fd = pfd[i + 1].fd;
>                                break; /* do poll() again */
>                        }

I think the "if less than zero" thingy is there so you don't need to
the shifting.

> With your fix, what happens when you power your screen back on?

CPU goes back to normal and everything seems to work as it did ...

> I guess the keyboard and mouse ACPI events are no longer detected by acpid,
> right?

... except for the keyboard and mouse. I havent really checked but
yes, I assume it stops to detect mouse and keyboard events since those
fd's are closed and never reopened.

fork/exec'ing itself again would solve that. Do you have other applets
that re-start themselves? I could send a new patch - including a
comment with a reference for the ignore fd == -1 thingy. Or should we
just exit the acpid and let a hook script restart it?

Maybe could use re_exec() to do it together with CLOEXEC?

I am pretty sure you can reproduce it yourself with an USB keybaord
connected when starting up acpid and unplug it afterwards.

> --
> vda

Thanks!

-- 
Natanael Copa


More information about the busybox mailing list