GNU bug report logs - #55628
debug-timer-check fails on macOS 11.6.6

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 55628 in the body.
You can then email your comments to 55628 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Mattias Engdegård <mattiase <at> acm.org>
To: bug-gnu-emacs <at> gnu.org
Subject: debug-timer-check fails on macOS 11.6.6
Date: Wed, 25 May 2022 12:46:39 +0200

Built with enable-checking, (debug-timer-check) returns nil.
It turns out that debug_timer_callback is never actually called, but I haven't dug further than that.

This is on macOS 11.6.6 and HAVE_TIMERFD is not defined. Does this code path work on other systems?

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: 55628 <at> debbugs.gnu.org
Subject: Re: bug#55628: debug-timer-check fails on macOS 11.6.6
Date: Wed, 25 May 2022 14:20:07 +0200

Mattias Engdegård <mattiase <at> acm.org> writes:

> Built with enable-checking, (debug-timer-check) returns nil.
> It turns out that debug_timer_callback is never actually called, but I
> haven't dug further than that.
>
> This is on macOS 11.6.6 and HAVE_TIMERFD is not defined. Does this
> code path work on other systems?

I tried this on Debian/bookworm (with nativecomp, but that probably
doesn't make a difference), and I got nil there, too.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Mattias Engdegård <mattiase <at> acm.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 55628 <at> debbugs.gnu.org
Subject: Re: bug#55628: debug-timer-check fails on macOS 11.6.6
Date: Wed, 25 May 2022 15:05:32 +0200

[Message part 1 (text/plain, inline)]

25 maj 2022 kl. 14.20 skrev Lars Ingebrigtsen <larsi <at> gnus.org>:
> 
> Mattias Engdegård <mattiase <at> acm.org> writes:
> 
>> Built with enable-checking, (debug-timer-check) returns nil.
>> It turns out that debug_timer_callback is never actually called, but I
>> haven't dug further than that.
>> 
>> This is on macOS 11.6.6 and HAVE_TIMERFD is not defined. Does this
>> code path work on other systems?
> 
> I tried this on Debian/bookworm (with nativecomp, but that probably
> doesn't make a difference), and I got nil there, too.

Thanks for checking. Apparently pending_signals is never set; handle_alarm_signal is never called.
More digging shows that set_alarm always sets the interval timer to at least 1 ms in the future even if the timer has expired.

The attached patch makes set_alarm signal SIGALRM immediately if the timer is due. We could call the signal handler directly but this way it should work correctly if the signal is masked at the moment. What do you think?

[set_alarm.diff (application/octet-stream, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: 55628 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#55628: debug-timer-check fails on macOS 11.6.6
Date: Wed, 25 May 2022 17:02:03 +0300

> Cc: 55628 <at> debbugs.gnu.org
> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Wed, 25 May 2022 15:05:32 +0200
> 
> Thanks for checking. Apparently pending_signals is never set; handle_alarm_signal is never called.
> More digging shows that set_alarm always sets the interval timer to at least 1 ms in the future even if the timer has expired.
> 
> The attached patch makes set_alarm signal SIGALRM immediately if the timer is due. We could call the signal handler directly but this way it should work correctly if the signal is masked at the moment. What do you think?

I'd prefer not to change the logic of set_alarm just to have a
debugging aid working.  Can we come up with a more compatible change,
which doesn't affect calls to set_alarm except during this test?

Thanks.

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

From: Mattias Engdegård <mattiase <at> acm.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 55628 <at> debbugs.gnu.org
Subject: Re: bug#55628: debug-timer-check fails on macOS 11.6.6
Date: Wed, 25 May 2022 17:34:57 +0200

25 maj 2022 kl. 15.05 skrev Mattias Engdegård <mattiase <at> acm.org>:

> More digging shows that set_alarm always sets the interval timer to at least 1 ms in the future even if the timer has expired.

In fact it was even worse than that: because of a second bug, the timer was then immediately reset to at least 1 s in the future. The proposed patch fixes both flaws.

Good to have a test for it -- maybe it should always be enabled, not just with enable-checking? People don't always reconfigure, rebuild and re-run the tests with enable-checking, nor do any of the build bots.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: 55628 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#55628: debug-timer-check fails on macOS 11.6.6
Date: Wed, 25 May 2022 20:11:16 +0300

> Resent-From: Mattias Engdegård <mattiase <at> acm.org>
> Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
> Resent-CC: bug-gnu-emacs <at> gnu.org
> Resent-Sender: help-debbugs <at> gnu.org
> Cc: 55628 <at> debbugs.gnu.org
> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Wed, 25 May 2022 17:34:57 +0200
> 
> 25 maj 2022 kl. 15.05 skrev Mattias Engdegård <mattiase <at> acm.org>:
> 
> > More digging shows that set_alarm always sets the interval timer to at least 1 ms in the future even if the timer has expired.
> 
> In fact it was even worse than that: because of a second bug, the timer was then immediately reset to at least 1 s in the future. The proposed patch fixes both flaws.

Btw, this test will never work on a platform that only has setitimer,
because only one ITIMER_REAL timer is guaranteed to exist, and the
function attempts to arm 10 timers at the same time.

> Good to have a test for it -- maybe it should always be enabled, not just with enable-checking? People don't always reconfigure, rebuild and re-run the tests with enable-checking, nor do any of the build bots.

We don't usually have testing code in a production build, so why make
an exception here?  Especially since atimers are an obscure feature
used internally by Emacs.

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

From: Mattias Engdegård <mattiase <at> acm.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 55628 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#55628: debug-timer-check fails on macOS 11.6.6
Date: Thu, 26 May 2022 11:57:58 +0200

25 maj 2022 kl. 19.11 skrev Eli Zaretskii <eliz <at> gnu.org>:

> Btw, this test will never work on a platform that only has setitimer,
> because only one ITIMER_REAL timer is guaranteed to exist, and the
> function attempts to arm 10 timers at the same time.

That's not how the code works. `setitimer` will only be called for the first timer to expire.
(The test works correctly on this computer, that only has setitimer, after applying the patch.)

Another demonstration of this bug:

(funcall-with-delayed-message
 0.1 "haha" (lambda () (sit-for 3)))

will, on this machine, display "haha" after 1 s if unpatched, but after 0.1 s if patched.

(This is with -nw because the NS backend seems to be slow handling async events on its own, but that's a different bug.)

> We don't usually have testing code in a production build, so why make
> an exception here?  Especially since atimers are an obscure feature
> used internally by Emacs.

There's nothing bad or even unusual about including interfaces for internal testing in production builds, as long as it doesn't unduly reduce performance or cause significant bloat. It's always a trade-off, of course.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: 55628 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#55628: debug-timer-check fails on macOS 11.6.6
Date: Thu, 26 May 2022 14:37:15 +0300

> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Thu, 26 May 2022 11:57:58 +0200
> Cc: larsi <at> gnus.org, 55628 <at> debbugs.gnu.org
> 
> 25 maj 2022 kl. 19.11 skrev Eli Zaretskii <eliz <at> gnu.org>:
> 
> > Btw, this test will never work on a platform that only has setitimer,
> > because only one ITIMER_REAL timer is guaranteed to exist, and the
> > function attempts to arm 10 timers at the same time.
> 
> That's not how the code works. `setitimer` will only be called for the first timer to expire.

Maybe I've misread the code, then.  Apologies.

> > We don't usually have testing code in a production build, so why make
> > an exception here?  Especially since atimers are an obscure feature
> > used internally by Emacs.
> 
> There's nothing bad or even unusual about including interfaces for internal testing in production builds, as long as it doesn't unduly reduce performance or cause significant bloat. It's always a trade-off, of course.

Not in principle, but we in this project don't do that.  E.g., there
are several debug-related functions and commands in xdisp.c that are
only compiled in under ENABLE_CHECKING.

So if we are going to make an exception for this case, we should have
some good reason.

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

From: Mattias Engdegård <mattiase <at> acm.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 55628 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#55628: debug-timer-check fails on macOS 11.6.6
Date: Thu, 26 May 2022 15:15:56 +0200

[Message part 1 (text/plain, inline)]

26 maj 2022 kl. 13.37 skrev Eli Zaretskii <eliz <at> gnu.org>:

> Not in principle, but we in this project don't do that.  E.g., there
> are several debug-related functions and commands in xdisp.c that are
> only compiled in under ENABLE_CHECKING.

I don't think that not having done it in the past is justification enough for not doing it now, but I agree that it would be preferable to have it covered by a test that doesn't need a special interface.

It looks like funcall-with-delayed-message could be used for this purpose. I came up with the attached test, which also has the benefit of extending test coverage to that function which previously had none.

[funcall-with-delayed-message-test.diff (application/octet-stream, attachment)]

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

From: Mattias Engdegård <mattiase <at> acm.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 55628 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#55628: debug-timer-check fails on macOS 11.6.6
Date: Mon, 30 May 2022 12:43:48 +0200

[Message part 1 (text/plain, inline)]

Attached is the final patch (just the two previously combined with a commit message).

Would someone please double-check that it works on a platform other than macOS before I push it?

[0001-Fix-atimer-setting-and-overdue-expiration-bug-55628.patch (application/octet-stream, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: 55628 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#55628: debug-timer-check fails on macOS 11.6.6
Date: Mon, 30 May 2022 15:42:53 +0200

Mattias Engdegård <mattiase <at> acm.org> writes:

> Attached is the final patch (just the two previously combined with a
> commit message).
>
> Would someone please double-check that it works on a platform other
> than macOS before I push it?

Works for me on Debian/bullseye (that is, no compilation warnings and
the tests pass).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #40 received at 55628-done <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 55628-done <at> debbugs.gnu.org
Subject: Re: bug#55628: debug-timer-check fails on macOS 11.6.6
Date: Mon, 30 May 2022 15:55:53 +0200

30 maj 2022 kl. 15.42 skrev Lars Ingebrigtsen <larsi <at> gnus.org>:

> Works for me on Debian/bullseye (that is, no compilation warnings and
> the tests pass).

Thank you, now pushed to master.

Previous Next

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