Release hardening: lifecycle synchronization, request signaling, bounds, and CI coverage

[zekageri]

Summary

Harden Link before the v0.1.0 release by eliminating lifecycle races, making queue publication and worker signaling atomic, validating all values narrowed into ESP-IDF int parameters, removing silent allocation failures from response copies, aligning redirect behavior, and ensuring every new subsystem is release-gated by CI.

This issue is release-blocking. The two lifecycle findings can lead to use-after-free behavior under concurrent submission, initialization, and shutdown.

Current review target: main at c03c31418f518487f40fbceaafb01a95b8fe2479.

---

Goals

• Make init(), fetch(), deinit(), and destruction safe under supported concurrent use.
• Guarantee that a queued request and its worker wake permit are published as one logical operation.
• Ensure runtime storage and synchronization primitives cannot be freed while another public operation can still access them.
• Preserve Link's bounded-memory and no-exception design.
• Make allocation failure explicit rather than silently returning incomplete successful responses.
• Ensure release tags cannot bypass persistent-client tests or lifecycle stress coverage.

Non-goals

• Adding automatic retries.
• Changing the default connection mode from PerRequest.
• Adding asynchronous cancellation of a currently blocking ESP-IDF HTTP call beyond the existing timeout-based shutdown contract.
• Redesigning the public fetch-style API unless required to remove unsafe copy semantics.

---

Phase 1 — Define and enforce lifecycle invariants

Required invariants

The implementation must maintain all of the following:

1. Exactly one lifecycle transition may execute at a time.
2. init() owns the full transition from Uninitialized to Running or back to Uninitialized on failure.
3. deinit() owns the full transition from Running/Starting/Stopping to Uninitialized, except when the public wait times out and intentionally leaves the instance in Stopping.
4. Once runtime storage begins being freed, no public operation may still access _items, _slots, _slotUsed, _queue, _workers, or configuration used to index them.
5. fetch() may only publish work while the instance is Running.
6. A successfully published queue entry always has a corresponding worker signal.
7. A worker signal is never sent through a semaphore that may already have been deleted.
8. User callbacks continue to execute without Link's internal state mutex held.
9. deinit() and destruction remain forbidden from Link callbacks because they wait for the current worker; this must remain documented and tested where practical.

Implementation direction

Add a dedicated lifecycle synchronization primitive separate from the current state/data mutex.

Suggested structure:

• _lifecycleMutex: serializes complete init() and deinitInternal() calls.
• _mutex: continues to protect short critical sections involving state, queue metadata, worker records, diagnostics, and runtime pointers.

The lifecycle mutex should be recursive only if an existing failure path requires init() to call a shutdown helper that reacquires it. Prefer restructuring cleanup so recursion is not required.

init() requirements

• Acquire lifecycle ownership before inspecting or changing lifecycle state.
• Validate configuration before publishing Starting where possible.
• Allocate all runtime storage into temporary/local ownership first, or ensure partial state is inaccessible to concurrent public calls.
• Create _items and worker records under a lifecycle regime that prevents deinit() from freeing them mid-startup.
• Publish Running only after every required worker has been created successfully.
• On any failure:
  • stop any workers already created;
  • wait for them to exit;
  • delete the semaphore;
  • release every partially allocated array;
  • clear pointers and queue metadata;
  • restore Uninitialized;
  • return the original failure.
• Never overwrite Stopping with Running.

deinitInternal() requirements

• Acquire lifecycle ownership for the full shutdown operation.
• Handle repeated calls while already Stopping.
• Mark Stopping exactly once.
• Issue shutdown wake permits exactly once per shutdown attempt unless a retry after timeout requires additional signaling by design.
• Wait for workers before freeing worker-owned storage.
• If public waiting times out:
  • keep state as Stopping;
  • keep all runtime pointers and synchronization objects alive;
  • allow a later deinit() call to continue the same shutdown safely.
• Blocking destruction may continue waiting indefinitely, relying on nonzero configured HTTP timeouts.

Acceptance criteria

• Concurrent init() and deinit() cannot access freed memory.
• A failed partial init() leaves the object fully reusable.
• A successful deinit() leaves all runtime pointers null and state Uninitialized.
• A timed-out deinit() leaves the object safely in Stopping without leaks or dangling pointers.
• Repeated deinit() after timeout eventually completes cleanup when workers exit.

---

Phase 2 — Make queue publication and semaphore signaling atomic

Problem to eliminate

The current flow inserts a request while holding _mutex, releases _mutex, and then calls xSemaphoreGive(_items). Shutdown can drain the request, exit workers, delete _items, and allow the submitting task to resume with a stale semaphore handle.

Required implementation

Treat the following as one publication transaction:

1. reserve a free request slot;
2. move the owned request into the slot;
3. append the slot index to the queue;
4. update queue metadata;
5. increment submission diagnostics;
6. signal one worker.

The worker signal must occur while runtime lifetime is still protected. The simplest acceptable design is to call xSemaphoreGive(_items) inside the same _mutex critical section that publishes the request.

Signal failure handling

Although xSemaphoreGive() should normally succeed with the reserved-capacity design, handle failure explicitly:

• remove the just-published queue entry;
• restore queue tail and count;
• reset and free the slot;
• restore _slotUsed;
• roll back requestsSubmitted;
• return InternalError or a dedicated signaling error if one is introduced.

Do not leave an accepted request without a permit.

Capacity invariant

Retain the counting semaphore capacity of:

queueSize + maxConcurrentRequests

The additional worker-count capacity remains reserved for shutdown wake permits even when every request slot is occupied.

Acceptance criteria

• No access to _items occurs outside synchronization that guarantees its lifetime.
• Every successful fetch() produces exactly one request permit.
• Every failed fetch() leaves queue state unchanged.
• Shutdown can wake every worker even when the request queue is full.
• Diagnostics remain consistent after signal rollback.

---

Phase 3 — Add deterministic lifecycle and publication tests

Host tests alone cannot exercise real FreeRTOS semaphore deletion and task scheduling. Add both deterministic logic tests and an on-device stress sketch/test.

Host-side tests

Extract small state-transition or queue-publication helpers where useful, without weakening encapsulation.

Add coverage for:

• lifecycle state transition legality;
• repeated shutdown calls;
• partial initialization rollback;
• queue publication rollback when signaling fails through a test seam;
• semaphore capacity overflow detection;
• request ID wrap behavior, confirming that wrap does not affect correctness;
• diagnostics invariants after submission failure and shutdown cancellation.

A testable signal adapter or function seam is acceptable if it has zero runtime allocation and negligible embedded overhead.

ESP32 stress test

Create a real runtime test, not compile-only, with at least:

• one or more producer tasks continuously calling get();
• a lifecycle task repeatedly calling deinit() and init();
• queue sizes that reach full capacity;
• two or more workers;
• short, nonzero HTTP timeouts;
• a deliberately unreachable address plus a local fast endpoint when available;
• both PerRequest and PersistentPerWorker modes;
• at least several thousand lifecycle cycles or an equivalent sustained runtime;
• heap integrity checks between rounds;
• callback count accounting for accepted requests;
• verification that every accepted request ends in exactly one terminal callback;
• verification that no callback arrives after successful final deinitialization;
• verification that requestsCompleted == requestsSubmitted after successful shutdown;
• verification that persistent client creates equal cleanups after shutdown.

Recommended ESP-IDF diagnostics when available:

• heap_caps_check_integrity_all(true);
• free heap;
• minimum free heap;
• largest free block;
• worker task stack high-water marks;
• Link diagnostics snapshots.

CI handling

The hardware test may remain a documented/manual release qualification if no hardware runner exists, but the sketch must compile in CI and the release checklist must require recorded on-device results.

---

Phase 4 — Validate all public sizes and timeouts before narrowing conversions

Timeout bounds

defaultTimeoutMs and per-request timeoutMs are uint32_t, while ESP-IDF receives int.

Add validation so every effective timeout satisfies:

1 <= timeoutMs <= INT_MAX

Requirements:

• reject an oversized LinkConfig::defaultTimeoutMs during init();
• reject an oversized per-request timeout during fetch() before queue publication;
• use one helper for the effective timeout calculation and validation;
• never rely on implementation-defined unsigned-to-signed conversion.

Request body bounds

esp_http_client_set_post_field() also receives an int length.

Require the configured and actual request body size to be representable by int:

• reject maxRequestBodySize > INT_MAX, or cap it through validation;
• reject any actual body length that exceeds INT_MAX before calling ESP-IDF;
• retain the stricter configured limit when it is lower.

Stream buffer bounds

Review every size converted into an ESP-IDF int, including buffer_size. Validate the corresponding configuration against the target parameter type.

Acceptance criteria

• No unchecked size_t/uint32_t to int conversion remains on an ESP-IDF API boundary.
• Boundary tests cover 0, 1, INT_MAX, and INT_MAX + 1 where the source type permits it.
• Error codes clearly distinguish invalid configuration from oversized individual requests.

---

Phase 5 — Remove silent allocation failure from copy operations

Problem to eliminate

Implicit copies of LinkOwnedBuffer and LinkHeaders can fail allocation while their constructors or assignments cannot return LinkResult. A copied LinkResponse can therefore retain error == Ok while silently losing body or headers.

Preferred design

Make response payload ownership move-oriented and explicit:

• delete copy construction and copy assignment for LinkOwnedBuffer;
• delete copy construction and copy assignment for LinkHeaders unless a strong non-silent contract is introduced;
• consequently make LinkResponse move-only if needed;
• retain explicit copyFrom()/cloneFrom() methods returning LinkResult for callers that require duplication.

For public ergonomics, consider:

LinkResult cloneFrom(const LinkResponse &source);

or separate explicit clone helpers for headers and body.

Compatibility evaluation

Before changing the API:

• search Core and other known consumers for copied LinkResponse, LinkHeaders, or LinkOwnedBuffer values;
• document the breaking change because v0.1.0 has not been released yet;
• prefer making the safe API change now rather than preserving silent corruption behavior.

Acceptance criteria

• No copy constructor or copy assignment can silently discard data.
• Every allocation-backed duplication operation returns an inspectable result.
• Move operations remain noexcept and allocation-free.
• Tests simulate allocation failure through a deterministic allocation seam and verify that success cannot be reported with missing response data.

---

Phase 6 — Align buffered and streaming redirect behavior

Problem to eliminate

Buffered mode accumulates the intermediate redirect body before redirect disposition is evaluated. A large 3xx body can trigger ResponseTooLarge and prevent a redirect that should otherwise be followed. Streaming mode already suppresses intermediate redirect chunks.

Required behavior

Once status and headers establish that the current response will be followed:

• do not expose intermediate stream callbacks;
• do not accumulate the intermediate buffered body;
• retain only the headers required for redirect evaluation;
• preserve current redirect limits and security policy;
• continue supporting only absolute http:// or https:// locations;
• continue stripping all caller-supplied headers after an allowed origin change;
• never restore stripped headers later in the redirect chain;
• never automatically replay non-GET requests.

Implementation options

Preferred: determine redirect disposition after headers are complete and before response body accumulation. If ESP-IDF event ordering makes this awkward, add a bounded discard path for known redirect responses.

Tests

Add cases for:

• same-origin redirect with an intermediate body larger than maxResponseBodySize;
• cross-origin redirect with header stripping;
• HTTPS-to-HTTP rejection;
• maximum redirect count;
• missing or relative Location;
• streaming and buffered parity;
• final non-redirect response still enforcing maxResponseBodySize.

Acceptance criteria

• A followable redirect is not rejected merely because its intermediate body exceeds the final buffered-body limit.
• Buffered and streaming modes make the same redirect-policy decision.
• Final response limits remain enforced.

---

Phase 7 — Strengthen persistent-client safety and validation

The persistent implementation is opt-in and should remain so for v0.1.0.

Required checks

• one client handle remains owned by exactly one worker;
• no concurrent use of one ESP-IDF handle is possible;
• origin matching includes scheme, case-insensitive host, and effective port;
• IPv6 literal normalization remains covered;
• request-specific headers and POST data are scrubbed before queue-owned memory is released;
• any setup, perform, event, buffering, parsing, callback, or scrub failure poisons and cleans the handle;
• no failed request is automatically replayed;
• shutdown cleans every retained handle before worker storage is freed;
• timeout changes on a reused handle are validated and applied per request;
• redirect origin changes replace the retained session safely;
• diagnostics cannot underflow activeHttpClients and remain balanced after shutdown.

Long-duration test

Run a same-origin HTTPS stress test comparing PerRequest and PersistentPerWorker with recorded:

• request success/failure counts;
• client creates, reuses, and cleanups;
• transport connects/disconnects;
• origin, idle, request-limit, and poisoned evictions;
• free heap and largest free block over time;
• minimum free heap;
• task stack high-water marks.

Test server behavior should include:

• keep-alive reuse;
• deliberate connection close;
• malformed/incomplete response;
• timeout;
• redirect within origin;
• redirect across origin;
• alternating GET and body-bearing methods;
• changing custom headers between requests.

---

Phase 8 — Make CI and release gating complete

Consolidate host tests

The persistent-client host tests must run in the main release-gating workflow. Either:

• merge test_persistent.cpp into the existing host test job; or
• add a dedicated persistent-host-tests job to ci.yml.

The release job must depend on it explicitly.

The separate persistent-http.yml workflow should then be removed unless it serves a distinct purpose that cannot be covered in ci.yml.

Add release-gating jobs

Release should depend on:

• metadata validation;
• formatting;
• embedded source audit;
• general host logic tests;
• persistent-client host tests;
• lifecycle/publication tests;
• example builds on all supported boards;
• Arduino CLI builds;
• shutdown/lifecycle stress sketch compilation.

Minimum dependency testing

Pin at least one CI lane to ArduinoJson 7.0.0, while another may test the current latest v7 release. This ensures the declared minimum is real.

Metadata alignment

Update library.properties to declare:

depends=ArduinoJson (>=7.0.0)

Retain matching 0.1.0 versions across library.properties and library.json.

Tag safety

Confirm that a v0.1.0 tag cannot create a GitHub release unless every release dependency succeeds on that exact tagged commit.

---

Phase 9 — Documentation updates

Update the README and relevant documents with the finalized contracts.

Lifecycle documentation

Document:

• whether public init() and deinit() calls are internally serialized;
• that fetch() is thread-safe while Running;
• behavior of submissions racing with shutdown;
• deinit() timeout and retry semantics;
• destructor blocking behavior;
• prohibition on shutdown/destruction from Link callbacks;
• exactly-one terminal callback guarantee for every accepted request.

Copy and lifetime documentation

Document:

• response and header move/copy semantics;
• explicit clone behavior and possible allocation failure;
• callback-scoped JSON lifetime;
• stream chunk lifetime;
• ownership of queued URL, headers, body, and callbacks.

Bounds documentation

Document:

• maximum timeout value;
• maximum request body representable by ESP-IDF;
• serialized JSON limit versus parsed-document heap usage;
• queue capacity including active requests;
• persistent handle bound by worker count.

Release checklist

Add a versioned release checklist requiring:

• clean CI on the exact tag commit;
• on-device lifecycle stress result;
• on-device persistent HTTPS soak result;
• heap integrity checks passing;
• balanced persistent client create/cleanup diagnostics after shutdown;
• final API and README consistency review.

---

Suggested implementation order

1. Add lifecycle serialization.
2. Make queue publication and signaling atomic.
3. Add deterministic regression tests for both races.
4. Add ESP-IDF integer-bound validation.
5. Remove unsafe implicit copy semantics.
6. Fix buffered redirect-body handling.
7. Consolidate CI and release gating.
8. Run on-device shutdown and persistent-client stress tests.
9. Update documentation and metadata.
10. Perform a final release review on the resulting commit.

Do not combine unrelated refactors with the concurrency fixes. Keep the first commits narrow enough that lifecycle invariants and queue publication can be reviewed independently.

---

Definition of done

This issue is complete only when all of the following are true:

• init() and deinit() are fully serialized across their complete operations.
• init() cannot publish Running after shutdown has started.
• Partial initialization always rolls back safely.
• Queue insertion and worker signaling form one atomic publication transaction.
• No task can call xSemaphoreGive() on a deleted _items semaphore.
• Signal failure rolls back the queue entry and diagnostics.
• Every accepted request receives exactly one terminal callback.
• Successful shutdown results in requestsCompleted == requestsSubmitted.
• Public shutdown timeout leaves a reusable Stopping state with storage intact.
• All ESP-IDF int conversions are range-validated.
• Allocation-backed copies cannot silently lose response data.
• Buffered redirects discard intermediate bodies consistently with streaming redirects.
• Persistent handles remain worker-owned, bounded, scrubbed, and balanced on shutdown.
• Persistent-client tests run in the primary release workflow.
• ArduinoJson 7.0.0 is tested and metadata declares the minimum version consistently.
• All examples compile on ESP32, S3, C3, and P4 in both supported CI toolchains.
• The lifecycle stress sketch compiles in CI and passes on actual ESP32 hardware.
• The persistent HTTPS soak test passes on hardware with stable heap metrics.
• README and docs describe the final lifecycle, memory, redirect, callback, and persistent-client contracts.
• A final review finds no remaining release-blocking issue for v0.1.0.