Description

Whenever something is passed in to a timer that's supposed to be called exactly once (esp. everything that frees up its own resources), pulling that out early is tricky.

Since the introduction of ztimer_is_set this is easier, but the necessary dance is

• disable interrupts
• query ztimer_is_set
• remove ztimer
• enable interrupts
• run the callback if it was actually set

which is not immediately obvious, and likely overkill in terms of interrupt disabling (given that all ztimer functions need to do that too).

AIU this pattern (as well as any integrated form of it that could come of this issue) has the slight quirk that a result of "it was not set" would usually mean it fired, but could also mean that it was fired but did not execute (or did not execute to completion). That's OK for how I'd use it, but needs to be made very clear in the documentation.

The alternative to admitting this possibility would be to make reporting removal unavailable from interrupts -- and that wouldn't be so great.

Roads towards fixing this

• Just make ztimer_remove return whether it did remove something
  • void -> bool API change -- is that OK?
  • Does this increase any constrained parameter? (I have the vague hope that the _is_set result is stored on the stack anyway and that returning it just means it gets put into a different register, but there'd be nontrivial comparisons of assembly involved to check whether that's the case)
• Introduce a ztimer_remove_checking (better name?) function that does ztimer_is_set and remove in a single efficient step

In the course of this I'd also like to look at the use cases for is_set -- I have a gut feeling that there may be cases when users are tempted to use it that are racy, and a "checking removal" would be the better thing to do.

Useful links

• gcoap: Suspected crosstalk between requests (possible NULL call) #14390 would be very hard to resolve without this (it could be resolved now that ztimer_is_set exists ... but hey it'll need porting over to ztimer in the first place)
• http://rustdoc.etonomy.org/riot_wrappers/ztimer/struct.ZTimer.html could offer something useful here (again, it's possible now with ztimer_is_set but why do it the complex way). Rust really insists that if a callback is "call once only", then it be called once only, and for some setups exactly once is a requirement punishable by UB.