Re: [Qemu-devel] [PATCH 2/2] qemu-ga: Add the guest-suspend command

[Eric Blake]

On 01/16/2012 03:51 AM, Jamie Lokier wrote:
> 
> Since you mention async-signal-safety, execlp() isn't
> async-signal-safe!  Last time I checked, in Glibc execlp() could call
> malloc().  Also reading PATH looks at the environment, which isn't
> always thread-safe either, depending on what else is going on.

That's why I questioned the use of a PATH lookup in the child.  Doing a
PATH lookup in the parent, then using an absolute name in the execle()
of the child, is indeed better.

> 
> I'm not sure if it's relevant to the this code, but on Glibc fork() is
> not async-signal-safe and has been known to crash in signal handlers.
> This is why fork() was removed from SUS async-signal-safe functions.

fork() is still in the list of async-signal-safe functions [1]; it was
only pthread_atfork() which was removed.  That is, fork() is _required_
to be async-signal-safe (and usable from signal handlers), provided that
the actions following the fork also follow safety rules.

[1]
http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03

> 
>> I didn't check whether slog() is async-signal safe (probably not, since
>> even snprintf() is not async-signal safe, and you are passing a printf
>> style format string).  But strerror() is not, so you shouldn't be using
>> it in the child if qemu-ga is multithreaded.
> 
> In general, why is multithreadedness relevant to async-signal-safety here?

Because POSIX 2008 (SUS inherits from POSIX, so it has the same
restriction) states that if a multithreaded app calls fork, the child
can only portably use async-signal-safe functions up until a successful
exec or _exit.  Even though the child is _not_ operating in a signal
handler context, it _is_ operating in a context of a single thread where
other threads from the parent may have been holding locks, and thus
calling any unsafe function (that is, any function that tries to obtain
a lock) may deadlock.

I don't know if qemu-ga is intended to be a multi-threaded app, so I
don't know if being paranoid about async-signal-safety matters in this
particular case, but I _do_ know that libvirt has encountered issues
with using non-safe functions prior to exec, which is why it always
raises red flags when I see unsafe code between fork and exec.

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature