luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2024-12-28 08:26:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

spawn: prefer allocating buffers on stack for small sizes to avoid valgrind leaks

Browse Source 
We preallocate buffers that are used after forked. That is because
malloc()/free() are not async-signal-safe and must not be used between
fork() and exec().

However, for the child process that exits without fork, valgrind wrongly
reports these buffers as leaked.
That can be suppressed with "--child-silent-after-fork=yes", but it is
cumbersome.

Work around by trying to allocate the buffers on the stack. At
least in the common cases where the pointers are small enough
so that we can reasonably do that.

If the buffers happen to be large, we still allocate them on the heap
and the problem still happens. Maybe we could have also allocated them
as thread_local, but currently glib doesn't use that.

[smcv: Cosmetic adjustments to address review comments from pwithnall]
This commit is contained in:
Thomas Haller 2021-01-29 12:26:00 +01:00 committed by Simon McVittie
parent 82adfd7e3a
commit e864c6577a
1 changed files with 30 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
glib/gspawn.c
View File

@ -1769,8 +1769,10 @@ fork_exec_with_fds (gboolean intermediate_child,
gint status; gint status;
const gchar *chosen_search_path; const gchar *chosen_search_path;
gchar *search_path_buffer = NULL; gchar *search_path_buffer = NULL;
gchar *search_path_buffer_heap = NULL;
gsize search_path_buffer_len = 0; gsize search_path_buffer_len = 0;
gchar **argv_buffer = NULL; gchar **argv_buffer = NULL;
gchar **argv_buffer_heap = NULL;
gsize argv_buffer_len = 0; gsize argv_buffer_len = 0;
#ifdef POSIX_SPAWN_AVAILABLE #ifdef POSIX_SPAWN_AVAILABLE
@ -1864,7 +1866,17 @@ fork_exec_with_fds (gboolean intermediate_child,
if (chosen_search_path != NULL) if (chosen_search_path != NULL)
{ {
search_path_buffer_len = strlen (chosen_search_path) + strlen (argv[0]) + 2; search_path_buffer_len = strlen (chosen_search_path) + strlen (argv[0]) + 2;
search_path_buffer = g_malloc (search_path_buffer_len); if (search_path_buffer_len < 4000)
{
/* Prefer small stack allocations to avoid valgrind leak warnings
* in forked child. The 4000B cutoff is arbitrary. */
search_path_buffer = g_alloca (search_path_buffer_len);
}
else
{
search_path_buffer_heap = g_malloc (search_path_buffer_len);
search_path_buffer = search_path_buffer_heap;
}
} }
if (search_path || search_path_from_envp) if (search_path || search_path_from_envp)
@ -1876,12 +1888,22 @@ fork_exec_with_fds (gboolean intermediate_child,
* script_execute() has to be called later on, it can build a wrapper argv * script_execute() has to be called later on, it can build a wrapper argv
* array in this buffer. */ * array in this buffer. */
argv_buffer_len = g_strv_length (argv) + 2; argv_buffer_len = g_strv_length (argv) + 2;
argv_buffer = g_new (gchar *, argv_buffer_len); if (argv_buffer_len < 4000 / sizeof (gchar *))
{
/* Prefer small stack allocations to avoid valgrind leak warnings
* in forked child. The 4000B cutoff is arbitrary. */
argv_buffer = g_newa (gchar *, argv_buffer_len);
}
else
{
argv_buffer_heap = g_new (gchar *, argv_buffer_len);
argv_buffer = argv_buffer_heap;
}
if (!g_unix_open_pipe (child_err_report_pipe, pipe_flags, error)) if (!g_unix_open_pipe (child_err_report_pipe, pipe_flags, error))
{ {
g_free (search_path_buffer); g_free (search_path_buffer_heap);
g_free (argv_buffer); g_free (argv_buffer_heap);
return FALSE; return FALSE;
} }
@ -2128,8 +2150,8 @@ fork_exec_with_fds (gboolean intermediate_child,
close_and_invalidate (&child_err_report_pipe[0]); close_and_invalidate (&child_err_report_pipe[0]);
close_and_invalidate (&child_pid_report_pipe[0]); close_and_invalidate (&child_pid_report_pipe[0]);
g_free (search_path_buffer); g_free (search_path_buffer_heap);
g_free (argv_buffer); g_free (argv_buffer_heap);
if (child_pid) if (child_pid)
*child_pid = pid; *child_pid = pid;
@ -2163,8 +2185,8 @@ fork_exec_with_fds (gboolean intermediate_child,
close_and_invalidate (&child_pid_report_pipe[0]); close_and_invalidate (&child_pid_report_pipe[0]);
close_and_invalidate (&child_pid_report_pipe[1]); close_and_invalidate (&child_pid_report_pipe[1]);
g_free (search_path_buffer); g_free (search_path_buffer_heap);
g_free (argv_buffer); g_free (argv_buffer_heap);
return FALSE; return FALSE;
} }