Re: g_spawn_async_with_pipes is not thread safe

[Chris Vine <chris cvine freeserve co uk>]

On Fri, 3 Jun 2016 10:46:30 +0300
Andrejs Hanins <andrejs hanins ubnt com> wrote:
>     I've discovered a subtle, but quite a serious issue with
> g_spawn_async_with_pipes API. The problem is that under some
> circumstances this API causes memory allocation operations to be
> called from the forked child process, which is illegal in
> multi-threaded environment an may lead to undefined behaviour (I've
> seen segfaults and infinite loops in malloc in the child). It happens
> at least in the following situations:
>
> 1. If SPAWN_LEAVE_DESCRIPTORS_OPEN flag is _not_ given, which is
> often default option, then child (at least on Linux) does the
> following call chain fork_exec_with_pipes->do_exec->fdwalk which uses
> opendir which in turn uses memory allocation for DIR structure. Some
> API, like g_spawn_command_line_sync, doesn't allow to specify flags
> at all.
>
> 2. If script or relative path command is given to execute in a child,
> then g_execute also uses g_malloc as seen from the source code. This
> can't be avoided by flags at all.
>
> 3. If child_setup is given by user which also may do some memory
> operations.
>
> This risky behaviour is not described in the docs and what makes the
> situation even worse is that although the application from user point
> of view may be single-threaded, the usage of, for example, DBus API
> implicitly creates an additional dbus thread which does memory
> allocations, so the application effectively becomes multi-threaded
> under the hood.
>
> As I understand, this is hard (maybe even impossible) to fix in a
> generic way, but at least docs should explicitly say that this API is
> not thread safe if SPAWN_LEAVE_DESCRIPTORS_OPEN is not given or
> script/relative command is given to be executed or schild_setup uses
> some mallocs.
>
> I have worked around this bug in my code by using g_spawn_sync with
> absolute path to a command and setting also
> SPAWN_LEAVE_DESCRIPTORS_OPEN flag. My code is glibmm based, but it
> shouldn't matter at all, because glibmm is a very thin wrapper around
> glib APIs in this case.

There is a (closed) bug report on this at
https://bugzilla.gnome.org/show_bug.cgi?id=746604

The short answer given given in relation to that bug report is that you
should be OK if you are using glibc. But any code linking to any other
allocator is not going to be OK.

Having said that, the glib documentation is somewhat contradictory.  The
g_spawn* functions ignore their own warning about GSpawnChildSetupFunc:
"However, even on POSIX, you are extremely limited in what you can
safely do from a GSpawnChildSetupFunc, because any mutexes that were
held by other threads in the parent process at the time of the fork()
will still be locked in the child process, and they will never be
unlocked (since the threads that held them don't exist in the child).
POSIX allows only async-signal-safe functions (see signal(7)) to be
called in the child between fork() and exec(), which drastically limits
the usefulness of child setup functions".  This presumably needs to be
qualified so that it does not apply to allocation using glibc's
malloc().

Chris