Prevent overlapping scheduler tasks after immediate restart

Author: Mattboxx

.github/workflows/build.yml

@@ -32,6 +32,8 @@ jobs:
       - uses: actions/setup-python@v6
         with:
           python-version: "3.12"
+      - name: Test scheduler lifecycle without hardware
+        run: python test/check_lifecycle.py
       - run: python -m pip install platformio==6.1.19
       - run: pio pkg pack . --output FindMyAdv.tar.gz
       - uses: actions/upload-artifact@v7

CHANGELOG.md

@@ -1,5 +1,14 @@
 # Changelog
 
+## Unreleased — Lifecycle regression
+
+- Drain the previous scheduler before restarting with `begin()` after `end()`.
+  Previously an immediate restart could reset the stop request and leave two
+  scheduler tasks active. The same bounded shutdown path serves `reconfigure()`.
+- Add host-side regression tests compiling the real implementation against fake
+  FreeRTOS/BLE calls, covering repeated restart and switching to manual polling.
+- No API, package version, radio payload, key or existing release-tag changes.
+
 All notable changes to FindMyAdv are documented here. The format follows
 [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and the project uses
 [Semantic Versioning](https://semver.org/).

docs/API.md

@@ -33,6 +33,11 @@ when neither key is valid, BLE initialization fails, or the task cannot start.
 Calling `begin()` while already running is a no-op that returns `true`. Use
 `reconfigure()` to replace a live configuration.
 
+After `end()`, an immediate `begin()` waits for the previous private scheduler
+to exit before starting a replacement, including when switching to manual
+polling. Shutdown waiting is bounded at one second; no second task is created
+while the old task is still present. Serialize lifecycle calls in your application.
+
 ### `bool reconfigure(const FindMyAdvConfig &config)`
 
 Stops advertising, safely copies the new configuration, and restarts the

src/FindMyAdv.cpp

@@ -435,6 +435,15 @@ bool FindMyAdvClass::begin(const FindMyAdvConfig &configuration) {
         return false;
     }
 
+    // end() requests asynchronous task shutdown. Drain that task before
+    // resetting stopRequested or overwriting its handle on a new begin().
+    const uint32_t waitStarted = millis();
+    while (schedulerTask && millis() - waitStarted < 1000) delay(10);
+    if (schedulerTask) {
+        vTaskDelete(schedulerTask);
+        schedulerTask = nullptr;
+    }
+
     cfg = configuration;
     copyKey(appleText, cfg.appleAdvertisementKeyBase64);
     copyKey(googleText, cfg.googleAdvertisementEidHex);
@@ -521,12 +530,6 @@ bool FindMyAdvClass::reconfigure(const FindMyAdvConfig &configuration) {
     next.googleAdvertisementEidHex = nextGoogle;
 
     end();
-    const uint32_t waitStarted = millis();
-    while (schedulerTask && millis() - waitStarted < 1000) delay(10);
-    if (schedulerTask) {
-        vTaskDelete(schedulerTask);
-        schedulerTask = nullptr;
-    }
     return begin(next);
 }
 

test/check_lifecycle.py

@@ -0,0 +1,21 @@
+"""Run real library lifecycle code against a deterministic host scheduler.
+
+This catches overlapping task creation; it does not emulate BLE or prove RF.
+Requires a C++11 compiler (g++ by default, or CXX).
+"""
+import os
+from pathlib import Path
+import subprocess
+import tempfile
+
+ROOT = Path(__file__).resolve().parents[1]
+with tempfile.TemporaryDirectory(prefix="findmyadv-lifecycle-") as directory:
+    executable = Path(directory) / ("lifecycle.exe" if os.name == "nt" else "lifecycle")
+    subprocess.run([
+        os.environ.get("CXX", "g++"), "-std=c++11",
+        "-DFINDMYADV_DISABLE_BUILTIN_ACCELEROMETER=1",
+        "-I", str(ROOT / "test/host"), str(ROOT / "test/host/lifecycle.cpp"),
+        "-o", str(executable),
+    ], check=True)
+    subprocess.run([str(executable)], check=True)
+print("Lifecycle: immediate restart, repeated restart, reconfigure, manual mode and bounded shutdown passed")

test/host/Arduino.h

@@ -0,0 +1,14 @@
+#pragma once
+#include <cstddef>
+#include <cstdint>
+#include <cstdlib>
+
+using TaskHandle_t = void *;
+constexpr int pdPASS = 1;
+constexpr uint32_t pdMS_TO_TICKS(uint32_t ms) { return ms; }
+uint32_t millis();
+void delay(uint32_t ms);
+void vTaskDelay(uint32_t ticks);
+void vTaskDelete(TaskHandle_t task);
+int xTaskCreate(void (*entry)(void *), const char *, uint32_t, void *, int,
+                TaskHandle_t *handle);

test/host/NimBLEDevice.h

@@ -0,0 +1,24 @@
+#pragma once
+#include <cstdint>
+
+enum class NimBLETxPowerType { Advertise };
+struct NimBLEDevice {
+    static bool isInitialized() { return true; }
+    static bool init(const char *) { return true; }
+    static void setPower(int8_t, NimBLETxPowerType) {}
+    static bool setOwnAddr(uint8_t *) { return true; }
+    static bool setOwnAddrType(int) { return true; }
+};
+constexpr int BLE_OWN_ADDR_RANDOM = 1;
+constexpr int BLE_GAP_CONN_MODE_NON = 0;
+constexpr int BLE_GAP_DISC_MODE_NON = 0;
+constexpr int BLE_HS_FOREVER = -1;
+struct ble_gap_adv_params {
+    int conn_mode, disc_mode;
+    uint16_t itvl_min, itvl_max;
+};
+inline int ble_gap_adv_stop() { return 0; }
+inline int ble_gap_adv_set_data(uint8_t *, uint8_t) { return 0; }
+inline int ble_gap_adv_start(int, void *, int, ble_gap_adv_params *, void *, void *) {
+    return 0;
+}

test/host/lifecycle.cpp

@@ -0,0 +1,73 @@
+// Compile the actual implementation; only hardware/FreeRTOS calls are faked.
+#include <cstdio>
+#include "../../src/FindMyAdv.cpp"
+
+// Exit normally on failure, avoiding Windows assertion-dialog side effects.
+#define CHECK(condition) do { if (!(condition)) { \
+    std::fprintf(stderr, "Lifecycle check failed at line %d: %s\n", __LINE__, #condition); \
+    std::exit(1); \
+} } while (false)
+
+static uint32_t clockMs = 0;
+static unsigned liveTasks = 0, createdTasks = 0, overlappingTasks = 0;
+static unsigned forcedDeletes = 0;
+static bool stalledTask = false;
+
+uint32_t millis() { return clockMs; }
+void delay(uint32_t ms) {
+    clockMs += ms;
+    // Model the old task seeing end()'s stop request on its next wakeup.
+    if (!stalledTask && stopRequested && schedulerTask) {
+        --liveTasks;
+        schedulerTask = nullptr;
+    }
+}
+void vTaskDelay(uint32_t ticks) { delay(ticks); }
+void vTaskDelete(TaskHandle_t task) {
+    CHECK(task != nullptr);  // The test never actually enters a scheduler loop.
+    CHECK(liveTasks > 0);
+    --liveTasks;
+    ++forcedDeletes;
+}
+int xTaskCreate(void (*)(void *), const char *, uint32_t, void *, int,
+                TaskHandle_t *handle) {
+    if (liveTasks) ++overlappingTasks;
+    ++liveTasks;
+    *handle = reinterpret_cast<void *>(static_cast<uintptr_t>(++createdTasks));
+    return pdPASS;
+}
+
+int main() {
+    FindMyAdvConfig config;
+    config.googleAdvertisementEidHex = "0000000000000000000000000000000000000000";
+    CHECK(FindMyAdv.begin(config));
+    CHECK(liveTasks == 1);
+    const unsigned initialCreated = createdTasks;
+    CHECK(FindMyAdv.begin(config));  // Already running is still a no-op.
+    CHECK(createdTasks == initialCreated);
+    for (int i = 0; i < 5; ++i) {
+        FindMyAdv.end();
+        CHECK(FindMyAdv.begin(config));
+        CHECK(overlappingTasks == 0);
+        CHECK(liveTasks == 1);
+    }
+    CHECK(FindMyAdv.reconfigure(config));
+    CHECK(liveTasks == 1 && overlappingTasks == 0);
+    FindMyAdv.end();
+    config.backgroundTask = false;
+    CHECK(FindMyAdv.begin(config));
+    CHECK(liveTasks == 0);  // Background -> manual must drain the old task.
+    FindMyAdv.poll();
+    FindMyAdv.end();
+    config.backgroundTask = true;
+    CHECK(FindMyAdv.begin(config));
+    FindMyAdv.end();
+    stalledTask = true;
+    CHECK(FindMyAdv.begin(config));
+    CHECK(forcedDeletes == 1);  // Preserve the bounded shutdown fallback.
+    CHECK(liveTasks == 1 && overlappingTasks == 0);
+    stalledTask = false;
+    FindMyAdv.end();
+    delay(100);
+    CHECK(liveTasks == 0);
+}

test/host/sdkconfig.h

@@ -0,0 +1,2 @@
+#pragma once
+// Host tests use the default NimBLE branch with the sensor compiled out.