bug#25265: [PATCH] Rework NS event handling (bug#25265)

[Alan Third]

* src/nsterm.m (unwind_apploopnr): Remove.
(ns_read_socket): Remove references to apploopnr.  Make processing the
NS event loop conditional on being in the main thread.
(ns_select): Remove references to apploopnr.  Remove all fd_handler
related stuff.  Check if there are events waiting on the NS event
queue rather than running the event loop.  Remove unused variables and
code.
(fd_handler): Remove.
(ns_term_init): Remove creation of fd_handler thread.
(hold_event, EmacsApp:sendEvent, EmacsView:mouseMoved,
EmacsView:windowDidExpose): Remove send_appdefined.
(ns_send_appdefined): Always check the event queue for
applicationDefined events rather than relying on send_appdefined var.
* src/nsterm.h: Remove reference to fd_handler method.
---
 src/nsterm.h |   1 -
 src/nsterm.m | 380 +++++++++++------------------------------------------------
 2 files changed, 68 insertions(+), 313 deletions(-)

diff --git a/src/nsterm.h b/src/nsterm.h
index 35c6e1a..dc222a7 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -392,7 +392,6 @@ char const * nstrace_fullscreen_type_name (int);
 - (void)sendEvent: (NSEvent *)theEvent;
 - (void)showPreferencesWindow: (id)sender;
 - (BOOL) openFile: (NSString *)fileName;
-- (void)fd_handler: (id)unused;
 - (void)timeout_handler: (NSTimer *)timedEntry;
 - (BOOL)fulfillService: (NSString *)name withArg: (NSString *)arg;
 #ifdef NS_IMPL_GNUSTEP
diff --git a/src/nsterm.m b/src/nsterm.m
index 7e6ec85..98fd8ab 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -279,18 +279,10 @@ - (NSColor *)colorUsingDefaultColorSpace
 /*static int debug_lock = 0; */
 
 /* event loop */
-static BOOL send_appdefined = YES;
 #define NO_APPDEFINED_DATA (-8)
 static int last_appdefined_event_data = NO_APPDEFINED_DATA;
 static NSTimer *timed_entry = 0;
 static NSTimer *scroll_repeat_entry = nil;
-static fd_set select_readfds, select_writefds;
-enum { SELECT_HAVE_READ = 1, SELECT_HAVE_WRITE = 2, SELECT_HAVE_TMO = 4 };
-static int select_nfds = 0, select_valid = 0;
-static struct timespec select_timeout = { 0, 0 };
-static int selfds[2] = { -1, -1 };
-static pthread_mutex_t select_mutex;
-static int apploopnr = 0;
 static NSAutoreleasePool *outerpool;
 static struct input_event *emacs_event = NULL;
 static struct input_event *q_event_ptr = NULL;
@@ -457,7 +449,6 @@ - (NSColor *)colorUsingDefaultColorSpace
   hold_event_q.q[hold_event_q.nr++] = *event;
   /* Make sure ns_read_socket is called, i.e. we have input.  */
   raise (SIGIO);
-  send_appdefined = YES;
 }
 
 static Lisp_Object
@@ -3872,31 +3863,17 @@ overwriting cursor (usually when cursor on a tab) */
       return;
     }
 
-  /* 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
-       this moment.  */
-
-#ifdef NS_IMPL_COCOA
-  if (! send_appdefined)
-    {
-      /* OS X 10.10.1 swallows the AppDefined event we are sending ourselves
-         in certain situations (rapid incoming events).
-         So check if we have one, if not add one.  */
-      NSEvent *appev = [NSApp 
nextEventMatchingMask:NSEventMaskApplicationDefined
-                                          untilDate:[NSDate distantPast]
-                                             inMode:NSDefaultRunLoopMode
-                                            dequeue:NO];
-      if (! appev) send_appdefined = YES;
-    }
-#endif
-
-  if (send_appdefined)
+  /* 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 this moment.  */
+  NSEvent *appev = [NSApp nextEventMatchingMask:NSEventMaskApplicationDefined
+                                      untilDate:[NSDate distantPast]
+                                         inMode:NSDefaultRunLoopMode
+                                        dequeue:NO];
+  if (! appev)
     {
       NSEvent *nxev;
 
-      /* We only need one NX_APPDEFINED event to stop NXApp from running.  */
-      send_appdefined = NO;
-
       /* Don't need wakeup timer any more */
       if (timed_entry)
         {
@@ -4011,14 +3988,6 @@ in certain situations (rapid incoming events).
 }
 #endif /* NS_IMPL_COCOA */
 
-static void
-unwind_apploopnr (Lisp_Object not_used)
-{
-  --apploopnr;
-  n_emacs_events_pending = 0;
-  ns_finish_events ();
-  q_event_ptr = NULL;
-}
 
 static int
 ns_read_socket (struct terminal *terminal, struct input_event *hold_quit)
@@ -4029,13 +3998,10 @@ in certain situations (rapid incoming events).
    -------------------------------------------------------------------------- 
*/
 {
   struct input_event ev;
-  int nevents;
+  int nevents = 0;
 
   NSTRACE_WHEN (NSTRACE_GROUP_EVENTS, "ns_read_socket");
 
-  if (apploopnr > 0)
-    return -1; /* Already within event loop. */
-
 #ifdef HAVE_NATIVE_FS
   check_native_fs ();
 #endif
@@ -4052,54 +4018,49 @@ in certain situations (rapid incoming events).
       return i;
     }
 
-  block_input ();
-  n_emacs_events_pending = 0;
-  ns_init_events (&ev);
-  q_event_ptr = hold_quit;
-
-  /* we manage autorelease pools by allocate/reallocate each time around
-     the loop; strict nesting is occasionally violated but seems not to
-     matter.. earlier methods using full nesting caused major memory leaks */
-  [outerpool release];
-  outerpool = [[NSAutoreleasePool alloc] init];
-
-  /* If have pending open-file requests, attend to the next one of those. */
-  if (ns_pending_files && [ns_pending_files count] != 0
-      && [(EmacsApp *)NSApp openFile: [ns_pending_files objectAtIndex: 0]])
+  if ([NSThread isMainThread])
     {
-      [ns_pending_files removeObjectAtIndex: 0];
-    }
-  /* Deal with pending service requests. */
-  else if (ns_pending_service_names && [ns_pending_service_names count] != 0
-    && [(EmacsApp *)
-         NSApp fulfillService: [ns_pending_service_names objectAtIndex: 0]
-                      withArg: [ns_pending_service_args objectAtIndex: 0]])
-    {
-      [ns_pending_service_names removeObjectAtIndex: 0];
-      [ns_pending_service_args removeObjectAtIndex: 0];
-    }
-  else
-    {
-      ptrdiff_t specpdl_count = SPECPDL_INDEX ();
-      /* Run and wait for events.  We must always send one NX_APPDEFINED event
-         to ourself, otherwise [NXApp run] will never exit.  */
-      send_appdefined = YES;
-      ns_send_appdefined (-1);
-
-      if (++apploopnr != 1)
+      block_input ();
+      n_emacs_events_pending = 0;
+      ns_init_events (&ev);
+      q_event_ptr = hold_quit;
+
+      /* we manage autorelease pools by allocate/reallocate each time around
+         the loop; strict nesting is occasionally violated but seems not to
+         matter.. earlier methods using full nesting caused major memory leaks 
*/
+      [outerpool release];
+      outerpool = [[NSAutoreleasePool alloc] init];
+
+      /* If have pending open-file requests, attend to the next one of those. 
*/
+      if (ns_pending_files && [ns_pending_files count] != 0
+          && [(EmacsApp *)NSApp openFile: [ns_pending_files objectAtIndex: 0]])
         {
-          emacs_abort ();
+          [ns_pending_files removeObjectAtIndex: 0];
         }
-      record_unwind_protect (unwind_apploopnr, Qt);
-      [NSApp run];
-      unbind_to (specpdl_count, Qnil);  /* calls unwind_apploopnr */
-    }
+      /* Deal with pending service requests. */
+      else if (ns_pending_service_names && [ns_pending_service_names count] != 0
+               && [(EmacsApp *)
+                    NSApp fulfillService: [ns_pending_service_names 
objectAtIndex: 0]
+                                 withArg: [ns_pending_service_args 
objectAtIndex: 0]])
+        {
+          [ns_pending_service_names removeObjectAtIndex: 0];
+          [ns_pending_service_args removeObjectAtIndex: 0];
+        }
+      else
+        {
+          /* Run and wait for events.  We must always send one NX_APPDEFINED 
event
+             to ourself, otherwise [NXApp run] will never exit.  */
+          ns_send_appdefined (-1);
 
-  nevents = n_emacs_events_pending;
-  n_emacs_events_pending = 0;
-  ns_finish_events ();
-  q_event_ptr = NULL;
-  unblock_input ();
+          [NSApp run];
+        }
+
+      nevents = n_emacs_events_pending;
+      n_emacs_events_pending = 0;
+      ns_finish_events ();
+      q_event_ptr = NULL;
+      unblock_input ();
+    }
 
   return nevents;
 }
@@ -4114,15 +4075,11 @@ in certain situations (rapid incoming events).
    -------------------------------------------------------------------------- 
*/
 {
   int result;
-  int t, k, nr = 0;
-  struct input_event event;
-  char c;
+  NSDate *timeout_date = nil;
+  NSEvent *ns_event;
 
   NSTRACE_WHEN (NSTRACE_GROUP_EVENTS, "ns_select");
 
-  if (apploopnr > 0)
-    return -1; /* Already within event loop. */
-
 #ifdef HAVE_NATIVE_FS
   check_native_fs ();
 #endif
@@ -4135,121 +4092,34 @@ in certain situations (rapid incoming events).
       return -1;
     }
 
-  for (k = 0; k < nfds+1; k++)
-    {
-      if (readfds && FD_ISSET(k, readfds)) ++nr;
-      if (writefds && FD_ISSET(k, writefds)) ++nr;
-    }
-
   if (NSApp == nil
+      || ![NSThread isMainThread]
       || (timeout && timeout->tv_sec == 0 && timeout->tv_nsec == 0))
-    return pselect (nfds, readfds, writefds, exceptfds, timeout, sigmask);
+    return pselect(nfds, readfds, writefds,
+                   exceptfds, timeout, sigmask);
+
+  result = pselect(nfds, readfds, writefds, exceptfds,
+                   &(struct timespec){.tv_sec = 0, .tv_nsec = 100},
+                   sigmask);
 
   [outerpool release];
   outerpool = [[NSAutoreleasePool alloc] init];
 
-
-  send_appdefined = YES;
-  if (nr > 0)
-    {
-      pthread_mutex_lock (&select_mutex);
-      select_nfds = nfds;
-      select_valid = 0;
-      if (readfds)
-        {
-          select_readfds = *readfds;
-          select_valid += SELECT_HAVE_READ;
-        }
-      if (writefds)
-        {
-          select_writefds = *writefds;
-          select_valid += SELECT_HAVE_WRITE;
-        }
-
-      if (timeout)
-        {
-          select_timeout = *timeout;
-          select_valid += SELECT_HAVE_TMO;
-        }
-
-      pthread_mutex_unlock (&select_mutex);
-
-      /* Inform fd_handler that select should be called */
-      c = 'g';
-      emacs_write_sig (selfds[1], &c, 1);
-    }
-  else if (nr == 0 && timeout)
+  if (timeout)
     {
-      /* No file descriptor, just a timeout, no need to wake fd_handler  */
       double time = timespectod (*timeout);
-      timed_entry = [[NSTimer scheduledTimerWithTimeInterval: time
-                                                      target: NSApp
-                                                    selector:
-                                  @selector (timeout_handler:)
-                                                    userInfo: 0
-                                                     repeats: NO]
-                      retain];
-    }
-  else /* No timeout and no file descriptors, can this happen?  */
-    {
-      /* Send appdefined so we exit from the loop */
-      ns_send_appdefined (-1);
-    }
-
-  block_input ();
-  ns_init_events (&event);
-  if (++apploopnr != 1)
-    {
-      emacs_abort ();
-    }
-
-  {
-    ptrdiff_t specpdl_count = SPECPDL_INDEX ();
-    record_unwind_protect (unwind_apploopnr, Qt);
-    [NSApp run];
-    unbind_to (specpdl_count, Qnil);  /* calls unwind_apploopnr */
-  }
-
-  ns_finish_events ();
-  if (nr > 0 && readfds)
-    {
-      c = 's';
-      emacs_write_sig (selfds[1], &c, 1);
+      timeout_date = [NSDate dateWithTimeIntervalSinceNow:time];
     }
-  unblock_input ();
 
-  t = last_appdefined_event_data;
+  /* Listen for a new NSEvent. */
+  ns_event = [NSApp nextEventMatchingMask:NSEventMaskAny
+                                untilDate:timeout_date
+                                   inMode:NSDefaultRunLoopMode
+                                  dequeue:NO];
 
-  if (t != NO_APPDEFINED_DATA)
+  if (ns_event != nil)
     {
-      last_appdefined_event_data = NO_APPDEFINED_DATA;
-
-      if (t == -2)
-        {
-          /* The NX_APPDEFINED event we received was a timeout. */
-          result = 0;
-        }
-      else if (t == -1)
-        {
-          /* The NX_APPDEFINED event we received was the result of
-             at least one real input event arriving.  */
-          errno = EINTR;
-          result = -1;
-        }
-      else
-        {
-          /* Received back from select () in fd_handler; copy the results */
-          pthread_mutex_lock (&select_mutex);
-          if (readfds) *readfds = select_readfds;
-          if (writefds) *writefds = select_writefds;
-          pthread_mutex_unlock (&select_mutex);
-          result = t;
-        }
-    }
-  else
-    {
-      errno = EINTR;
-      result = -1;
+      raise (SIGIO);
     }
 
   return result;
@@ -4765,21 +4635,6 @@ static Lisp_Object ns_string_to_lispmod (const char *s)
   baud_rate = 38400;
   Fset_input_interrupt_mode (Qnil);
 
-  if (selfds[0] == -1)
-    {
-      if (emacs_pipe (selfds) != 0)
-        {
-          fprintf (stderr, "Failed to create pipe: %s\n",
-                   emacs_strerror (errno));
-          emacs_abort ();
-        }
-
-      fcntl (selfds[0], F_SETFL, O_NONBLOCK|fcntl (selfds[0], F_GETFL));
-      FD_ZERO (&select_readfds);
-      FD_ZERO (&select_writefds);
-      pthread_mutex_init (&select_mutex, NULL);
-    }
-
   ns_pending_files = [[NSMutableArray alloc] init];
   ns_pending_service_names = [[NSMutableArray alloc] init];
   ns_pending_service_args = [[NSMutableArray alloc] init];
@@ -4792,11 +4647,6 @@ Needs to be here because ns_initialize_display_info () 
uses AppKit classes.
     return NULL;
   [NSApp setDelegate: NSApp];
 
-  /* Start the select thread.  */
-  [NSThread detachNewThreadSelector:@selector (fd_handler:)
-                           toTarget:NSApp
-                         withObject:nil];
-
   /* debugging: log all notifications */
   /*   [[NSNotificationCenter defaultCenter] addObserver: NSApp
                                          selector: @selector (logNotification:)
@@ -5178,10 +5028,6 @@ - (void)sendEvent: (NSEvent *)theEvent
           last_appdefined_event_data = [theEvent data1];
           [self stop: self];
         }
-      else
-        {
-          send_appdefined = YES;
-        }
     }
 
 
@@ -5484,95 +5330,6 @@ - (void)sendFromMainThread:(id)unused
   ns_send_appdefined (nextappdefined);
 }
 
-- (void)fd_handler:(id)unused
-/* --------------------------------------------------------------------------
-     Check data waiting on file descriptors and terminate if so
-   -------------------------------------------------------------------------- 
*/
-{
-  int result;
-  int waiting = 1, nfds;
-  char c;
-
-  fd_set readfds, writefds, *wfds;
-  struct timespec timeout, *tmo;
-  NSAutoreleasePool *pool = nil;
-
-  /* NSTRACE ("fd_handler"); */
-
-  for (;;)
-    {
-      [pool release];
-      pool = [[NSAutoreleasePool alloc] init];
-
-      if (waiting)
-        {
-          fd_set fds;
-          FD_ZERO (&fds);
-          FD_SET (selfds[0], &fds);
-          result = select (selfds[0]+1, &fds, NULL, NULL, NULL);
-          if (result > 0 && read (selfds[0], &c, 1) == 1 && c == 'g')
-           waiting = 0;
-        }
-      else
-        {
-          pthread_mutex_lock (&select_mutex);
-          nfds = select_nfds;
-
-          if (select_valid & SELECT_HAVE_READ)
-            readfds = select_readfds;
-          else
-            FD_ZERO (&readfds);
-
-          if (select_valid & SELECT_HAVE_WRITE)
-            {
-              writefds = select_writefds;
-              wfds = &writefds;
-            }
-          else
-            wfds = NULL;
-          if (select_valid & SELECT_HAVE_TMO)
-            {
-              timeout = select_timeout;
-              tmo = &timeout;
-            }
-          else
-            tmo = NULL;
-
-          pthread_mutex_unlock (&select_mutex);
-
-          FD_SET (selfds[0], &readfds);
-          if (selfds[0] >= nfds) nfds = selfds[0]+1;
-
-          result = pselect (nfds, &readfds, wfds, NULL, tmo, NULL);
-
-          if (result == 0)
-            ns_send_appdefined (-2);
-          else if (result > 0)
-            {
-              if (FD_ISSET (selfds[0], &readfds))
-                {
-                  if (read (selfds[0], &c, 1) == 1 && c == 's')
-                   waiting = 1;
-                }
-              else
-                {
-                  pthread_mutex_lock (&select_mutex);
-                  if (select_valid & SELECT_HAVE_READ)
-                    select_readfds = readfds;
-                  if (select_valid & SELECT_HAVE_WRITE)
-                    select_writefds = writefds;
-                  if (select_valid & SELECT_HAVE_TMO)
-                    select_timeout = timeout;
-                  pthread_mutex_unlock (&select_mutex);
-
-                  ns_send_appdefined (result);
-                }
-            }
-          waiting = 1;
-        }
-    }
-}
-
 
 
 /* ==========================================================================
@@ -6361,7 +6118,7 @@ - (void)mouseMoved: (NSEvent *)e
                       help_echo_object, help_echo_pos);
     }
 
-  if (emacsframe->mouse_moved && send_appdefined)
+  if (emacsframe->mouse_moved)
     ns_send_appdefined (-1);
 }
 
@@ -7058,8 +6815,7 @@ - (void)windowDidExpose: sender
   SET_FRAME_VISIBLE (emacsframe, 1);
   SET_FRAME_GARBAGED (emacsframe);
 
-  if (send_appdefined)
-    ns_send_appdefined (-1);
+  ns_send_appdefined (-1);
 }
 
 
-- 

Here we go. This seems to work. It turns out there are trade‐offs
between the GUI and network performance. If I remove
NSApp:nextEventMatchingMask from ns_select, eww loads web pages
*significantly* faster, but the scrollbars become effectively
unusable.

-- 
Alan Third