Re: MPS: a random backtrace while toying with gdb

[Pip Cet]

On Wednesday, July 3rd, 2024 at 11:50, Eli Zaretskii <eliz@gnu.org> wrote:
> > Date: Wed, 03 Jul 2024 11:31:48 +0000
> 
> > From: Pip Cet pipcet@protonmail.com
> > Cc: Eli Zaretskii eliz@gnu.org, eller.helmut@gmail.com, 
> > gerd.moellmann@gmail.com, yantar92@posteo.net, emacs-devel@gnu.org
> > 
> > So what are the proposed solutions that are still on the table? I can think 
> > of these:
> > 
> > 1. block signals around MPS calls and block signals while the MPS SIGSEGV 
> > handler is running
> > 2. handle signals in a special thread
> > 3. switch Emacs to an event loop model and handle all signals asynchronously
> 
> 
> I'd start with the first half of (1). It is not clear to me that the
> other part is needed, and in any case we need a reproducer for it
> first.

Ihor's original SIGPROF-based reproducer works if you revert the 
(SIGPROF-specific) workaround by Helmut. What makes you think it can't happen 
with other signals (which, naturally, aren't as frequent or badly-timed as 
SIGPROF, which strikes precisely when the CPU is active)?

Obviously a reproducer is highly desirable in this case, but we shouldn't leave 
known and understood bugs in the code.

> Most of the crashes we've seen until now were not when the MPS
> handler was running. Also, didn't someone say that when the MPS

Ihor's first crash backlog (MPS: profiler) clearly shows that was the case: 
https://lists.gnu.org/archive/html/emacs-devel/2024-06/msg00568.html

> SIGSEGV handler is active, we could detect that from our code and
> return doing nothing?

Dropping the signal in the process.

> > (1) is hard to do because we'd need to hijack the SIGSEGV handler and/or 
> > modify MPS
> > (3) is a very major change which would mean permanently losing features we 
> > want
> > (2) is very easy to do, but gets complicated for SIGPROF (which needs to be 
> > on the thread it's profiling) and emergency breakout signals which need to 
> > modify the main thread's state.
> > 
> > I've tried (2) and it fixes the specific reproducer I posted yesterday, as 
> > it should do in theory. I believe it's the most flexible approach which 
> > still gives us well-defined semantics for signal handlers. We'd need to add 
> > per-thread signals for the specific cases mentioned above. And, of course, 
> > its pthread-specific.
> 
> Doing (2) adds a whole lot of complexity to Emacs.

You're right.

> Most importantly,
> we will be unable to access Lisp data safely, unwind-protect and the
> entire specpdl stuff generally cannot be used, and signaling an error
> would be fatal. So I'd rather avoid that.

Very good points, though I wonder to what extent our current code is safe...

Pip