Subject: bug#69561: 30.0.50; Freeze from M-x gnus on macOS

From: Gerd Möllmann <[email-masked]>
Date: Thu, 7 Mar 2024 21:23:35 +0100

On 07.03.24 20:29, Gerd Möllmann wrote:
> Alan Third <[email-masked]> writes:
> 
>> On Thu, Mar 07, 2024 at 06:01:02PM +0100, Gerd Möllmann wrote:
>>> Alan Third <[email-masked]> writes:
>>>
>>>> On Thu, Mar 07, 2024 at 05:30:28PM +0100, Gerd Möllmann wrote:
>>>>> Alan Third <[email-masked]> writes:
>>>>>
>>>>>> But keyboard input (ns_read_socket) is handled immediately after that
>>>>>> "if (nfds < 0)" block and well before the "if (no_avail...".
>>>>>
>>>>> Could you please tell the line number?
>>>>
>>>> detect_input_pending_run_timers at process.c:5839 calls
>>>> get_input_pending which calls gobble_input which calls
>>>> t->read_socket_hook.
>>>>
>>>> There seem to be a lot of ways for it to bail out, though.
>>>
>>> Thanks. That's in if (read_kbd), and the first backtrace I sent had
>>>
>>>      frame #6: 0x00000001001d94d2
>>> emacs`wait_reading_process_output(time_limit=<unavailable>,
>>> nsecs=<unavailable>, read_kbd=0, do_display=false,
>>> wait_for_cell=(struct Lisp_Symbol *) $123 = 0x00000001007d24b0,
>>> wait_proc=0x00007fccffdcc9d8, just_wait_proc=0) at process.c:5484:9
>>> [opt]
>>>
>>> i.e. read_kbd should be 0.
>>>
>>> Maybe that's also an explanation why it doesn't freeze most of time?
>>> If it sometimes does detect_input_pending...
>>
>> So this
>>
>>     READ_KBD is:
>>       0 to ignore keyboard input, or
>>       1 to return when input is available, or
>>      -1 meaning caller will actually read the input, so don't throw to
>>         the quit handler
>>
>> implies that if read_kbd is zero then we should be able to quit?
> 
> I'm afraid I can't answer that. Maybe Eli can, or knows someone who can?
> 
>> If that's the case then we need some special handling in nsterm.m for
>> C-g, I suppose.
>>
>> Having dug around in other terms I assume this means setting
>> Vquit_flag? So in the keyDown method we should identify C-g and set
>> Vquit_flag...?
> 
> As far as I understand the code, Vquit_flag will also be set by storing
> the event in question with kbd_buffer_store_event_hold. I think that
> would be the easiest way. And it's what ns_read_socket already does.
> 
> In addition, returning -1 from ns_select if events are in the hold
> queue, and not doing anything else, looks super suspicious to me. If
> no-one else does an [NSApp run], like in my freeze, I think it's natural
> that the system says Emacs is not responding. And what dowensides would
> it have to [NSApp run]?
> 
> And finally, I would at least question the raise (SIGIO). I don't
> understand the reason for that if, for example, keyboard input is the
> hold queue. And, of course there is no comment...
> 
> But that just my 2 cents as a newcomer :-).

And I had another freeze a few minutes ago, while starting Emacs, and 
from Eglot starting and doing json_rpc. Backtrace as usual, and it 
happened with the patch I sent before and without.

This time the problem was from something else in ns_select: the "return 
thread_select..." without doing [NSApp run].

With the change below Emacs started without problems. I guess something 
is seriously wrong in ns_select and/or wait_reading_process_output...

1 file changed, 7 insertions(+), 6 deletions(-)
src/nsterm.m | 13 +++++++------

modified   src/nsterm.m
@@ -4739,12 +4739,13 @@ Function modeled after x_draw_glyph_string_box ().
    check_native_fs ();
  #endif

-  if (hold_event_q.nr > 0 && !run_loop_only)
+  /* If there are input events pending, store them so that
+     Emacs can recognize C-g.  */
+  if (hold_event_q.nr > 0)
      {
-      /* We already have events pending.  */
-      raise (SIGIO);
-      errno = EINTR;
-      return -1;
+      for (int i = 0; i < hold_event_q.nr; ++i)
+        kbd_buffer_store_event_hold (&hold_event_q.q[i], NULL);
+      hold_event_q.nr = 0;
      }

    eassert (nfds <= FD_SETSIZE);
@@ -4757,7 +4758,7 @@ Function modeled after x_draw_glyph_string_box ().
    if (NSApp == nil
        || ![NSThread isMainThread]
        || (timeout && timeout->tv_sec == 0 && timeout->tv_nsec == 0))
-    return thread_select (pselect, nfds, readfds, writefds,
+    thread_select (pselect, nfds, readfds, writefds,
  			  exceptfds, timeout, sigmask);
    else
      {