bug#65837: 30.0.50; Debugger in non-main threads

bug#65837: 30.0.50; Debugger in non-main threads

[Helmut Eller]

When the debugger is entered in a non-main thread, like so

  emacs -Q --eval '(make-thread (lambda() (debug)))'

Then debugger-quit doesn't work as usual, instead it prints

  debugger-quit: No catch for tag: top-level, nil

or it prints

  Back to top level

but doesn't actually close the debugger window.

Also, debugger-continue doesn't work as usual, instead it sometimes prints:

  debugger-continue: No catch for tag: exit, nil

sometimes it prints:

  Continuing.

without deleting the debugger window, sometimes it manages to leave the
debugger.

If Emacs is started with "emacs -Q -nw" then the behaviour seems to be a
bit more uniform, but equally useless.

Helmut


In GNU Emacs 30.0.50 (build 192, x86_64-pc-linux-gnu, GTK+ Version
 3.24.37, cairo version 1.16.0) of 2023-09-09 built on caladan
Repository revision: e865ee05d7f974bace724dd241a311c753006933
Repository branch: interruptible-condition-wait
Windowing system distributor 'The X.Org Foundation', version 11.0.12101007
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
 'configure --enable-checking=yes --with-xpm=ifavailable
 --with-gif=ifavailable 'CFLAGS=-g -O1''

Configured features:
CAIRO DBUS FREETYPE GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG LIBSELINUX
LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG SECCOMP SOUND
SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 GTK3
ZLIB

bug#65837: 30.0.50; Debugger in non-main threads

[Eli Zaretskii]

> From: Helmut Eller <eller.helmut@gmail.com>
> Date: Sat, 09 Sep 2023 11:15:52 +0200
> 
> When the debugger is entered in a non-main thread, like so
> 
>   emacs -Q --eval '(make-thread (lambda() (debug)))'
> 
> Then debugger-quit doesn't work as usual, instead it prints
> 
>   debugger-quit: No catch for tag: top-level, nil
> 
> or it prints
> 
>   Back to top level
> 
> but doesn't actually close the debugger window.
> 
> Also, debugger-continue doesn't work as usual, instead it sometimes prints:
> 
>   debugger-continue: No catch for tag: exit, nil
> 
> sometimes it prints:
> 
>   Continuing.
> 
> without deleting the debugger window, sometimes it manages to leave the
> debugger.
> 
> If Emacs is started with "emacs -Q -nw" then the behaviour seems to be a
> bit more uniform, but equally useless.

Yes, error handling in non-main threads is generally rudimentary and
barely useful.  For example, if a thread signals an error, it simply
silently exits and leaves the error form in a variable that can be
accessed via thread-last-error.  AFAIR, it is not trivial to improve
the thread error handling significantly, but patches are welcome, of
course.

bug#65837: 30.0.50; Debugger in non-main threads

[Helmut Eller]

On Sat, Sep 09 2023, Eli Zaretskii wrote:

> AFAIR, it is not trivial to improve
> the thread error handling significantly, but patches are welcome, of
> course.

Is there a design/plan for how recursive-edit is supposed to work in
non-main threads?

E.g. some options that come to mind:

1) It should be allowed without restriction (what currently seems to be
happening).

2) There should be some locking/multiplexing scheme so that only some
"foreground thread" is allowed to read events from the keyboard.

3) Only the main thread is allowed to call recursive-edit; all other
threads have to communicate with the main thread in some way.  (Maybe
there should be one thread per terminal that runs a command loop, but
that's an exotic detail.)

Helmut

bug#65837: 30.0.50; Debugger in non-main threads

[Eli Zaretskii]

> From: Helmut Eller <eller.helmut@gmail.com>
> Cc: 65837@debbugs.gnu.org
> Date: Sat, 09 Sep 2023 17:35:22 +0200
> 
> On Sat, Sep 09 2023, Eli Zaretskii wrote:
> 
> > AFAIR, it is not trivial to improve
> > the thread error handling significantly, but patches are welcome, of
> > course.
> 
> Is there a design/plan for how recursive-edit is supposed to work in
> non-main threads?

Not that I know of, no.  But you seem to be asking mainly about
reading events, not about recursive-edit?

> E.g. some options that come to mind:
> 
> 1) It should be allowed without restriction (what currently seems to be
> happening).

Almost: the keyboard input is processed by the thread which "grabs"
it.  AFAIR, this is usually the main thread, but it is not enforced.

> 2) There should be some locking/multiplexing scheme so that only some
> "foreground thread" is allowed to read events from the keyboard.
> 
> 3) Only the main thread is allowed to call recursive-edit; all other
> threads have to communicate with the main thread in some way.  (Maybe
> there should be one thread per terminal that runs a command loop, but
> that's an exotic detail.)

User interaction when multiple threads are present is an issue we
didn't figure out.  I think based on past discussions it is quite
clear that some kind of protocol or discipline is needed to avoid
creating a terrible mess whereby the user could even completely lose
the ability to interact, but it is not clear what that protocol should
be and how it would work.

At least the two alternatives you listed above each have problems that
need to be resolved.  The main issues, AFAIR, are:

  . if only one thread can read input, what do other threads do when
    they want to ask the user some question? if they should wait, then
    this should be somehow incorporated in the thread_select machinery
  . how would the user know which thread prompts him/her? (this is
    sometimes important)
  . what about just displaying messages, without any input -- should
    that be allowed from any thread, or should there be
    synchronization here as well? what about redisplay in general?

This all is exacerbated by two important factors:

  . there's no scheduler between threads, they basically schedule
    themselves, so there's no entity besides the threads themselves to
    implement whatever protocols are needed for input multiplexing
  . Emacs doesn't have a separate input mechanism: input from
    minibuffer just reuses the normal editing and display
    capabilities, so whatever we do about multiple threads will
    directly affect these general-purpose capabilities

Feel free to suggest possible solutions and ideas, just not here, on
emacs-devel.

bug#65837: 30.0.50; Debugger in non-main threads

[Helmut Eller]

On Sat, Sep 09 2023, Eli Zaretskii wrote:

>> Is there a design/plan for how recursive-edit is supposed to work in
>> non-main threads?
>
> Not that I know of, no.  But you seem to be asking mainly about
> reading events, not about recursive-edit?

Well, the debugger calls recursive-edit and if it's not clear what
recursive-edit is supposed to do, then replacing recursive-edit with
something more reliable would be my first step to improving the
debugger.

Helmut

bug#65837: 30.0.50; Debugger in non-main threads

[Eli Zaretskii]

> From: Helmut Eller <eller.helmut@gmail.com>
> Cc: 65837@debbugs.gnu.org
> Date: Sat, 09 Sep 2023 18:23:29 +0200
> 
> On Sat, Sep 09 2023, Eli Zaretskii wrote:
> 
> >> Is there a design/plan for how recursive-edit is supposed to work in
> >> non-main threads?
> >
> > Not that I know of, no.  But you seem to be asking mainly about
> > reading events, not about recursive-edit?
> 
> Well, the debugger calls recursive-edit and if it's not clear what
> recursive-edit is supposed to do, then replacing recursive-edit with
> something more reliable would be my first step to improving the
> debugger.

If recursive-edit is run by the same thread which was running before
the debugger was entered, and the debugger never calls one of the
primitives that enter the idle loop (sit-for etc.), then I don't think
problems will happen, because it basically means the thread which
entered the debugger keeps running.  But if the debugger causes its
thread to yield, some other thread could grab the global lock, and
then all hell will break loose.

So I think the only reasonably practical way to make this particular
use case stable is to disable thread switching for as long as the
debugger is active.  WDYT?

bug#65837: 30.0.50; Debugger in non-main threads

[Helmut Eller]

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

On Sat, Sep 09 2023, Eli Zaretskii wrote:

> So I think the only reasonably practical way to make this particular
> use case stable is to disable thread switching for as long as the
> debugger is active.  WDYT?

Having to disable thread switching for so long would be quite
unfortunate.

I think that is safe for the debuggee to call condition-wait or
accept-process-output on a specific pipe process.  That should be enough
to implement some form of channel/mailbox to communicate between the
debuggee and the main thread.  The debuggee would then read commands
from that channel and the main thread would send messages to that
channel instead of executing the commands.  Something like the patch in
the attachment.


[-- Attachment #2: debug-patch.el --]
[-- Type: text/plain, Size: 1775 bytes --]

--- /scratch/emacs/emacs-git/lisp/emacs-lisp/debug.el	2023-01-29 15:26:37.886911531 +0100
+++ debug.el	2023-09-09 19:20:49.039995072 +0200
@@ -283,7 +283,7 @@
 		  (message "")
 		  ;; Make sure we unbind buffer-read-only in the right buffer.
 		  (save-excursion
-		    (recursive-edit))))
+                    (debugger--recursive-edit))))
 	    (when (and (window-live-p debugger-window)
 		       (eq (window-buffer debugger-window) debugger-buffer))
 	      ;; Record height of debugger window.
@@ -314,6 +314,24 @@
 	    (set-match-data debugger-outer-match-data)))
         (setq debug-on-next-call debugger-step-after-exit)
         debugger-value))))
+
+(require 'channel)
+(defvar-local debugger--channel nil)
+(defvar-local debugger--thread nil)
+
+(defun debugger--recursive-edit ()
+  (setq debugger--thread (current-thread))
+  (cond ((eq (current-thread) main-thread)
+         (recursive-edit))
+        (t
+         (let ((ch (channel-make-channel 0)))
+           (setq debugger--channel (cdr ch))
+           (while t
+             (pcase-exhaustive (channel-receive (car ch))
+               ('(quit)
+                (message "quit received...")
+                (debugger-quit))))))))
+
 \f
 (defun debugger--print (obj &optional stream)
   (condition-case err
@@ -755,9 +773,12 @@
 (defun debugger-quit ()
   "Quit debugging and return to the top level."
   (interactive)
-  (if (= (recursion-depth) 0)
-      (quit-window)
-    (top-level)))
+  (cond ((eq (current-thread) debugger--thread)
+         (if (= (recursion-depth) 0)
+             (quit-window)
+           (top-level)))
+        (t
+         (channel-send debugger--channel 'quit nil))))
 
 (defun debug--implement-debug-watch (symbol newval op where)
   "Conditionally call the debugger.

[-- Attachment #3: Type: text/plain, Size: 8 bytes --]


Helmut

bug#65837: 30.0.50; Debugger in non-main threads

[Eli Zaretskii]

> From: Helmut Eller <eller.helmut@gmail.com>
> Cc: 65837@debbugs.gnu.org
> Date: Sat, 09 Sep 2023 19:24:31 +0200
> 
> On Sat, Sep 09 2023, Eli Zaretskii wrote:
> 
> > So I think the only reasonably practical way to make this particular
> > use case stable is to disable thread switching for as long as the
> > debugger is active.  WDYT?
> 
> Having to disable thread switching for so long would be quite
> unfortunate.

But not unheard of.  When you debug a program, you almost always want
the other threads stopped (unless the bug you are investigating
happens because of the other threads).

> I think that is safe for the debuggee to call condition-wait or
> accept-process-output on a specific pipe process.  That should be enough
> to implement some form of channel/mailbox to communicate between the
> debuggee and the main thread.  The debuggee would then read commands
> from that channel and the main thread would send messages to that
> channel instead of executing the commands.  Something like the patch in
> the attachment.

I don't see how you could implement that in a portable way.  I also
don't understand how you will support all the different input event
types using this "channel".  And finally, you prevent the main thread
from running "normally", because it is now busy serving the debuggee
thread -- which seems to contradict your desire not to interfere with
other threads.

Preventing thread switch is definitely orders of magnitude simpler,
IMO.

bug#65837: 30.0.50; Debugger in non-main threads

[Helmut Eller]

On Sat, Sep 09 2023, Eli Zaretskii wrote:

> Preventing thread switch is definitely orders of magnitude simpler,
> IMO.

Well, then I'll wait until you fix this bug.

Helmut

bug#65837: 30.0.50; Debugger in non-main threads

[Eli Zaretskii]

> From: Helmut Eller <eller.helmut@gmail.com>
> Cc: 65837@debbugs.gnu.org
> Date: Sat, 09 Sep 2023 20:29:15 +0200
> 
> On Sat, Sep 09 2023, Eli Zaretskii wrote:
> 
> > Preventing thread switch is definitely orders of magnitude simpler,
> > IMO.
> 
> Well, then I'll wait until you fix this bug.

Fair enough.

Can you describe the bug, though?  What exactly happens that is wrong,
and how to reproduce it?

bug#65837: 30.0.50; Debugger in non-main threads

[Helmut Eller]

On Sat, Sep 09 2023, Eli Zaretskii wrote:

> Can you describe the bug, though?  What exactly happens that is wrong,
> and how to reproduce it?

Start the debugger with

  emacs -Q --eval '(make-thread (lambda() (debug)))'

The bug is that pressing q doesn't actually delete the *Backtrace*
window.  The expected behavior in that the window should be deleted and
the buffer should be killed.

Helmut