Re: Bug#23924 - crashes on next OS X release

[Alan Third <alan@idiocy.org>]

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

On Mon, Jul 18, 2016 at 08:03:46PM +0100, Alan Third wrote:
> On Mon, Jul 18, 2016 at 09:16:09AM -0700, John Wiegley wrote:
> > >>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > EZ> If you are confident that it won't do any trouble, and that 10.10, and
> > EZ> only that version, needs this fix, feel free to push to emacs-25.
> > 
> > Before you do, can you show us the patch here so that I can have a sense of
> > what it changes?  I've been running emacs-25 crash-free on 10.11 for many
> > months now.
> 
> I’m not that confident as I couldn’t find anything about the bug in
> question except what was in the bug report and comment in the code. I
> tried removing the fix and can’t reproduce the original problem on
> 10.11. It’s possible it’s actually been fixed in later releases of
> 10.10 too.

I’ve got a new patch. One of the testers of the first patch ran into
bug#18993, so it’s clearly still an issue, and further investigation
leaves me suspecting that, contrary to the bug report thread, it’s not
an OS X bug, but something to do with how Emacs is handling events.

The new patch leaves the fix in place and uses an existing GNUStep fix
to ensure that postEvent always runs in the main thread. That fixes
bug#23924.

There’s probably an argument to be made that we should fix 18993
properly, but I don’t think we have anyone who knows the NS port well
enough to do it, as it would require a substantial rewrite of the
event loop.

If this is to be applied to Emacs 25, let me know if it’s to go to
master too. I’m unsure how it’s handled.
-- 
Alan Third

[-- Attachment #2: patch for bug#23924 --]
[-- Type: text/plain, Size: 2248 bytes --]

From 50d21628a5288420ead32ba97731a496101289b5 Mon Sep 17 00:00:00 2001
In-Reply-To: <6629E4B5-5D34-4840-B0A1-A62BA025C472@play-bow.org>
References: <6629E4B5-5D34-4840-B0A1-A62BA025C472@play-bow.org>
From: Alan Third <alan@idiocy.org>
Date: Wed, 20 Jul 2016 21:59:17 +0100
Subject: [PATCH] Post AppDefined events from the main thread ONLY (bug#23934)
To: Bob Halley <halley@play-bow.org>
Cc: 23924@debbugs.gnu.org

* src/nsterm.h: Make nextappdefined var not just GNUStep.
* src/nsterm.c (ns_send_appdefined, sendFromMainThread): Remove GNUStep
---
 src/nsterm.h | 2 +-
 src/nsterm.m | 5 +----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/nsterm.h b/src/nsterm.h
index 862ff2e..3d8b1a1 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -380,9 +380,9 @@ char const * nstrace_fullscreen_type_name (int);
 #endif
 #ifdef NS_IMPL_GNUSTEP
   BOOL applicationDidFinishLaunchingCalled;
+#endif
 @public
   int nextappdefined;
-#endif
 }
 - (void)logNotification: (NSNotification *)notification;
 - (void)antialiasThresholdDidChange:(NSNotification *)notification;
diff --git a/src/nsterm.m b/src/nsterm.m
index 8da2ffe..dcc1e87 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3927,8 +3927,8 @@ overwriting cursor (usually when cursor on a tab) */
 {
   NSTRACE_WHEN (NSTRACE_GROUP_EVENTS, "ns_send_appdefined(%d)", value);
 
-#ifdef NS_IMPL_GNUSTEP
   // GNUstep needs postEvent to happen on the main thread.
+  // Cocoa needs nextEventMatchingMask to happen on the main thread too.
   if (! [[NSThread currentThread] isMainThread])
     {
       EmacsApp *app = (EmacsApp *)NSApp;
@@ -3938,7 +3938,6 @@ overwriting cursor (usually when cursor on a tab) */
                          waitUntilDone:YES];
       return;
     }
-#endif
 
   /* Only post this event if we haven't already posted one.  This will end
        the [NXApp run] main loop after having processed all events queued at
@@ -5551,12 +5550,10 @@ - (void)timeout_handler: (NSTimer *)timedEntry
   ns_send_appdefined (-2);
 }
 
-#ifdef NS_IMPL_GNUSTEP
 - (void)sendFromMainThread:(id)unused
 {
   ns_send_appdefined (nextappdefined);
 }
-#endif
 
 - (void)fd_handler:(id)unused
 /* --------------------------------------------------------------------------
-- 
2.7.4