diff --git a/CHANGELOG.md b/CHANGELOG.md index 529fcbb..a616d99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,10 +12,13 @@ All notable changes to this project will be documented in this file. - Hooks for retries/backoff strategies on top of the existing async API. - Added `FetchConfig::usePSRAMBuffers` and routed JSON-mode response body/header storage plus request-body/copied-request-header storage through `ESPBufferManager` with automatic fallback to normal heap paths. - Switched request task creation/lifecycle to native FreeRTOS `xTaskCreatePinnedToCore(...)` handling. +- Added explicit teardown-contract lifecycle coverage (`deinit()` pre-init, repeated `deinit()`, and `init -> deinit -> init`). +- Added `isInitialized()` as the public runtime-state contract accessor. ### Fixed - Normalize malformed `http:/` or `https:/` URLs to `http://`/`https://` to avoid DNS failures with parsed hosts like `:example.com`. - Collapse extra slashes (`https:///`) and strip a leading `://:` host typo before handing URLs to esp_http_client. +- Teardown now requests active workers to abort in-flight operations and waits for worker completion before releasing shared runtime resources. ### Notes - Streaming requests bypass all body buffering and ArduinoJson processing. diff --git a/README.md b/README.md index 9c27c7d..de0ad2f 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,23 @@ void setup() { } void loop() {} -```` +``` + +--- + +## Teardown Contract + +`ESPFetch` supports explicit teardown and re-init: + +- `deinit()` is safe before `init()` +- `deinit()` is idempotent +- Active requests are asked to abort during teardown and teardown waits for worker tasks to finish + +```cpp +if (fetch.isInitialized()) { + fetch.deinit(); +} +``` --- @@ -195,6 +211,7 @@ If the limit is exceeded: ```cpp bool init(const FetchConfig& cfg = {}); void deinit(); +bool isInitialized() const; ``` --- diff --git a/examples/basic_fetch/basic_fetch.ino b/examples/basic_fetch/basic_fetch.ino index 7a6c760..1f1134a 100644 --- a/examples/basic_fetch/basic_fetch.ino +++ b/examples/basic_fetch/basic_fetch.ino @@ -2,6 +2,7 @@ #include ESPFetch fetch; +bool deinitialized = false; const char *POST_URL = "https://httpbin.org/post"; const char *GET_URL = "https://httpbin.org/get"; @@ -62,5 +63,10 @@ void setup() { } void loop() { + if (!deinitialized && fetch.isInitialized() && millis() > 15000UL) { + fetch.deinit(); + deinitialized = true; + ESP_LOGI("FETCH_DEMO", "ESPFetch deinitialized"); + } vTaskDelay(pdMS_TO_TICKS(1000)); } diff --git a/src/esp_fetch/fetch.cpp b/src/esp_fetch/fetch.cpp index 21c3297..406d3cb 100644 --- a/src/esp_fetch/fetch.cpp +++ b/src/esp_fetch/fetch.cpp @@ -239,7 +239,7 @@ ESPFetch::~ESPFetch() { } bool ESPFetch::init(const FetchConfig &config) { - if (_initialized) { + if (isInitialized()) { deinit(); } @@ -255,12 +255,18 @@ bool ESPFetch::init(const FetchConfig &config) { return false; } - _initialized = true; + _teardownRequested.store(false, std::memory_order_release); + _initialized.store(true, std::memory_order_release); return true; } void ESPFetch::deinit() { - _initialized = false; + if (!isInitialized() && _activeTasks.load(std::memory_order_acquire) == 0 && _slotSemaphore == nullptr) { + return; + } + + _teardownRequested.store(true, std::memory_order_release); + _initialized.store(false, std::memory_order_release); while (_activeTasks.load(std::memory_order_acquire) > 0) { #if defined(INCLUDE_xTaskGetSchedulerState) && (INCLUDE_xTaskGetSchedulerState == 1) @@ -270,13 +276,17 @@ void ESPFetch::deinit() { #endif vTaskDelay(pdMS_TO_TICKS(1)); } - _activeTasks.store(0, std::memory_order_release); if (_slotSemaphore) { vSemaphoreDelete(_slotSemaphore); _slotSemaphore = nullptr; } + _teardownRequested.store(false, std::memory_order_release); +} + +bool ESPFetch::isInitialized() const { + return _initialized.load(std::memory_order_acquire); } bool ESPFetch::get(const char *url, FetchCallback callback, const FetchRequestOptions &options) { @@ -417,7 +427,7 @@ bool ESPFetch::enqueueRequest(const std::string &url, FetchCallback callback, std::shared_ptr syncHandle, const FetchRequestOptions &options) { - if (!_initialized) { + if (!isInitialized()) { ESP_LOGE(TAG, "ESPFetch not initialized"); return false; } @@ -490,7 +500,7 @@ bool ESPFetch::enqueueStreamRequest(const std::string &url, FetchChunkCallback onChunk, FetchStreamCallback onDone, const FetchRequestOptions &options) { - if (!_initialized) { + if (!isInitialized()) { ESP_LOGE(TAG, "ESPFetch not initialized"); return false; } @@ -596,6 +606,12 @@ esp_err_t ESPFetch::handleHttpEvent(esp_http_client_event_t *event) { return ESP_OK; } auto *job = static_cast(event->user_data); + if (job->owner && job->owner->_teardownRequested.load(std::memory_order_acquire)) { + if (job->isStream && job->streamAbortError == ESP_OK) { + job->streamAbortError = ESP_ERR_INVALID_STATE; + } + return ESP_FAIL; + } switch (event->event_id) { case HTTP_EVENT_ON_DATA: @@ -674,57 +690,65 @@ void ESPFetch::runJob(std::unique_ptr job) { const int64_t start = esp_timer_get_time(); - esp_http_client_config_t config = {}; - config.url = job->url.c_str(); - config.method = job->method; - config.timeout_ms = job->requestOptions.timeoutMs ? job->requestOptions.timeoutMs : _config.defaultTimeoutMs; - config.event_handler = &ESPFetch::handleHttpEvent; - config.user_data = job.get(); - config.disable_auto_redirect = !(job->requestOptions.allowRedirects && _config.followRedirects); - config.skip_cert_common_name_check = job->requestOptions.skipTlsCommonNameCheck || _config.skipTlsCommonNameCheck; - - esp_http_client_handle_t client = esp_http_client_init(&config); - if (!client) { - ESP_LOGE(TAG, "esp_http_client_init failed"); - job->response.error = ESP_ERR_NO_MEM; + if (_teardownRequested.load(std::memory_order_acquire)) { + job->response.error = ESP_ERR_INVALID_STATE; } else { - auto hasHeader = [&](const char *key) { - for (const auto &header : job->requestOptions.headers) { - if (equalsIgnoreCase(header.name, key)) { - return true; + esp_http_client_config_t config = {}; + config.url = job->url.c_str(); + config.method = job->method; + config.timeout_ms = job->requestOptions.timeoutMs ? job->requestOptions.timeoutMs : _config.defaultTimeoutMs; + config.event_handler = &ESPFetch::handleHttpEvent; + config.user_data = job.get(); + config.disable_auto_redirect = !(job->requestOptions.allowRedirects && _config.followRedirects); + config.skip_cert_common_name_check = + job->requestOptions.skipTlsCommonNameCheck || _config.skipTlsCommonNameCheck; + + esp_http_client_handle_t client = esp_http_client_init(&config); + if (!client) { + ESP_LOGE(TAG, "esp_http_client_init failed"); + job->response.error = ESP_ERR_NO_MEM; + } else if (_teardownRequested.load(std::memory_order_acquire)) { + job->response.error = ESP_ERR_INVALID_STATE; + esp_http_client_cleanup(client); + } else { + auto hasHeader = [&](const char *key) { + for (const auto &header : job->requestOptions.headers) { + if (equalsIgnoreCase(header.name, key)) { + return true; + } } - } - return false; - }; + return false; + }; - if (_config.userAgent && !hasHeader("User-Agent")) { - esp_http_client_set_header(client, "User-Agent", _config.userAgent); - } + if (_config.userAgent && !hasHeader("User-Agent")) { + esp_http_client_set_header(client, "User-Agent", _config.userAgent); + } - const char *contentType = - job->requestOptions.contentType ? job->requestOptions.contentType : _config.defaultContentType; - if (!job->isStream && job->method == HTTP_METHOD_POST && contentType && !hasHeader("Content-Type")) { - esp_http_client_set_header(client, "Content-Type", contentType); - } + const char *contentType = + job->requestOptions.contentType ? job->requestOptions.contentType : _config.defaultContentType; + if (!job->isStream && job->method == HTTP_METHOD_POST && contentType && !hasHeader("Content-Type")) { + esp_http_client_set_header(client, "Content-Type", contentType); + } - for (const auto &header : job->requestOptions.headers) { - esp_http_client_set_header(client, header.name.c_str(), header.value.c_str()); - } + for (const auto &header : job->requestOptions.headers) { + esp_http_client_set_header(client, header.name.c_str(), header.value.c_str()); + } - if (!job->body.empty()) { - esp_http_client_set_post_field(client, job->body.c_str(), job->body.length()); - } + if (!job->body.empty()) { + esp_http_client_set_post_field(client, job->body.c_str(), job->body.length()); + } - job->response.error = esp_http_client_perform(client); - if (job->response.error == ESP_OK) { - job->response.statusCode = esp_http_client_get_status_code(client); - } - // Preserve intentional stream abort reason (onChunk returned false or maxBodyBytes clipping), - // instead of losing it to generic ESP_FAIL from esp_http_client_perform(). - if (job->isStream && job->response.error != ESP_OK && job->streamAbortError != ESP_OK) { - job->response.error = job->streamAbortError; + job->response.error = esp_http_client_perform(client); + if (job->response.error == ESP_OK) { + job->response.statusCode = esp_http_client_get_status_code(client); + } + // Preserve intentional stream abort reason (onChunk returned false or maxBodyBytes clipping), + // instead of losing it to generic ESP_FAIL from esp_http_client_perform(). + if (job->isStream && job->response.error != ESP_OK && job->streamAbortError != ESP_OK) { + job->response.error = job->streamAbortError; + } + esp_http_client_cleanup(client); } - esp_http_client_cleanup(client); } job->response.durationUs = esp_timer_get_time() - start; diff --git a/src/esp_fetch/fetch.h b/src/esp_fetch/fetch.h index 7fdfbd3..59c674b 100644 --- a/src/esp_fetch/fetch.h +++ b/src/esp_fetch/fetch.h @@ -72,7 +72,7 @@ class ESPFetch { bool init(const FetchConfig &config = FetchConfig{}); void deinit(); - bool initialized() const { return _initialized; } + bool isInitialized() const; bool get(const char *url, FetchCallback callback, const FetchRequestOptions &options = FetchRequestOptions{}); bool get(const String &url, FetchCallback callback, const FetchRequestOptions &options = FetchRequestOptions{}); @@ -133,7 +133,8 @@ class ESPFetch { static void deliverResult(const std::unique_ptr &job, const JsonDocument &result); FetchConfig _config{}; - bool _initialized = false; + std::atomic _initialized{false}; + std::atomic _teardownRequested{false}; std::atomic _activeTasks{0}; SemaphoreHandle_t _slotSemaphore = nullptr; }; diff --git a/test/test_esp_fetch/test_esp_fetch.cpp b/test/test_esp_fetch/test_esp_fetch.cpp index ccbf50f..0f56d08 100644 --- a/test/test_esp_fetch/test_esp_fetch.cpp +++ b/test/test_esp_fetch/test_esp_fetch.cpp @@ -7,15 +7,15 @@ static void test_init_rejects_zero_concurrency() { FetchConfig cfg{}; cfg.maxConcurrentRequests = 0; TEST_ASSERT_FALSE(fetch.init(cfg)); - TEST_ASSERT_FALSE(fetch.initialized()); + TEST_ASSERT_FALSE(fetch.isInitialized()); } static void test_init_and_deinit_cycle_updates_initialized_flag() { ESPFetch fetch; TEST_ASSERT_TRUE(fetch.init()); - TEST_ASSERT_TRUE(fetch.initialized()); + TEST_ASSERT_TRUE(fetch.isInitialized()); fetch.deinit(); - TEST_ASSERT_FALSE(fetch.initialized()); + TEST_ASSERT_FALSE(fetch.isInitialized()); } static void test_init_accepts_psram_buffer_toggle() { @@ -23,10 +23,35 @@ static void test_init_accepts_psram_buffer_toggle() { FetchConfig cfg{}; cfg.usePSRAMBuffers = true; TEST_ASSERT_TRUE(fetch.init(cfg)); - TEST_ASSERT_TRUE(fetch.initialized()); + TEST_ASSERT_TRUE(fetch.isInitialized()); fetch.deinit(); } +static void test_deinit_is_safe_before_init() { + ESPFetch fetch; + fetch.deinit(); + TEST_ASSERT_FALSE(fetch.isInitialized()); +} + +static void test_deinit_is_idempotent() { + ESPFetch fetch; + TEST_ASSERT_TRUE(fetch.init()); + TEST_ASSERT_TRUE(fetch.isInitialized()); + fetch.deinit(); + fetch.deinit(); + TEST_ASSERT_FALSE(fetch.isInitialized()); +} + +static void test_reinit_after_deinit_is_supported() { + ESPFetch fetch; + TEST_ASSERT_TRUE(fetch.init()); + fetch.deinit(); + TEST_ASSERT_TRUE(fetch.init()); + TEST_ASSERT_TRUE(fetch.isInitialized()); + fetch.deinit(); + TEST_ASSERT_FALSE(fetch.isInitialized()); +} + static void test_async_get_requires_initialization() { ESPFetch fetch; volatile bool invoked = false; @@ -84,6 +109,9 @@ void setup() { RUN_TEST(test_init_rejects_zero_concurrency); RUN_TEST(test_init_and_deinit_cycle_updates_initialized_flag); RUN_TEST(test_init_accepts_psram_buffer_toggle); + RUN_TEST(test_deinit_is_safe_before_init); + RUN_TEST(test_deinit_is_idempotent); + RUN_TEST(test_reinit_after_deinit_is_supported); RUN_TEST(test_async_get_requires_initialization); RUN_TEST(test_sync_get_reports_error_when_not_initialized); RUN_TEST(test_sync_get_requires_url);