GNU bug report logs - #75632
31.0.50; igc: Crash report

Previous Next

Full log

Message #70 received at 75632 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: yantar92 <at> posteo.net, 75632 <at> debbugs.gnu.org
Subject: Re: bug#75632: 31.0.50; igc: Crash report
Date: Sun, 19 Jan 2025 12:38:49 +0200

> Date: Sun, 19 Jan 2025 09:44:26 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: 75632 <at> debbugs.gnu.org, yantar92 <at> posteo.net
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> > OK, but could we at least make this somewhat cleaner?  Instead of
> 
> I see no reason for why the old code behaved as it did, so I'm proposing
> this:
> 
> 1. doc change: don't use "special event" to mean two totally different
> things in process_special_events and special_event_name; remove
> special_event_name.
> 
> 2. when debug-on-event is modified/called, set a per-signal flag in the
> user_signal_info struct, saving us a strcmp.
> 
> 3. Add a watcher as on feature/igc to reset the flags of all user
> signals.

Can we add the watcher in Lisp instead?  Are there any reasons to have
it in C and at temacs time?

Also, perhaps make the function debug-on-event call the watcher, not
itself be the watcher.  I think that might make the API cleaner.  The
watcher function doesn't have to be exposed to Lisp, btw.

> 4. Make that watcher callable as Fdebug_on_event directly.
> 
> I stopped there.  I'm confused about input_available_clear_time: it's
> only cleared if we don't enter the debugger.  Adding the code to clear
> it to the debug_on_event case might help us break out of some broken
> pselects, and it shouldn't ever hurt us (famous last words).
> 
> But maybe I totally misunderstood what you suggested, too.

No, I think you understood me.

> +DEFUN ("debug_on_event", Fdebug_on_event, Sdebug_on_event, 1, 4, 0,
> +       doc: /* Make EVENT enter the debugger.
> +
> +EVENT should be a symbol, either sigusr1 or sigusr2.
> +
> +This function supports an additional calling convention to be usable as
> +a variable watcher.  */)
> +  (Lisp_Object arg_or_symbol, Lisp_Object newval, Lisp_Object operation,
> +   Lisp_Object where)

There's no EVENT in the function's arguments.  And what are the other
arguments?  The last two seem to be unused?

> +	if (p->debug_on_event)
>            {
>              /* Enter the debugger in many ways.  */
>              debug_on_next_call = true;
>              debug_on_quit = true;
>              Vquit_flag = Qt;
>              Vinhibit_quit = Qnil;
> -
> -            /* Eat the event.  */
> -            break;

Why did you remove this last part?  With the change to a flag, we no
longer have a problem that requires delaying the event processing,
right?  So why risk the change in behavior, something that you warned
against when I suggested the same?

> Sorry, I ignored this when writing the patch.  I don't think we should
> hardcode any assumptions about signal numbers, and I don't see another
> readily available integer we can use

I thought about just inventing one.  But that's a moot point if we go
with the flag approach.

> > integers from a signal handler should not be a problem, right?  (In
> > retrospect, we should have probably made debug-on-event a function,
> > then we could produce that number inside the function, thus avoiding
> > the need for watching the variable.  Maybe we should introduce the
> > function now and deprecate the variable?)
> 
> I've introduced a function, so you can call debug-on-event or set it,
> but I haven't deprecated the variable yet, because there's no way to
> read the value from Lisp otherwise.

Yes, good.  We should also deprecate the usage of the variable, in the
doc string and in the manual.

> With this change, we still have six (!) debug-on-* variables, and two
> additional debug-on-* functions.  They all behave slightly differently.
> 
> Is this at all what you meant?  I realize it's incomplete, but it's
> probably worth it to do things properly.

Yes, that's going in the right direction, I think.  Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.