unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Philipp Stephani <p.stephani2@gmail.com>
To: emacs-devel@gnu.org
Cc: Philipp Stephani <phst@google.com>
Subject: [PATCH] Make error checking for thread functions stricter.
Date: Sat,  2 Jun 2018 22:42:31 +0200	[thread overview]
Message-ID: <20180602204231.92373-1-phst@google.com> (raw)

* 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, 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 | 61 ++++++++++++++++++++++++++++++++++---------------
 src/systhread.h | 14 ++++++++----
 2 files changed, 52 insertions(+), 23 deletions(-)

diff --git a/src/systhread.c b/src/systhread.c
index e972ed398a..71c13aa67b 100644
--- a/src/systhread.c
+++ b/src/systhread.c
@@ -18,6 +18,8 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 
 #include <config.h>
 #include <setjmp.h>
+#include <stdio.h>
+#include <string.h>
 #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,60 @@ sys_thread_yield (void)
 void
 sys_mutex_init (sys_mutex_t *mutex)
 {
-  pthread_mutex_init (mutex, NULL);
+  int error = pthread_mutex_init (mutex, NULL);
+  /* 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 ();
+    }
 }
 
 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 +171,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 +187,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 +216,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 +384,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 +408,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 +423,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 5dbb12dffb..a392feb080 100644
--- a/src/systhread.h
+++ b/src/systhread.h
@@ -19,6 +19,8 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #ifndef SYSTHREAD_H
 #define SYSTHREAD_H
 
+#include <stdbool.h>
+
 #ifdef THREADS_ENABLED
 
 #ifdef HAVE_PTHREAD
@@ -99,12 +101,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.17.1




             reply	other threads:[~2018-06-02 20:42 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-02 20:42 Philipp Stephani [this message]
2018-06-09 16:48 ` [PATCH] Make error checking for thread functions stricter Philipp Stephani

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180602204231.92373-1-phst@google.com \
    --to=p.stephani2@gmail.com \
    --cc=emacs-devel@gnu.org \
    --cc=phst@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).