scheduled functions: fixes (#6137)

* scheduled functions: properly reset structure

* fence against recursion, rename variables for clarity

* update comments

cores/esp8266/Schedule.cpp

@@ -33,6 +33,7 @@ static scheduled_fn_t* get_fn_unsafe()
         result = sUnused;
         sUnused = sUnused->mNext;
         result->mNext = nullptr;
+        result->callNow.reset(esp8266::polledTimeout::periodicFastUs::alwaysExpired);
     }
     // if no unused items, and count not too high, allocate a new one
     else if (sCount < SCHEDULED_FN_MAX_COUNT)
@@ -100,32 +101,43 @@ void run_scheduled_functions()
     // its purpose is that it is never called from an interrupt
     // (always on cont stack).
 
-    scheduled_fn_t* lastRecurring = nullptr;
+    static bool fence = false;
-    scheduled_fn_t* toCall = sFirst;
-    while (toCall)
     {
-        scheduled_fn_t* item = toCall;
+        InterruptLock lockAllInterruptsInThisScope;
-        toCall = toCall->mNext;
+        if (fence)
-        if (item->callNow)
+            // prevent recursive calls from yield()
+            return;
+        fence = true;
+    }
+
+    scheduled_fn_t* lastRecurring = nullptr;
+    scheduled_fn_t* nextCall = sFirst;
+    while (nextCall)
+    {
+        scheduled_fn_t* toCall = nextCall;
+        nextCall = nextCall->mNext;
+        if (toCall->callNow)
         {
-            if (item->mFunc())
+            if (toCall->mFunc())
             {
-                lastRecurring = item;
+                lastRecurring = toCall;
             }
             else
             {
                 InterruptLock lockAllInterruptsInThisScope;
 
-                if (sFirst == item)
+                if (sFirst == toCall)
                     sFirst = sFirst->mNext;
                 else if (lastRecurring)
-                    lastRecurring->mNext = item->mNext;
+                    lastRecurring->mNext = toCall->mNext;
 
-                if (sLast == item)
+                if (sLast == toCall)
                     sLast = lastRecurring;
 
-                recycle_fn_unsafe(item);
+                recycle_fn_unsafe(toCall);
             }
         }
     }
+
+    fence = false;
 }

cores/esp8266/Schedule.h

@@ -3,25 +3,38 @@
 
 #include <functional>
 
+// This API is stabilizing
+// Function signatures may change, internal queue will remain FIFO.
+//
+// * Add the given lambda to a fifo list of lambdas, which is run when
+//   - `loop` function returns,
+//   - or `yield` is called,
+//   - or `run_scheduled_functions` is called.
+//
+// * Use lambdas to pass arguments to a function, or call a class/static
+//   member function.
+//
+// * Please ensure variables or instances used from inside lambda will exist
+//   when lambda is later called
+//
+// * There is no mechanism for cancelling scheduled functions.
+//
+// * `yield` can be called from inside lambdas
+//
+// * Returns false if the number of scheduled functions exceeds
+//   SCHEDULED_FN_MAX_COUNT.
+
 #define SCHEDULED_FN_MAX_COUNT 32
 
-// This API was not considered stable but is now stabilizing.
+// * Run the lambda only once next time
-// Function signatures may change, queue must stay FIFO.
-// You have been warned.
-
-// Run given function ONCE next time `loop` function returns,
-// or `yield` is called,
-// or `run_scheduled_functions` is called.
-// Use std::bind to pass arguments to a function, or call a class member function.
-// Note: there is no mechanism for cancelling scheduled functions.
-// Keep that in mind when binding functions to objects which may have short lifetime.
-// Returns false if the number of scheduled functions exceeds SCHEDULED_FN_MAX_COUNT.
 //bool schedule_function(std::function<void(void)>&& fn);
 bool schedule_function(const std::function<void(void)>& fn);
 
-// Run given function periodically about every <repeat_us> microseconds until it returns false.
+// * Run the lambda periodically about every <repeat_us> microseconds until
-// Note that it may be more than <repeat_us> microseconds between calls if `yield` is not called
+//   it returns false.
-// frequently, and therefore should not be used for timing critical operations.
+// * Note that it may be more than <repeat_us> microseconds between calls if
+//   `yield` is not called frequently, and therefore should not be used for
+//   timing critical operations.
 //bool schedule_function_us(std::function<bool(void)>&& fn, uint32_t repeat_us);
 bool schedule_function_us(const std::function<bool(void)>& fn, uint32_t repeat_us);