#41 Ensure events set the running state properly, ensure all possible interrupt points use ISR_ATTR

Author: davetcc

src/TaskManagerIO.cpp

@@ -186,6 +186,21 @@ void TaskManager::yieldForMicros(uint32_t microsToWait) {
 	tm_internal::atomicWritePtr(&runningTask, prevTask);
 }
 
+class TaskExecutionRecorder {
+private:
+    TimerTask* prevTask;
+    TaskManager* taskMgr;
+public:
+    TaskExecutionRecorder(TaskManager* tm, TimerTask* task) : taskMgr(tm) {
+        prevTask = tm->getRunningTask();
+        tm_internal::atomicWritePtr(&tm->runningTask, task);
+    }
+
+    ~TaskExecutionRecorder() {
+        tm_internal::atomicWritePtr(&taskMgr->runningTask, prevTask);
+    }
+};
+
 void TaskManager::dealWithInterrupt() {
     interrupted = false;
     if(interruptCallback != nullptr) interruptCallback(lastInterruptTrigger);
@@ -194,16 +209,20 @@ void TaskManager::dealWithInterrupt() {
     for(taskid_t i=0; i<lastSlot; i++) {
         auto* task = getTask(i);
         if(task->isInUse() && task->isEvent()) {
-            tm_internal::atomicWritePtr(&runningTask, task);
-            task->processEvent();
-            tm_internal::atomicWritePtr(&runningTask, nullptr);
-            removeFromQueue(task);
-            if(task->isRepeating()) {
-                putItemIntoQueue(task);
+            if (!task->isRunning()) {
+                TaskExecutionRecorder taskExecutionRecorder(this, task);
+                task->processEvent();
+                removeFromQueue(task);
+                if (task->isRepeating()) {
+                    putItemIntoQueue(task);
+                }
+                else {
+                    task->clear();
+                    tm_internal::tmNotification(tm_internal::TM_INFO_TASK_FREE, TASKMGR_INVALIDID);
+                }
             }
             else {
-                task->clear();
-                tm_internal::tmNotification(tm_internal::TM_INFO_TASK_FREE, TASKMGR_INVALIDID);
+                interrupted = true; // we have to assume we still need to process this event next time around.
             }
         }
     }
@@ -223,9 +242,8 @@ void TaskManager::runLoop() {
             // by here we know that the task is in use. If it's in use nothing will touch it until it's marked as
             // available. We can do this part without a lock, knowing that we are the only thing that will touch
             // the task. We further know that all non-immutable fields on TimerTask are volatile.
-            tm_internal::atomicWritePtr(&runningTask, tm);
+            TaskExecutionRecorder executionRecorder(this, tm);
             tm->execute();
-            tm_internal::atomicWritePtr(&runningTask, nullptr);
             removeFromQueue(tm);
             if (tm->isRepeating()) {
                 putItemIntoQueue(tm);

src/TaskManagerIO.h

@@ -71,6 +71,8 @@ class InterruptAbstraction {
     virtual void attachInterrupt(pintype_t pin, RawIntHandler fn, uint8_t mode) = 0;
 };
 
+class TaskExecutionRecorder;
+
 /**
  * TaskManager is a lightweight cooperative co-routine implementation for Arduino, it works by scheduling tasks to be
  * done either immediately, or at a future point in time. It is quite efficient at scheduling tasks as internally tasks
@@ -187,7 +189,7 @@ class TaskManager {
      * Generally used by the BaseEvent class in the markTriggeredAndNotify to indicate that at least
      * one event has now triggered and needs to be evaluated.
      */
-    void triggerEvents() {
+    ISR_ATTR void triggerEvents() {
         lastInterruptTrigger = 0xff; // 0xff is the shorthand for event trigger basically.
         interrupted = true;
     }
@@ -299,6 +301,7 @@ class TaskManager {
      */
     TimerTask* getRunningTask() { return runningTask; }
 
+    friend class TaskExecutionRecorder;
 private:
     /**
      * Finds and allocates the next free task, once this returns a task will either have been allocated, making task

src/TaskTypes.cpp

@@ -127,6 +127,7 @@ void TimerTask::clear() {
 }
 
 void TimerTask::processEvent() {
+    RunningState runningState(this);
     myTimingSchedule = eventRef->timeOfNextCheck();
     if(eventRef->isTriggered()) {
         eventRef->setTriggered(false);
@@ -147,7 +148,7 @@ bool TimerTask::isRepeating() const {
     }
 }
 
-void BaseEvent::markTriggeredAndNotify() {
-    setTriggered(true);
+ISR_ATTR void BaseEvent::markTriggeredAndNotify() {
+    triggered = true;
     taskMgrAssociation->triggerEvents();
 }