Re: [libvirt PATCH 2/2] tests: don't set G_DEBUG=fatal-warnings on macOS
On Thu, Apr 28, 2022 at 06:39:42AM -0700, Andrea Bolognani wrote:
On Thu, Apr 28, 2022 at 12:22:14PM +0100, Daniel P. Berrangé wrote:
> +# On macOS when BROKEN_POLL is set for GLib, our tests will
> +# periodically trigger a warning:
> +#
> +# (process:50880): GLib-WARNING **: 02:54:15.272: poll(2) failed due to: Bad file
descriptor.
> +#
> +# Our code is inherantly racy, calling g_source_destroy which
> +# removes the FD from the event thread poll asynchronously
> +# but we close the FD immediately after g_source_destroy returns.
> +#
> +# With poll() this results in POLLNVAL, but with select() it
> +# generates the BADF error on macOS
> +if host_machine.system() != 'darwin'
> + tests_env += ['G_DEBUG=fatal-warnings']
> +endif
Doesn't this just sweep the issue under the carpet? Shouldn't GLib be
changed to do something like
#ifndef BROKEN_POLL
if (poll() < 0)
if (!POLLNVAL)
return -1;
That's not the way POLLNVAL works - this is an even that is set in
each 'struct pollfd', so it is associated with individual file
descriptors. The application code can thus receive and handle the
G_IO_NVAL condition - it isn't fatal from POV of g_poll
#else
if (select() < 0)
if (!EBADF)
return -1;
#endif
instead? If acting on an fd that's already been closed is okay when
using the poll()-based implementation, the same should apply to the
select()-based one as well.
The only way to get the same semantics for select() would be to see
EBADF and then iterate over struct pollfd, calling fcntl() to see
which one(s) are BADF. Possible, but its unpleasantly non-scalable,
so I imagine that's why they didn't do this.
I'll still probably inclined to file a bug against glib, if nothing
else to provide a bug that other people can discover to explain why
the behaviour is different.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|