fix(http): prevent use-after-free from synchronous abort/close()

[willmmiles]

Alternative implementation for #464 using std::shared_ptr instead of locks.

Ensure that AsyncWebRequest objects are held in scope until the end of their handler functions, avoiding any possible use-after-free cases when aborted or closed. This is managed by using std::shared_ptr to allow a scoped lifecycle "lock" to be taken without an OS mutex.

[Copilot]

Pull request overview

This PR changes the HTTP request lifetime management in ESPAsyncWebServer to prevent request/client destruction during synchronous abort()/close() re-entrancy (as introduced by AsyncTCP’s synchronous callbacks), by moving AsyncWebServerRequest to a std::shared_ptr-managed lifecycle and holding scoped references during key TCP callbacks.

Changes:

• Introduces AsyncWebServerRequest::create() factory and stores a self-referential std::shared_ptr to control request lifetime.
• Updates request TCP callbacks to hold a local shared_ptr in _onData/_onPoll/_onAck to prevent premature destruction during nested disconnect paths.
• Removes the server-side _handleDisconnect() deletion path and adjusts auth failure handling to avoid leaking responses / accessing the request after abort.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/WebServer.cpp	Creates requests via the new AsyncWebServerRequest::create() shared_ptr factory and removes server-driven deletion.
src/WebRequest.cpp	Adds scoped shared_ptr holds in several callbacks and switches disconnect cleanup to releasing the self-reference; fixes auth allocation-failure paths.
src/ESPAsyncWebServer.h	Makes the request constructor private and adds the create() factory that initializes the self-owned shared_ptr.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mathieucarbou]

@willmmiles : I wasn't able to push a commit until your branch so I copied yours into origin repo so that you and I can push and commit. (and have someone test the changes).,

Here is the result: #466

It contains 2 commits, yours plus one that I added to fix the discussions above.

I tested with the WebSocket example and it works fine now.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/WebRequest.cpp:1488

• clientRelease() calls _this.reset(). If _this is the last owning shared_ptr, this can destroy the request object while std::shared_ptr::reset() is still executing on the _this member subobject, which is undefined behavior (same issue this PR is avoiding in _onDisconnect()). Break the self-reference by moving _this to a local variable instead.

  // Now that we are no longer bound to the client, self-destruct at the earliest opportunity
  _this.reset();
  return c;

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/AsyncEventSource.h:150

• The constructor documentation is now out of sync with the actual signature: it references AsyncWebServerRequest::releaseClient() and documents a request parameter, but the constructor takes an AsyncClient* and the request method is clientRelease(). This makes the public API docs misleading for consumers.

   * @brief Construct a new Async Event Source Client object
   * @note constructor is normally passed a client object from AsyncWebServerRequest::releaseClient(); see AsyncEventSourceResponse::_respond()
   *
   * @param request
   * @param server

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

src/WebRequest.cpp:1485

• Same as above: use explicit nullptrs to clear AsyncClient callbacks. Using {} risks overload/initializer-list ambiguity and is inconsistent with existing callback-clearing patterns in the repo.

  c->onTimeout({}, nullptr);
  c->onData({}, nullptr);
  c->onPoll({}, nullptr);

src/AsyncEventSource.h:148

• The doc comment refers to AsyncWebServerRequest::releaseClient(), but the API is clientRelease(). This makes the public header documentation misleading for users migrating to the new ownership model.

   * @brief Construct a new Async Event Source Client object
   * @note constructor is normally passed a client object from AsyncWebServerRequest::releaseClient(); see AsyncEventSourceResponse::_respond()
   *

src/AsyncWebSocket.h:246

• Removing the public AsyncWebSocketClient(AsyncWebServerRequest*, AsyncWebSocket*) ctor is an API breaking change for any downstream code that instantiated websocket clients directly. If this constructor was intended to be public, consider keeping a deprecated overload (or clearly documenting the migration path in release notes).

  void *_tempObject;

  AsyncWebSocketClient(AsyncClient *client, AsyncWebSocket *server);
  ~AsyncWebSocketClient();

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[mathieucarbou]

I tested the PR and I didn't see any issue.
When running too many SSE clients (16) in the perf test ESP crashes because of memory pressure but this is not linked to the changes. And I defaulted the logger to DEBUG.