Question about IntervalTimer::beginCycles and "ISR safety"

Status
Not open for further replies.

shawn

Well-known member
I was having a look at IntervalTimer::beginCycles() and noticed that, in theory, there might be a race condition with the channel search.

The scenario:
  1. Call begin(). Internally, beginCycles() finds an open channel, say channel 1, and is about to change channel->TCTRL.
  2. An interrupt happens and in the ISR, another begin() call is made to a different IntervalTimer instance.
  3. Before the ISR ends, it finds the first open channel, say channel 1, and then sets it up.
  4. The original code resumes and now tries to set up channel 1.

I was wondering why the free channel search isn't wrapped with __disable_irq()/__enable_irq()?
 
I'm talking about the begin() call (for example) for one-shot operation, not for continuous operation. In this case, I don't consider a call to begin() as single-time initialization.
 
I'm working on one, but now I need to know, from inside a function, the object or function that called it. Do you have some fancy stack pointer magic you know of that can obtain this information? I'm unaware of a way that the callback can access state from its caller otherwise, and I need this to construct a case where the function pointer is being called from an unexpected container, and also be able to print this information.

My question was a theoretical one, based on how I'm using IntervalTimer, as a one-shot, started from one or more ISR's. I'm currently wrapping any IntervalTimer::begin() calls with __disable_irq()/__enable_irq().
 
My question was a theoretical one, based on how I'm using IntervalTimer, as a one-shot, started from one or more ISR's. I'm currently wrapping any IntervalTimer::begin() calls with __disable_irq()/__enable_irq().

I would suggest that if you are operating in an interrupt context, that there is no need to wrap calls to begin() as interrupts are already locked out due to you being inside an interrupt function. I would further suggest that using _enable_irq() inside an interrupt is a bad idea as you will be enabling other interrupts prematurely( as they will be enabled normally when you exit the interrupt routine ). You could actually produce the type of race conditions you are trying to avoid.

To avoid the condition that your original question asks, you would only need to disable interrupts on any call to begin that is in your normal loop processing.
 
In normal code, I'm already wrapping disable/enable IRQ's around any calls to begin(). I'm doing this because there's still a chance that a begin() call in normal code will get preempted by an ISR also calling begin(), but on a different instance of IntervalTimer, causing the potential race condition I mentioned in my first post.

This brings me to your point about __enable_irq() being called from an ISR. I'd like to clarify my understanding of this and its companion function, __disable_irq(). I always thought that these functions simply turned on and off the ability for maskable interrupts to happen. i.e. when you call __disable_irq() from inside an ISR, no other interrupts can happen, even higher-priority ones, but when you call __enable_irq() to restore state, it just re-enables higher-priority interrupts, which were already enabled to begin with. Isn't it the case that higher-priority interrupts are not locked out to begin with? Or is that incorrect?

Unless I'm misunderstanding entirely, and higher-priority interrupts can't interrupt already-running lower-priority ISR's?

[Update] I re-read https://www.pjrc.com/teensy/interrupts.html and apparently, unless very special actions are taken, "nested interrupts" don't usually happen. I guess I won't spend too much time trying to guard against this case. I had been assuming this was normal. :(
 
Last edited:
Yes you are probably correct in the case of the Teensy. One of the features of the Arduino programming environment is that the user is sheltered from the inner workings of the processor. My write up is simplistic and applies more to a processor with a single interrupt priority.

Still when in the processing of the interval timer, another instance cannot interrupt that processing as it would have the same priority and be masked out. So wrapping the call to begin is not necessary, is redundant in your case, wastes processing time; but may not cause any harm depending upon what the underlying hardware has for bits to mask interrupts and if a global flag is available to disable all levels of interrupts.
 
I could be wrong, but I believe that the link https://www.pjrc.com/teensy/interrupts.html is more setup for the 8 bit AVR boards than the newer Teensy boards. Yes higher priority interrupts can happen (nested).

Would not surprise me that the some of this code is not fully interrupt safe. Which is why I try to avoid doing things like that on an ISR, likewise do things like allocate memory or ...
 
So if higher priority interrupts can happen within an ISR, why is calling __enable_irq() from inside it (after calling __disable_irq()) incorrect? Wouldn't that just re-enable higher-priority interrupts? I guess my question is, then: Are higher-priority interrupts disabled in the Teensy firmware when inside an ISR?
 
Paul is a pretty sharp guy and I suspect he has implemented the interrupt priority levels correctly on the Teensy's.
While in an ISR, any higher level ISR can interrupt. Same level or lower ISR will wait.
I believe I didn't say anything was incorrect, but said generally it is not a good idea and on some processors could lead to trouble. I did try to point out that wrapping the call to begin with _disable_irq() and _enable_irq() while in an ISR does not perform the function you originally intended and is unnecessary.
I think you understand how it works and sorry for any confusion.
 
No worries. Interrupts and concurrency are hard. :)

So it sounds like __enable_irq() is indeed appropriate in an ISR if __disable_irq() was called first?
 
Status
Not open for further replies.
Back
Top