Forum Rule: Always post complete source code & details to reproduce any issue!
Results 1 to 12 of 12

Thread: Question about IntervalTimer::beginCycles and "ISR safety"

  1. #1
    Member
    Join Date
    Mar 2017
    Location
    Oakland, CA, USA
    Posts
    90

    Question about IntervalTimer::beginCycles and "ISR safety"

    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()?

  2. #2
    Senior Member+ Theremingenieur's Avatar
    Join Date
    Feb 2014
    Location
    Colmar, France
    Posts
    2,534
    Because it's bad style to initialize classes within interrupt routines?

  3. #3
    Member
    Join Date
    Mar 2017
    Location
    Oakland, CA, USA
    Posts
    90
    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.

  4. #4
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    20,576
    Any chance you could craft a test case?

  5. #5
    Member
    Join Date
    Mar 2017
    Location
    Oakland, CA, USA
    Posts
    90
    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().

  6. #6
    Senior Member
    Join Date
    May 2017
    Posts
    208
    Quote Originally Posted by shawn View Post
    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.

  7. #7
    Member
    Join Date
    Mar 2017
    Location
    Oakland, CA, USA
    Posts
    90
    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 by shawn; 11-14-2018 at 02:16 PM.

  8. #8
    Senior Member
    Join Date
    May 2017
    Posts
    208
    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.

  9. #9
    Senior Member+ KurtE's Avatar
    Join Date
    Jan 2014
    Posts
    5,423
    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 ...

  10. #10
    Member
    Join Date
    Mar 2017
    Location
    Oakland, CA, USA
    Posts
    90
    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?

  11. #11
    Senior Member
    Join Date
    May 2017
    Posts
    208
    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.

  12. #12
    Member
    Join Date
    Mar 2017
    Location
    Oakland, CA, USA
    Posts
    90
    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?

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •