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

[Alan Third <alan@idiocy.org>]

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

On Fri, Jun 16, 2017 at 04:05:58PM -0400, Noam Postavsky wrote:
> On Fri, Jun 16, 2017 at 3:45 PM, Alan Third <alan@idiocy.org> wrote:
> > The first one looks like the thread’s starting too quick?
> 
> I think it's rather failing to be killed by the `thread-signal' call.

Ah, yes. I completely misread the test.

> > The second looks like the thread’s not dying quick enough?
> 
> Same for that one. I don't know much about the thread internals, and
> even less macOS internals though...

Well, I’ve discovered that if I keep the mouse moving over the Emacs
frame while the tests are running then none of them fail, so I think
it’s something to do with ns_select jamming everything up for the
duration of its timeout.

I could probably get round that by getting the thread to fire off an
NS app defined event when it dies. That would break ns_select out of
its event loop, presumably letting the dying thread actually die...

Amazingly that seems to have worked! Updated patch attached.
-- 
Alan Third

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

From ab35f15d5091c29011f6a538009612f1b5c5137d 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_select_break) [HAVE_PTHREAD]: New function.
src/nsterm.m (ns_select): Call thread_select from within ns_select.
(ns_select_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.
---
 src/nsterm.h    |  7 +++++--
 src/nsterm.m    | 21 +++++++++++++++++----
 src/process.c   | 13 ++++++-------
 src/systhread.c |  7 +++++++
 4 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/src/nsterm.h b/src/nsterm.h
index bed0b92c79..eac54e1bcf 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_select_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..29a45ed851 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,13 @@ in certain situations (rapid incoming events).
   return result;
 }
 
+#ifdef HAVE_PTHREAD
+void
+ns_select_break ()
+{
+  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..53dfe7a9c4 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,9 @@ void
 sys_cond_broadcast (sys_cond_t *cond)
 {
   pthread_cond_broadcast (cond);
+#ifdef HAVE_NS
+  ns_select_break ();
+#endif
 }
 
 void
-- 
2.12.0