On Wed, Jul 31, 2019 at 05:01:52PM -0500, Eric Blake wrote:
On 7/31/19 4:31 PM, Eric Blake wrote:
> The rate filter is potentially opening fds in one thread while another
> thread is processing a fork() in the plugin. Although the file is not
> open for long, we MUST atomically use CLOEXEC to avoid fd leaks. This
> one is a bit harder to observe using only the sh plugin, because the
> window is small; you'll have better success at catching the leak by
> using gdb or recompiling code to insert strategic sleeps.
In fact, I have to tweak this commit message: you CAN'T observe this one
with the sh plugin unless you recompile it to use #define THREAD_MODEL
NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS, as well as introducing the
timing hacks mentioned above (that's because with our current
SERIALIZE_ALL_REQUESTS, there is never more than one thread in
filter/plugin code at a time).
The current nbdkit-sh-plugin is only SERIALIZE_ALL_REQUESTS in order
to make writing the shell scripts a bit more sane. I believe it could
be fully PARALLEL.
(As an aside: Ideally in future we'll allow the thread model to be
specified by the plugin dynamically. It's one of the things I thought
I had listed in the TODO file - it wasn't there so I've added it now.)
But it does raise an interesting point - if we hit platforms that
are
unable to support atomic CLOEXEC, one possibility is a patch that forces
SERIALIZE_ALL_REQUESTS as the maximum parallelism allowed on that
platform (while remaining at our goal of PARALLEL on more competent
systems) - once we do that, the lacking systems will be serialized to
the point that there is no race window where one thread can fork() while
another is obtaining an fd.
Yup. But probably better to encourage those platforms to support
atomic CLOEXEC everywhere.
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/