Sign up ×
Arduino Stack Exchange is a question and answer site for developers of open-source hardware and software that is compatible with Arduino. It's 100% free, no registration required.

The following snippets are from TimerOne library source code:

// TimerOne.h:
void (*isrCallback)();

// TimerOne.cpp:
ISR(TIMER1_OVF_vect) // interrupt service routine that wraps a user defined function supplied by attachInterrupt
{
  Timer1.isrCallback();
}

// TimerOne.cpp:
void TimerOne::attachInterrupt(void (*isr)(), long microseconds)
{
  if(microseconds > 0) setPeriod(microseconds);
  isrCallback = isr; // register the user's callback with the real ISR
  TIMSK1 = _BV(TOIE1); // sets the timer overflow interrupt enable bit
  resume();                                                                                            
}

The question: if the timer is already running, and the main program calls attachInterrupt(), could the timer interrupt occur there during the function pointer assignment isrCallback = isr;? Then, with lucky timing, Timer1.isrCallback(); function pointer would consist partly of the old and partly of the new address, causing the ISR jump to a bogus location?

I suppose this might be the case, since function pointers are certainly wider than 1 byte, and accessing > 1 byte data isn't atomic. Possible workarounds could be:

  • Always call detachInterrupt() to make sure the timer isn't running, before calling attachInterrupt(), i.e. clarify Timer1 docs.
  • Or, modify Timer1, disabling timer overflow interrupts temporarily just before isrCallback = isr;

Does this make sense, or is there something in Timer1 sources or function pointer assignments that I've missed?

share|improve this question

1 Answer 1

up vote 6 down vote accepted

Have a look at the code for attachInterrupt() and detachInterrupt() in /Applications/Arduino.app/Contents/Resources/Java/hardware/arduino/cores/arduino/WInterrupts.c (well, that's where they are on a Mac, anyway. Arduino file structure on other OSes' probably looks similar in the lower levels of the path).

It appears that attachInterrupt() assumes that the interrupt in question is not yet enabled because it writes the function pointer without taking any precautions. Note that detachInterrupts() disables the target interrupt before writing a NULL-pointer to its vector. So I'd at least use a detachInterrupt() / attachInterrupt() pair

I'd want to run any such code in a critical section, myself. It appears your first way (detach, then attach) would work, though I can't be sure it couldn't miss an unfortunately timed interrupt. The datasheet for your MCU might have more to say on that. But neither am I sure at this point, that a global cli()/sei() wouldn't miss it either. The ATMega2560 datasheet, section 6.8, does say "When using the SEI instruction to enable interrupts, the instruction following SEI will be executed before any pending interrupts, as shown in this example", seeming to imply that it can buffer an interrupt while the interrupts are off.

share|improve this answer
    
It's indeed useful to dive to the sources:) TimerOne's attach / detach interrupt mechanism seems to be done similarly as the standard (WInterrupt's) and so has the same "features". –  Joonas Pulakka Jun 3 '14 at 12:17

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.