Libguestfs - Nov 2018 - Re: [collectd] collectd leaks SIGCHLD == SIG_IGN into plugins

Hi,

thank you very much for reporting this! Sounds like a bug in the exec plugin –
it never ceases to amaze me how many issues a single plugin can have ;)
> > > This means that any plugin that does the usual pattern of:
> > > 
> > >   pid = fork ();
Note that the exec plugin is the *only* plugin that does this. All other
plugins are forbidden to fork(), popen() or create new processes in any other
way. The only plugin doing that, the exec plugin, has had enough issues over
the years for me to feel justified in that decision. ;-)

As mentioned before, a Github issue would be appreciated so we can properly
track this problem.

Thanks and best regards,
—octo
-- 
Florian octo Forster
Hacker in training
GnuPG: 0x0C705A15
http://octo.it/

On Tue, Nov 13, 2018 at 12:07:23PM +0100, Florian Forster
wrote:
> A quick glance shows that the exec plugin actually is clearing the signal
mask:
> https://github.com/collectd/collectd/blob/master/src/exec.c#L526
The problem is the SIGCHLD handler being set to SIG_IGN, which is
different from the signal mask.
> Can you give some more context when this problem comes up? Ideally
> in a Github issue (hint, hint ;).
I'd like Peter to file a github issue because he knows the version
that he is using.

However in general terms a trivial plugin which just does:

  #include <guestfs.h>
  ...
  guestfs_h *g;
  if ((g = guestfs_create ()) == NULL || guestfs_launch (g) == -1)
    exit (1);

should be sufficient to demonstrate the problem.  (Work around it by
adding ‘signal (SIGCHLD, SIG_DFL);’ to the plugin code.)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

I was extending the virt plugin.
It already collects similar data (about VMs) using Libvirt's API, but lacks
disk usage information.

I went through some hoops to link libguestfs correctly to collectd.
Is it okay to just include fork(), waitpid() example? It does reproduce the
issue.

Best Regards,
Peter


On Tue, Nov 13, 2018 at 1:34 PM Richard W.M. Jones <rjones@redhat.com>
wrote:
> On Tue, Nov 13, 2018 at 12:00:19PM +0100, Florian Forster wrote:
> > Hi,
> >
> > thank you very much for reporting this! Sounds like a bug in the exec
> plugin –
> > it never ceases to amaze me how many issues a single plugin can have
;)
> >
> > > > > This means that any plugin that does the usual pattern
of:
> > > > >
> > > > >   pid = fork ();
> >
> > Note that the exec plugin is the *only* plugin that does this. All
other
> > plugins are forbidden to fork(), popen() or create new processes in
any
> other
> > way. The only plugin doing that, the exec plugin, has had enough
issues
> over
> > the years for me to feel justified in that decision. ;-)
> >
> > As mentioned before, a Github issue would be appreciated so we can
> properly
> > track this problem.
>
> I don't know if Peter is using the exec plugin or is trying to write
> an ordinary plugin.  However the library he is using (libguestfs)
> certainly does fork subprocess(es).
>
> Rich.
>
> --
> Richard Jones, Virtualization Group, Red Hat
> http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> virt-builder quickly builds VMs from scratch
> http://libguestfs.org/virt-builder.1.html
>