GThreadPool: Always use the thread-spawning thread for the global shared thread pool

Setting the main thread's scheduler settings is not reliably possible,
especially not if SELinux or similar mechanisms are used to limit what
can be done.

As such, get rid of all the complicated code that tried to do this
better and use a separate thread for spawning threads for the global
shared thread pool. These will always inherit the priority of the main
thread (or rather the thread that created the first shared thread pool).

Fixes https://gitlab.gnome.org/GNOME/glib/-/issues/2769
This commit is contained in:
Sebastian Dröge 2023-01-17 18:40:01 +02:00
parent 438006899e
commit 4d2e77a554
7 changed files with 40 additions and 211 deletions

View File

@ -376,7 +376,7 @@ g_thread_create_full (GThreadFunc func,
GThread *thread;
thread = g_thread_new_internal (NULL, g_deprecated_thread_proxy,
func, data, stack_size, NULL, error);
func, data, stack_size, error);
if (thread && !joinable)
{

View File

@ -1157,9 +1157,6 @@ typedef struct
GMutex lock;
void *(*proxy) (void *);
/* Must be statically allocated and valid forever */
const GThreadSchedulerSettings *scheduler_settings;
} GThreadPosix;
void
@ -1175,103 +1172,9 @@ g_system_thread_free (GRealThread *thread)
g_slice_free (GThreadPosix, pt);
}
gboolean
g_system_thread_get_scheduler_settings (GThreadSchedulerSettings *scheduler_settings)
{
/* FIXME: Implement the same for macOS and the BSDs so it doesn't go through
* the fallback code using an additional thread. */
#if defined(HAVE_SYS_SCHED_GETATTR)
pid_t tid;
int res;
/* FIXME: The struct definition does not seem to be possible to pull in
* via any of the normal system headers and it's only declared in the
* kernel headers. That's why we hardcode 56 here right now. */
guint size = 56; /* Size as of Linux 5.3.9 */
guint flags = 0;
tid = (pid_t) syscall (SYS_gettid);
scheduler_settings->attr = g_malloc0 (size);
do
{
int errsv;
res = syscall (SYS_sched_getattr, tid, scheduler_settings->attr, size, flags);
errsv = errno;
if (res == -1)
{
if (errsv == EAGAIN)
{
continue;
}
else if (errsv == E2BIG)
{
g_assert (size < G_MAXINT);
size *= 2;
scheduler_settings->attr = g_realloc (scheduler_settings->attr, size);
/* Needs to be zero-initialized */
memset (scheduler_settings->attr, 0, size);
}
else
{
g_debug ("Failed to get thread scheduler attributes: %s", g_strerror (errsv));
g_free (scheduler_settings->attr);
return FALSE;
}
}
}
while (res == -1);
/* Try setting them on the current thread to see if any system policies are
* in place that would disallow doing so */
res = syscall (SYS_sched_setattr, tid, scheduler_settings->attr, flags);
if (res == -1)
{
int errsv = errno;
g_debug ("Failed to set thread scheduler attributes: %s", g_strerror (errsv));
g_free (scheduler_settings->attr);
return FALSE;
}
return TRUE;
#else
return FALSE;
#endif
}
#if defined(HAVE_SYS_SCHED_GETATTR)
static void *
linux_pthread_proxy (void *data)
{
GThreadPosix *thread = data;
/* Set scheduler settings first if requested */
if (thread->scheduler_settings)
{
pid_t tid = 0;
guint flags = 0;
int res;
int errsv;
tid = (pid_t) syscall (SYS_gettid);
res = syscall (SYS_sched_setattr, tid, thread->scheduler_settings->attr, flags);
errsv = errno;
if (res == -1)
g_error ("Failed to set scheduler settings: %s", g_strerror (errsv));
}
return thread->proxy (data);
}
#endif
GRealThread *
g_system_thread_new (GThreadFunc proxy,
gulong stack_size,
const GThreadSchedulerSettings *scheduler_settings,
const char *name,
GThreadFunc func,
gpointer data,
@ -1290,7 +1193,6 @@ g_system_thread_new (GThreadFunc proxy,
base_thread->thread.func = func;
base_thread->thread.data = data;
base_thread->name = g_strdup (name);
thread->scheduler_settings = scheduler_settings;
thread->proxy = proxy;
posix_check_cmd (pthread_attr_init (&attr));
@ -1310,18 +1212,13 @@ g_system_thread_new (GThreadFunc proxy,
#endif /* HAVE_PTHREAD_ATTR_SETSTACKSIZE */
#ifdef HAVE_PTHREAD_ATTR_SETINHERITSCHED
if (!scheduler_settings)
{
/* While this is the default, better be explicit about it */
pthread_attr_setinheritsched (&attr, PTHREAD_INHERIT_SCHED);
}
#endif /* HAVE_PTHREAD_ATTR_SETINHERITSCHED */
#if defined(HAVE_SYS_SCHED_GETATTR)
ret = pthread_create (&thread->system_thread, &attr, linux_pthread_proxy, thread);
#else
ret = pthread_create (&thread->system_thread, &attr, (void* (*)(void*))proxy, thread);
#endif
posix_check_cmd (pthread_attr_destroy (&attr));

View File

@ -463,19 +463,9 @@ g_thread_win32_proxy (gpointer data)
return 0;
}
gboolean
g_system_thread_get_scheduler_settings (GThreadSchedulerSettings *scheduler_settings)
{
HANDLE current_thread = GetCurrentThread ();
scheduler_settings->thread_prio = GetThreadPriority (current_thread);
return TRUE;
}
GRealThread *
g_system_thread_new (GThreadFunc proxy,
gulong stack_size,
const GThreadSchedulerSettings *scheduler_settings,
const char *name,
GThreadFunc func,
gpointer data,
@ -515,16 +505,10 @@ g_system_thread_new (GThreadFunc proxy,
* On Windows, by default all new threads are created with NORMAL thread
* priority.
*/
if (scheduler_settings)
{
thread_prio = scheduler_settings->thread_prio;
}
else
{
HANDLE current_thread = GetCurrentThread ();
thread_prio = GetThreadPriority (current_thread);
}
{
HANDLE current_thread = GetCurrentThread ();
thread_prio = GetThreadPriority (current_thread);
}
if (thread_prio == THREAD_PRIORITY_ERROR_RETURN)
{

View File

@ -885,7 +885,7 @@ g_thread_new (const gchar *name,
GError *error = NULL;
GThread *thread;
thread = g_thread_new_internal (name, g_thread_proxy, func, data, 0, NULL, &error);
thread = g_thread_new_internal (name, g_thread_proxy, func, data, 0, &error);
if G_UNLIKELY (thread == NULL)
g_error ("creating thread '%s': %s", name ? name : "", error->message);
@ -916,7 +916,7 @@ g_thread_try_new (const gchar *name,
gpointer data,
GError **error)
{
return g_thread_new_internal (name, g_thread_proxy, func, data, 0, NULL, error);
return g_thread_new_internal (name, g_thread_proxy, func, data, 0, error);
}
GThread *
@ -925,7 +925,6 @@ g_thread_new_internal (const gchar *name,
GThreadFunc func,
gpointer data,
gsize stack_size,
const GThreadSchedulerSettings *scheduler_settings,
GError **error)
{
g_return_val_if_fail (func != NULL, NULL);
@ -933,16 +932,7 @@ g_thread_new_internal (const gchar *name,
g_atomic_int_inc (&g_thread_n_created_counter);
g_trace_mark (G_TRACE_CURRENT_TIME, 0, "GLib", "GThread created", "%s", name ? name : "(unnamed)");
return (GThread *) g_system_thread_new (proxy, stack_size, scheduler_settings,
name, func, data, error);
}
gboolean
g_thread_get_scheduler_settings (GThreadSchedulerSettings *scheduler_settings)
{
g_return_val_if_fail (scheduler_settings != NULL, FALSE);
return g_system_thread_get_scheduler_settings (scheduler_settings);
return (GThread *) g_system_thread_new (proxy, stack_size, name, func, data, error);
}
/**

View File

@ -117,9 +117,6 @@ static gint max_unused_threads = 2;
static gint kill_unused_threads = 0;
static guint max_idle_time = 15 * 1000;
static GThreadSchedulerSettings shared_thread_scheduler_settings;
static gboolean have_shared_thread_scheduler_settings = FALSE;
typedef struct
{
/* Either thread or error are set in the end. Both transfer-full. */
@ -471,30 +468,22 @@ g_thread_pool_start_thread (GRealThreadPool *pool,
{
/* For non-exclusive thread-pools this can be called at any time
* when a new thread is needed. We make sure to create a new thread
* here with the correct scheduler settings: either by directly
* providing them if supported by the GThread implementation or by
* going via our helper thread.
* here with the correct scheduler settings by going via our helper
* thread.
*/
if (have_shared_thread_scheduler_settings)
{
thread = g_thread_new_internal (name, g_thread_proxy, g_thread_pool_thread_proxy, pool, 0, &shared_thread_scheduler_settings, error);
}
else
{
SpawnThreadData spawn_thread_data = { (GThreadPool *) pool, NULL, NULL };
SpawnThreadData spawn_thread_data = { (GThreadPool *) pool, NULL, NULL };
g_async_queue_lock (spawn_thread_queue);
g_async_queue_lock (spawn_thread_queue);
g_async_queue_push_unlocked (spawn_thread_queue, &spawn_thread_data);
g_async_queue_push_unlocked (spawn_thread_queue, &spawn_thread_data);
while (!spawn_thread_data.thread && !spawn_thread_data.error)
g_cond_wait (&spawn_thread_cond, _g_async_queue_get_mutex (spawn_thread_queue));
while (!spawn_thread_data.thread && !spawn_thread_data.error)
g_cond_wait (&spawn_thread_cond, _g_async_queue_get_mutex (spawn_thread_queue));
thread = spawn_thread_data.thread;
if (!thread)
g_propagate_error (error, g_steal_pointer (&spawn_thread_data.error));
g_async_queue_unlock (spawn_thread_queue);
}
thread = spawn_thread_data.thread;
if (!thread)
g_propagate_error (error, g_steal_pointer (&spawn_thread_data.error));
g_async_queue_unlock (spawn_thread_queue);
}
if (thread == NULL)
@ -547,6 +536,11 @@ g_thread_pool_start_thread (GRealThreadPool *pool,
* since their threads are never considered idle and returned to the
* global pool.
*
* Note that the threads used by exclusive threadpools will all inherit the
* scheduler settings of the current thread while the threads used by
* non-exclusive threadpools will inherit the scheduler settings from the
* first thread that created such a threadpool.
*
* @error can be %NULL to ignore errors, or non-%NULL to report
* errors. An error can only occur when @exclusive is set to %TRUE
* and not all @max_threads threads could be created.
@ -620,43 +614,31 @@ g_thread_pool_new_full (GFunc func,
if (!unused_thread_queue)
unused_thread_queue = g_async_queue_new ();
/* For the very first non-exclusive thread-pool we remember the thread
* scheduler settings of the thread creating the pool, if supported by
* the GThread implementation. This is then used for making sure that
* all threads created on the non-exclusive thread-pool have the same
* scheduler settings, and more importantly don't just inherit them
* from the thread that just happened to push a new task and caused
* a new thread to be created.
/*
* Spawn a helper thread that is only responsible for spawning new threads
* with the scheduler settings of the current thread.
*
* This is then used for making sure that all threads created on the
* non-exclusive thread-pool have the same scheduler settings, and more
* importantly don't just inherit them from the thread that just happened to
* push a new task and caused a new thread to be created.
*
* Not doing so could cause real-time priority threads or otherwise
* threads with problematic scheduler settings to be part of the
* non-exclusive thread-pools.
*
* If this is not supported by the GThread implementation then we here
* start a thread that will inherit the scheduler settings from this
* very thread and whose only purpose is to spawn new threads with the
* same settings for use by the non-exclusive thread-pools.
*
*
* For non-exclusive thread-pools this is not required as all threads
* are created immediately below and are running forever, so they will
* For exclusive thread-pools this is not required as all threads are
* created immediately below and are running forever, so they will
* automatically inherit the scheduler settings from this very thread.
*/
if (!exclusive && !have_shared_thread_scheduler_settings && !spawn_thread_queue)
if (!exclusive && !spawn_thread_queue)
{
if (g_thread_get_scheduler_settings (&shared_thread_scheduler_settings))
{
have_shared_thread_scheduler_settings = TRUE;
}
else
{
GThread *pool_spawner = NULL;
GThread *pool_spawner = NULL;
spawn_thread_queue = g_async_queue_new ();
g_cond_init (&spawn_thread_cond);
pool_spawner = g_thread_new ("pool-spawner", g_thread_pool_spawn_thread, NULL);
g_ignore_leak (pool_spawner);
}
spawn_thread_queue = g_async_queue_new ();
g_cond_init (&spawn_thread_cond);
pool_spawner = g_thread_new ("pool-spawner", g_thread_pool_spawn_thread, NULL);
g_ignore_leak (pool_spawner);
}
G_UNLOCK (init);

View File

@ -90,25 +90,10 @@ struct _GRealThread
#endif
/* Platform-specific scheduler settings for a thread */
typedef struct
{
#if defined(HAVE_SYS_SCHED_GETATTR)
/* This is for modern Linux */
struct sched_attr *attr;
#elif defined(G_OS_WIN32)
gint thread_prio;
#else
/* TODO: Add support for macOS and the BSDs */
void *dummy;
#endif
} GThreadSchedulerSettings;
void g_system_thread_wait (GRealThread *thread);
GRealThread *g_system_thread_new (GThreadFunc proxy,
gulong stack_size,
const GThreadSchedulerSettings *scheduler_settings,
const char *name,
GThreadFunc func,
gpointer data,
@ -118,19 +103,14 @@ void g_system_thread_free (GRealThread *thread);
void g_system_thread_exit (void);
void g_system_thread_set_name (const gchar *name);
gboolean g_system_thread_get_scheduler_settings (GThreadSchedulerSettings *scheduler_settings);
/* gthread.c */
GThread *g_thread_new_internal (const gchar *name,
GThreadFunc proxy,
GThreadFunc func,
gpointer data,
gsize stack_size,
const GThreadSchedulerSettings *scheduler_settings,
GError **error);
gboolean g_thread_get_scheduler_settings (GThreadSchedulerSettings *scheduler_settings);
gpointer g_thread_proxy (gpointer thread);
guint g_thread_n_created (void);

View File

@ -1952,10 +1952,6 @@ else
glib_conf.set('HAVE_PTHREAD_GETNAME_NP', 1)
endif
if cc.has_header_symbol('sys/syscall.h', 'SYS_sched_getattr')
glib_conf.set('HAVE_SYS_SCHED_GETATTR', 1)
endif
# Assume that pthread_setname_np is available in some form; same as configure
if cc.links(pthread_prefix + '''
int main() {