From b4dcac2d6ae376788dca11b88b874a9ecaf986df Mon Sep 17 00:00:00 2001 From: Philipp Stephani Date: Sat, 2 Jun 2018 22:35:22 +0200 Subject: [PATCH] Make error checking for thread functions stricter. * src/systhread.c (sys_thread_create): Change return type to bool. Check for errors returned by pthread_attr_setstacksize and pthread_attr_destroy. (sys_mutex_init): Abort on errors. Enable mutex checks when checking is enabled. (sys_cond_init): Abort on errors. (sys_mutex_lock, sys_mutex_unlock, sys_cond_wait) (sys_cond_signal, sys_cond_broadcast, sys_cond_destroy): Check for errors in debug mode. --- src/systhread.c | 78 +++++++++++++++++++++++++++++++++++++------------ src/systhread.h | 24 +++++++++++---- 2 files changed, 79 insertions(+), 23 deletions(-) diff --git a/src/systhread.c b/src/systhread.c index e972ed398ac..d53b5c207b6 100644 --- a/src/systhread.c +++ b/src/systhread.c @@ -18,6 +18,8 @@ along with GNU Emacs. If not, see . */ #include #include +#include +#include #include "lisp.h" #ifdef HAVE_NS @@ -80,11 +82,11 @@ sys_thread_equal (sys_thread_t t, sys_thread_t u) return t == u; } -int +bool sys_thread_create (sys_thread_t *t, const char *name, thread_creation_function *func, void *datum) { - return 0; + return false; } void @@ -103,43 +105,77 @@ sys_thread_yield (void) void sys_mutex_init (sys_mutex_t *mutex) { - pthread_mutex_init (mutex, NULL); + pthread_mutexattr_t *attr_ptr; +#ifdef ENABLE_CHECKING + pthread_mutexattr_t attr; + { + int error = pthread_mutexattr_init (&attr); + eassert (error == 0); + error = pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_ERRORCHECK); + eassert (error == 0); + } + attr_ptr = &attr; +#else + attr_ptr = NULL; +#endif + int error = pthread_mutex_init (mutex, attr_ptr); + /* We could get ENOMEM. Can't do anything except aborting. */ + if (error != 0) + { + fprintf (stderr, "\npthread_mutex_init failed: %s\n", strerror (error)); + emacs_abort (); + } +#ifdef ENABLE_CHECKING + error = pthread_mutexattr_destroy (&attr); + eassert (error == 0); +#endif } void sys_mutex_lock (sys_mutex_t *mutex) { - pthread_mutex_lock (mutex); + int error = pthread_mutex_lock (mutex); + eassert (error == 0); } void sys_mutex_unlock (sys_mutex_t *mutex) { - pthread_mutex_unlock (mutex); + int error = pthread_mutex_unlock (mutex); + eassert (error == 0); } void sys_cond_init (sys_cond_t *cond) { - pthread_cond_init (cond, NULL); + int error = pthread_cond_init (cond, NULL); + /* We could get ENOMEM. Can't do anything except aborting. */ + if (error != 0) + { + fprintf (stderr, "\npthread_cond_init failed: %s\n", strerror (error)); + emacs_abort (); + } } void sys_cond_wait (sys_cond_t *cond, sys_mutex_t *mutex) { - pthread_cond_wait (cond, mutex); + int error = pthread_cond_wait (cond, mutex); + eassert (error == 0); } void sys_cond_signal (sys_cond_t *cond) { - pthread_cond_signal (cond); + int error = pthread_cond_signal (cond); + eassert (error == 0); } void sys_cond_broadcast (sys_cond_t *cond) { - pthread_cond_broadcast (cond); + int error = pthread_cond_broadcast (cond); + eassert (error == 0); #ifdef HAVE_NS /* Send an app defined event to break out of the NS run loop. It seems that if ns_select is running the NS run loop, this @@ -152,7 +188,8 @@ sys_cond_broadcast (sys_cond_t *cond) void sys_cond_destroy (sys_cond_t *cond) { - pthread_cond_destroy (cond); + int error = pthread_cond_destroy (cond); + eassert (error == 0); } sys_thread_t @@ -167,22 +204,25 @@ sys_thread_equal (sys_thread_t t, sys_thread_t u) return pthread_equal (t, u); } -int +bool sys_thread_create (sys_thread_t *thread_ptr, const char *name, thread_creation_function *func, void *arg) { pthread_attr_t attr; - int result = 0; + bool result = false; if (pthread_attr_init (&attr)) - return 0; + return false; /* Avoid crash on macOS with deeply nested GC (Bug#30364). */ size_t stack_size; size_t required_stack_size = sizeof (void *) * 1024 * 1024; if (pthread_attr_getstacksize (&attr, &stack_size) == 0 && stack_size < required_stack_size) - pthread_attr_setstacksize (&attr, required_stack_size); + { + if (pthread_attr_setstacksize (&attr, required_stack_size) != 0) + goto out; + } if (!pthread_attr_setdetachstate (&attr, PTHREAD_CREATE_DETACHED)) { @@ -193,7 +233,9 @@ sys_thread_create (sys_thread_t *thread_ptr, const char *name, #endif } - pthread_attr_destroy (&attr); + out: ; + int error = pthread_attr_destroy (&attr); + eassert (error == 0); return result; } @@ -359,7 +401,7 @@ w32_beginthread_wrapper (void *arg) (void)thread_start_address (arg); } -int +bool sys_thread_create (sys_thread_t *thread_ptr, const char *name, thread_creation_function *func, void *arg) { @@ -383,7 +425,7 @@ sys_thread_create (sys_thread_t *thread_ptr, const char *name, rule in many places... */ thandle = _beginthread (w32_beginthread_wrapper, stack_size, arg); if (thandle == (uintptr_t)-1L) - return 0; + return false; /* Kludge alert! We use the Windows thread ID, an unsigned 32-bit number, as the sys_thread_t type, because that ID is the only @@ -398,7 +440,7 @@ sys_thread_create (sys_thread_t *thread_ptr, const char *name, Therefore, we return some more or less arbitrary value of the thread ID from this function. */ *thread_ptr = thandle & 0xFFFFFFFF; - return 1; + return true; } void diff --git a/src/systhread.h b/src/systhread.h index 5dbb12dffb6..3805cb261f1 100644 --- a/src/systhread.h +++ b/src/systhread.h @@ -19,6 +19,18 @@ along with GNU Emacs. If not, see . */ #ifndef SYSTHREAD_H #define SYSTHREAD_H +#include + +#ifndef __has_attribute +# define __has_attribute(a) false +#endif + +#if __has_attribute (__warn_unused_result__) +# define ATTRIBUTE_WARN_UNUSED_RESULT __attribute__ ((__warn_unused_result__)) +#else +# define ATTRIBUTE_WARN_UNUSED_RESULT +#endif + #ifdef THREADS_ENABLED #ifdef HAVE_PTHREAD @@ -99,12 +111,14 @@ extern void sys_cond_signal (sys_cond_t *); extern void sys_cond_broadcast (sys_cond_t *); extern void sys_cond_destroy (sys_cond_t *); -extern sys_thread_t sys_thread_self (void); -extern bool sys_thread_equal (sys_thread_t, sys_thread_t); +extern sys_thread_t sys_thread_self (void) + ATTRIBUTE_WARN_UNUSED_RESULT; +extern bool sys_thread_equal (sys_thread_t, sys_thread_t) + ATTRIBUTE_WARN_UNUSED_RESULT; -extern int sys_thread_create (sys_thread_t *, const char *, - thread_creation_function *, - void *); +extern bool sys_thread_create (sys_thread_t *, const char *, + thread_creation_function *, void *) + ATTRIBUTE_WARN_UNUSED_RESULT; extern void sys_thread_yield (void); -- 2.39.2