On Wed, 2011-12-28 at 14:29 +0200, Tanu Kaskinen wrote:
> I don't see any reason why POLLNVAL should be ignored like
> it is now without this patch. PA_IO_EVENT_ERROR seems to be
> the best choice for reporting the IO event type, because
> POLLNVAL is an error condition and adding a new IO event
> type just for POLLNVAL probably wouldn't add any value.
It gives more information about what went wrong, and I don't see the
gain of throwing away this information (after looking at the code,
adding a PA_IO_EVENT_INVALID to the places where PA_IO_EVENT_ERROR is
handled should be relatively easy).
> This patch seems to have made one hard to reproduce bug
> impossible to reproduce. The problem was that a certain
> client application (involving GStreamer) occasionally
> started to consume 100% of CPU. Based on some stack traces
> it was speculated that the threaded mainloop of libpulse
> was spinning on POLLNVAL, because some user of the mainloop
> (maybe GStreamer) kept the IO event active when the IO
> event was triggered with zero flags. But that's just
> speculation - there's no direct evidence of POLLNVAL being
> triggered at all, only that with this patch the application
> has been behaving nicely so far.
> ---
> src/pulse/mainloop.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/src/pulse/mainloop.c b/src/pulse/mainloop.c
> index 5c0345e..a27cc55 100644
> --- a/src/pulse/mainloop.c
> +++ b/src/pulse/mainloop.c
> @@ -144,6 +144,7 @@ static pa_io_event_flags_t map_flags_from_libc(short flags) {
> (flags & POLLIN ? PA_IO_EVENT_INPUT : 0) |
> (flags & POLLOUT ? PA_IO_EVENT_OUTPUT : 0) |
> (flags & POLLERR ? PA_IO_EVENT_ERROR : 0) |
> + (flags & POLLNVAL ? PA_IO_EVENT_ERROR : 0) |
> (flags & POLLHUP ? PA_IO_EVENT_HANGUP : 0);
> }
Regardless of the conclusion of the discussion above, the corresponding
changes should go into the glib-mainloop code as well.
-- Arun
