Re: igc, macOS avoiding signals

[Pip Cet]

"Helmut Eller" <eller.helmut@gmail.com> writes:

> On Mon, Dec 30 2024, Gerd Möllmann wrote:
>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>>>> Cc: Helmut Eller <eller.helmut@gmail.com>,  pipcet@protonmail.com,
>>>>   spd@toadstyle.org,  emacs-devel@gnu.org
>>>> Date: Mon, 30 Dec 2024 19:37:38 +0100
>>>>
>>>> So, to summarize, everyone agrees with Helmut?
>
> Except the POSIX police: it says that pthread_mutex_trylock isn't async
> signal safe.  I suppose this also makes it's unsafe to use MPS's fault
> handler in an async signal handler.  Bummer.  (Does the police take
> bribes?)
>
>>> That the SIGPROF handler in the form he described would be safe?  I
>>> agree.
>>
>> What we have in scratch/igc:
> [...]
>> static void
>> add_sample (struct profiler_log *plog, EMACS_INT count)
>> {
>> #ifdef HAVE_MPS
>>   if (igc_busy_p ())
>> #else
>>   if (EQ (backtrace_top_function (), QAutomatic_GC)) /* bug#60237 */
>> #endif
>>     /* Special case the time-count inside GC because the hash-table
>>        code is not prepared to be used while the GC is running.
>>        More specifically it uses ASIZE at many places where it does
>>        not expect the ARRAY_MARK_FLAG to be set.  We could try and
>>        harden the hash-table code, but it doesn't seem worth the
>>        effort.  */
>>     plog->gc_count = saturated_add (plog->gc_count, count);
>>   else
>>     record_backtrace (plog, count);
>> }
> [...]
>>
>> Now the question is if that's what Helmut was describing.
>
> Yes, that's what I meant.
>
> I wonder if the backtrace that we see in the signal handler is any
> different from the backrace that we would see at the next safe point
> (i.e. the next time maybe_quit is called).

If we keep a shadow signal mask, the only requirement for a safe point
is that we made some progress OR the lock was released.  But the
backtrace will change if we wait for the next maybe_quit, IIUC.

maybe_quit is not a great safe point, it's just the best we have.  It's
insufficient if Emacs becomes idle, and how often we call rarely_quit is
quite unpredictable.

> If the backtraces are the same, then we could record the backtrace
> there; that would be much nicer.

I'm still hoping for more useful backtraces.  Those require looking at
the C stack or global variables; I'd prefer not to make the assumption
that the SIGPROF handler is interested only in some words of the specpdl

If modifying MPS is fair game, we could at least eliminate the
igc_busy_p () false positives, on systems nice enough to let us know
which thread holds a locked mutex.  On GNU/Linux, only same-thread
deadlocks are detected with EDEADLK so the distinction can be made.

Pip