arkhipenko / TaskScheduler

Cooperative multitasking for Arduino, ESPx, STM32, nRF and other microcontrollers
http://playground.arduino.cc/Code/TaskScheduler
BSD 3-Clause "New" or "Revised" License
1.22k stars 224 forks source link

deleteTask() could kill unrelated tasks #97

Closed vortigont closed 4 years ago

vortigont commented 4 years ago

I'm not sure if this a proper way or not to deleteTask(), can't find any specific info in the API. The issue is if deleteTask() has beelncalled for a task that has never been added() - the previous task is going to be deleted instead!

For ex:

Scheduler ts;
Task t1;
Task t2;

t1.set(TASK_SECOND, TASK_FOREVER, &incrementer);
ts.addTask(t1);
t1.enable();

ts.deleteTask(t2);
t2.set(TASK_SECOND, TASK_FOREVER, &decrementer);
ts2.addTask(t2);
t2.enable();

in that case t1 task would be deleted from ts and never ever get executed! Only t2 task will be running forever. If that is an expected behaviour how can one make sure that t2 really exists on the scheduler before deleting it? For ex. if (ts2.isTask(t2) ts.deleteTask(t2); Thanks!

arkhipenko commented 4 years ago

Yes indeed, if you delete the task from the scheduler that was never added to it, the behaviour is incorrect. I will be adding guards to avoid such and similar situation (like adding a task to a chain if a task already belongs to another chain, etc. )

arkhipenko commented 4 years ago

Pushed fix into the testing branch. Please test.

// #define _TASK_TIMECRITICAL      // Enable monitoring scheduling overruns
// #define _TASK_SLEEP_ON_IDLE_RUN // Enable 1 ms SLEEP_IDLE powerdowns between tasks if no callback methods were invoked during the pass
// #define _TASK_STATUS_REQUEST    // Compile with support for StatusRequest functionality - triggering tasks on status change events in addition to time only
// #define _TASK_WDT_IDS           // Compile with support for wdt control points and task ids
// #define _TASK_LTS_POINTER       // Compile with support for local task storage pointer
// #define _TASK_PRIORITY          // Support for layered scheduling priority
// #define _TASK_MICRO_RES         // Support for microsecond resolution
// #define _TASK_STD_FUNCTION      // Support for std::function (ESP8266 and ESP32 ONLY)
// #define _TASK_DEBUG             // Make all methods and variables public for debug purposes
// #define _TASK_INLINE            // Make all methods "inline" - needed to support some multi-tab, multi-file implementations
// #define _TASK_TIMEOUT           // Support for overall task timeout
// #define _TASK_OO_CALLBACKS      // Support for dynamic callback method binding
// #define _TASK_DEFINE_MILLIS     // Force forward declaration of millis() and micros() "C" style
// #define _TASK_EXPOSE_CHAIN      // Methods to access tasks in the task chain

#include <TaskScheduler.h>

// Debug and Test options
#define _DEBUG_
//#define _TEST_

#ifdef _DEBUG_
#define _PP(a) Serial.print(a);
#define _PL(a) Serial.println(a);
#else
#define _PP(a)
#define _PL(a)
#endif

// Scheduler
Scheduler ts;

void incrementer() ;
void decrementer() ;

/*
  Scheduling defines:
  TASK_MILLISECOND
  TASK_SECOND
  TASK_MINUTE
  TASK_HOUR
  TASK_IMMEDIATE
  TASK_FOREVER
  TASK_ONCE
  TASK_NOTIMEOUT
*/

Task t1;
Task t2;

void setup() {
  // put your setup code here, to run once:
#if defined(_DEBUG_) || defined(_TEST_)
  Serial.begin(115200);
  delay(2000);
  _PL("Delete test: setup()");
#endif

  t1.set(TASK_SECOND, TASK_FOREVER, &incrementer);
  ts.addTask(t1);
  t1.enable();

  ts.deleteTask(t2); // this is now ignored (Issue 97)
  t2.set(TASK_SECOND, TASK_FOREVER, &decrementer);
  ts.addTask(t2);
  t2.enable();
}

void loop() {
  ts.execute();
}

void incrementer() {
_PP(millis());
_PL(": incrementer()");

}

void decrementer() {
_PP(millis());
_PL(": decrementer()");

}
vortigont commented 4 years ago

testing branch now works perfectly for my test case and project! Thanks for the fast fix! Appreciate your support! :)

vortigont commented 4 years ago

@arkhipenko any chance to merge this fix into master?

arkhipenko commented 4 years ago

Done.