Re: igc, macOS avoiding signals

[Gerd Möllmann]

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

> On Mon, Dec 30 2024, Pip Cet wrote:
>
>> "Helmut Eller" <eller.helmut@gmail.com> writes:
>>> Very few of Emacs' signal handlers actually touch a barrier.  I've also
>>
>> Indeed.  These crashes are rare in typical usage, which doesn't mean we
>> should delay fixing them until Emacs is "unstable enough".  It already
>> is, IMHO, because we take that approach too frequently.
>>
>>> not seen any reproducable receipes for the "signal issues" that the igc
>>> branch supposedly has.
>>
>> Removing the SIGPROF protection code should allow Ihor's recipe to crash
>> again.
>
> Talking about SIGPROF protection code.  It appears to me now (again)
> that, for the SIGPROF handler, this pseudo code
>
>   if (mps_arena_busy (<arena>))
>     plog->gc_count = saturated_add (plog->gc_count, count);
>   else
>     record_backtrace (plog, count);
>
> is safe.  If we don't hold the lock, then mps_arena_busy returns false
> and we can access memory.  We are safe even if another thread has
> claimed the lock by the time that we reach record_backtrace: the SIGPROF
> handler will just block until the lock is released.
>
> Does somebody disagree?
>
> Helmut

That's this:

mps_bool_t mps_arena_busy(mps_arena_t arena)
{
  /* Don't call ArenaEnter -- the purpose of this function is to
   * determine if the arena lock is held */
  AVER(TESTT(Arena, arena));
  return ArenaBusy(arena);
}

Bool ArenaBusy(Arena arena)
{
  return LockIsHeld(ArenaGlobals(arena)->lock);
}

Bool (LockIsHeld)(Lock lock)
{
  AVERT(Lock, lock);
  if (pthread_mutex_trylock(&lock->mut) == 0) {
    Bool claimed = lock->claims > 0;
    int res = pthread_mutex_unlock(&lock->mut);
    AVER(res == 0);
    return claimed;
  }
  return TRUE;
}

There might be a small window after pthread_mutex_trylock and being back
in the signal handler. Can anything happen in this window?

If no other Emacs threads are running, and the Emacs thread is in the
signal handler, we can trust the "false" from the mps_arena_busy.

If other threads were running (and I don't think that's currently the case),
the "false" also means that the Emacs thread in the signal handler
can't have the lock.

In the "true" case from mps_arena_busy, we're anyway not doing much.

So I think I agree.