Subject: bug#75632: 31.0.50; igc: Crash report

From: Pip Cet <[email-masked]>
Date: Sat, 18 Jan 2025 17:31:57 +0000

"Eli Zaretskii" <[email-masked]> writes:

>> Date: Sat, 18 Jan 2025 14:27:03 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], [email-masked]
>>
>> "Eli Zaretskii" <[email-masked]> writes:
>>
>> >> I was talking about --with-dumping=none, not unexec builds.  I"m not
>> >> fixing unexec bugs!
>> >
>> > OK, so the only interesting use case is running temacs when not
>> > dumping it.
>>
>> I think it's important to keep temacs/emacs without dumping runnable.
>> It's certainly still interesting when temacs is called emacs.
>
> Yes, of course, no argument here.

So (!initialized) it is?  That works for temacs and emacs (I originally
thought it would be wasteful for non-interactive temacs, but I'm sure
someone's using that :-) ).

>> >> Vdebug_on_event points to data behind a memory barrier.  It's replaced
>> >> by a C string which is xstrdup'd, so that's not behind a memory barrier.
>> >> keeping the two in sync is done with a variable watcher, as is the case
>> >> for gc-cons-threshold.
>> >>
>> >> Variable watchers survive dump/reload cycles, so we set it up just once,
>> >> when !initialized; when entering the code in the post-dump state, we
>> >> perform the initial dummy "watch" event to initialize the C string.
>> >
>> > OK, but what does this have to do with SIGUSR handling?
>>
>> The SIGUSR* handler needs to access that string.  Signal handlers cannot
>> access MPS-managed memory, so we need to put the string in regular
>> xmalloc memory.
>
> So this is just to allow the signal handler access to the event name?

So it can determine whether to set some flags, or mark an input signal
pending and set some other flags, yes.

> I'd be much happier if we instead treated this as we treat SIGIO that
> is delivered when the user presses a keyboard key: set a flag saying
> that input is available, and then process it as we do with keyboard.

IIRC, there are some loops that test Vinhibit_quit or Vquit_flag
directly (or use QUITP); we wouldn't be able to break out of one of
those if we only set Vinhibit_quit from what's currently
store_user_signal_events, would we?

If we want to change behavior in this way, we should do it on master,
then merge it into feature/igc.

> The SIGUSR event is eventually entered into the keyboard buffer
> anyway, so why should we treat it so differently from SIGIO?

Vdebug_on_event isn't, AFAICS.  Not a major change to make it so,
though.  I don't know why we eat this event, but we do.

>> I think that's clear from the commit message, by the way.  If it isn't,
>> I'd appreciate advice on how to improve it.
>
> These details are better said in comments to the code than in commit
> log messages.  Looking for explanations for what the code does in Git
> history is less convenient, and I'm quite sure some people will not
> even think about it, at least not at first.

Good point, agreed.

Pip