bug#25265: make-thread crashes in OS X 10.6

[Alan Third <alan@idiocy.org>]

[-- Attachment #1: Type: text/plain, Size: 1491 bytes --]

On Sun, Jun 18, 2017 at 03:05:04PM +0200, Charles A. Roelli wrote:
> The tests sometimes all pass with the updated patch, but then I got
> this once (the errors you've seen previously, I think):
> 
<snip>

Yeah, they look the same. I’m not sure what I can do to fix them if
they’re still happening.

> I also tried this way of running the tests in batch mode:
> 
> cd test && rm -rf src/thread-tests.log && make src/thread-tests.log
> 
> but got a segfault:
> 
>   GEN      src/thread-tests.log
> /bin/sh: line 1: 87214 Segmentation fault      HOME=/nonexistent
> EMACSLOADPATH= LC_ALL=C
> EMACS_TEST_DIRECTORY=/Volumes/Toblerone/Code/emacs-devel/test "../src/emacs"
> -batch --no-site-file --no-site-lisp -L ":." -l ert -l src/thread-tests.el
> --eval "(ert-run-tests-batch-and-exit nil)" > src/thread-tests.log 2>&1
> Running 28 tests (2017-06-18 15:00:03+0200)
> make: *** [src/thread-tests.log] Error 139
> 
> I'm not sure where the problem is happening (or if it's specific to
> NS).  The file 'src/thread-tests.log' also doesn't give any extra
> information (besides 'Running 28 tests (2017-06-18 15:00:03+0200)').

Yeah, I think that’s because I, accidentally, have the non‐GUI version
trying to run some GUI stuff. I noticed this myself yesterday.

Running NS Emacs without a GUI has always worked with threads because
it just bypasses the NS run loop which is the problem.

I’ve attached a new patch that actually checks whether we’re running
the NS GUI.
-- 
Alan Third

[-- Attachment #2: 0001-Fix-threads-on-NS-bug-25265.patch --]
[-- Type: text/plain, Size: 5282 bytes --]

From 62ac72333084ae32719d63b82166f1c6daee1ed9 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Tue, 13 Jun 2017 21:40:25 +0100
Subject: [PATCH] Fix threads on NS (bug#25265)

src/nsterm.h (ns_select): Compiler doesn't like sigmask being const.
(ns_run_loop_break) [HAVE_PTHREAD]: New function.
src/nsterm.m (ns_select): Call thread_select from within ns_select.
(ns_run_loop_break) [HAVE_PTHREAD]: New function.
(ns_send_appdefined): Don't wait for main thread when sending app
defined event.
src/process.c (wait_reading_process_output): Call thread_select from
within ns_select.
src/systhread.c (sys_cond_broadcast) [HAVE_NS]: Break ns_select out of
its event loop using ns_run_loop_break.
---
 src/nsterm.h    |  7 +++++--
 src/nsterm.m    | 26 ++++++++++++++++++++++----
 src/process.c   | 13 ++++++-------
 src/systhread.c | 11 +++++++++++
 4 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/src/nsterm.h b/src/nsterm.h
index bed0b92c79..73d846d114 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -1225,8 +1225,11 @@ extern void x_set_no_accept_focus (struct frame *f, Lisp_Object new_value,
 extern void x_set_z_group (struct frame *f, Lisp_Object new_value,
                            Lisp_Object old_value);
 extern int ns_select (int nfds, fd_set *readfds, fd_set *writefds,
-		      fd_set *exceptfds, struct timespec const *timeout,
-		      sigset_t const *sigmask);
+		      fd_set *exceptfds, struct timespec *timeout,
+		      sigset_t *sigmask);
+#ifdef HAVE_PTHREAD
+extern void ns_run_loop_break (void);
+#endif
 extern unsigned long ns_get_rgb_color (struct frame *f,
                                        float r, float g, float b, float a);
 
diff --git a/src/nsterm.m b/src/nsterm.m
index e05dbf45fb..bf83550b3d 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -4068,7 +4068,7 @@ overwriting cursor (usually when cursor on a tab) */
       app->nextappdefined = value;
       [app performSelectorOnMainThread:@selector (sendFromMainThread:)
                             withObject:nil
-                         waitUntilDone:YES];
+                         waitUntilDone:NO];
       return;
     }
 
@@ -4293,8 +4293,8 @@ in certain situations (rapid incoming events).
 
 int
 ns_select (int nfds, fd_set *readfds, fd_set *writefds,
-	   fd_set *exceptfds, struct timespec const *timeout,
-	   sigset_t const *sigmask)
+	   fd_set *exceptfds, struct timespec *timeout,
+	   sigset_t *sigmask)
 /* --------------------------------------------------------------------------
      Replacement for select, checking for events
    -------------------------------------------------------------------------- */
@@ -4327,7 +4327,13 @@ in certain situations (rapid incoming events).
   if (NSApp == nil
       || ![NSThread isMainThread]
       || (timeout && timeout->tv_sec == 0 && timeout->tv_nsec == 0))
-    return pselect (nfds, readfds, writefds, exceptfds, timeout, sigmask);
+    return thread_select(pselect, nfds, readfds, writefds,
+                         exceptfds, timeout, sigmask);
+  else
+    {
+      struct timespec t = {0, 0};
+      thread_select(pselect, 0, NULL, NULL, NULL, &t, sigmask);
+    }
 
   [outerpool release];
   outerpool = [[NSAutoreleasePool alloc] init];
@@ -4430,6 +4436,18 @@ in certain situations (rapid incoming events).
   return result;
 }
 
+#ifdef HAVE_PTHREAD
+void
+ns_run_loop_break ()
+/* Break out of the NS run loop in ns_select or ns_read_socket. */
+{
+  NSTRACE_WHEN (NSTRACE_GROUP_EVENTS, "ns_run_loop_break");
+
+  /* If we don't have a GUI, don't send the event. */
+  if (NSApp != NULL)
+    ns_send_appdefined(-1);
+}
+#endif
 
 
 /* ==========================================================================
diff --git a/src/process.c b/src/process.c
index 2a1c2eecde..abd017bb90 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5371,14 +5371,13 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 	  nfds = xg_select (max_desc + 1,
 			    &Available, (check_write ? &Writeok : 0),
 			    NULL, &timeout, NULL);
+#elif defined HAVE_NS
+          /* And NS builds call thread_select in ns_select. */
+          nfds = ns_select (max_desc + 1,
+			    &Available, (check_write ? &Writeok : 0),
+			    NULL, &timeout, NULL);
 #else  /* !HAVE_GLIB */
-	  nfds = thread_select (
-# ifdef HAVE_NS
-				ns_select
-# else
-				pselect
-# endif
-				, max_desc + 1,
+	  nfds = thread_select (pselect, max_desc + 1,
 				&Available,
 				(check_write ? &Writeok : 0),
 				NULL, &timeout, NULL);
diff --git a/src/systhread.c b/src/systhread.c
index a84060c18f..aee12a9b48 100644
--- a/src/systhread.c
+++ b/src/systhread.c
@@ -20,6 +20,10 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <setjmp.h>
 #include "lisp.h"
 
+#ifdef HAVE_NS
+#include "nsterm.h"
+#endif
+
 #ifndef THREADS_ENABLED
 
 void
@@ -130,6 +134,13 @@ void
 sys_cond_broadcast (sys_cond_t *cond)
 {
   pthread_cond_broadcast (cond);
+#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
+     broadcast has no effect until the loop is done, breaking a couple
+     of tests in thread-tests.el. */
+  ns_run_loop_break ();
+#endif
 }
 
 void
-- 
2.12.0