Ticker callback should be allowed to be destroyed

Mainly, we protect std::function and variant itself from undefined behaviour when the underlying object is destroyed and then re-created with something else inside of it. For example, every captured object would be destroyed. ```cpp Ticker ticker; void oops(Job job) { ticker.once(1.0f, [job = std::move(job)]() { ticker.once(job.next(), do_something_else); job.work(); // ~Job() just happened! }); } ``` Another important case is repeating through once() ```cpp Ticker ticker; void repeat() { ticker.once(1.0f, repeat); puts("tick"); } void setup() { repeat(); } ``` Temporarily moving callback object into the function and returning earlier should solve both cases.
esp8266 · mcspr · Jan 26, 2023 · Jan 19, 2023 · Jan 20, 2023 · Jan 26, 2023
commit 1623b6a71fb59d7fc09e87f35286d7ccfa9034e9
diff --git a/libraries/Ticker/src/Ticker.cpp b/libraries/Ticker/src/Ticker.cpp
@@ -115,14 +115,27 @@ void Ticker::_static_callback()
         }
     }
 
+    // it is technically allowed to call either schedule or detach
+    // *during* callback execution. allow both to happen
+    auto tmp = std::move(_callback);
+
     std::visit([](auto&& callback) {
         using T = std::decay_t<decltype(callback)>;
         if constexpr (std::is_same_v<T, callback_ptr_t>) {
             callback.func(callback.arg);
         } else if constexpr (std::is_same_v<T, callback_function_t>) {
             callback();
         }
-    }, _callback);
+    }, tmp);
+
+    // ...and move ourselves back only when object is in a valid state
+    // * ticker was not detached, zeroing timer pointer
+    // * nothing else replaced callback variant
+    if ((_timer == nullptr) || !std::holds_alternative<std::monostate>(_callback)) {
+        return;
+    }
+
+    _callback = std::move(tmp);
 
     if (_repeat) {
         if (_tick) {