Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HTTPD] support for multiple simultaneous requests (IDFGH-9214) #10602

Conversation

chipweinberger
Copy link
Contributor

@chipweinberger chipweinberger commented Jan 24, 2023

Rejecting in favor of new PR: #10669

Related Issue: #10594

Add basic support for supporting multiple simultaneous requests in HTTPD server.

Implementation

  • adds httpd_req_copy(dest, source) so that request memory ownership can be given to the user
  • adds httpd_req_claim_ownership(req) to fully give ownership of the req to another thread, so the socket will not get purged by lru_purge_enable, until httpd_req_relinquish_ownership(req) is called.

Open Question 1: It might be better to remove httpd_req_copy(dest, source), and instead have the server allocate a new request object for each request. Possibly behind a, httpd_server.new_request_per_handler, flag. Or, have httpd_req_claim_ownership(req) return a copy of the request.

Open Question 2: Instead of calling httpd_req_claim_ownership(req), the user's request handler could return ESP_ERR_NOT_FINISHED, which would do the same thing. The user would then call httpd_req_finish(req) to mark the socket as purgeable again.

In later ESP-IDF versions, we should probably make HTTPD have more "proper" simultaneous request support, at which point we could deprecate these functions perhaps. Lots of people have use cases for this, and this approach gives lots of control to users.

Example

  • add an example to ESP-IDF, showing usage.

Testing

  • tested on a ESP32-S3

@chipweinberger chipweinberger force-pushed the user/chip/http-multiple-requests branch 2 times, most recently from 8382df4 to a940faa Compare January 24, 2023 06:30
@chipweinberger chipweinberger force-pushed the user/chip/http-multiple-requests branch 3 times, most recently from 5e0a1a1 to 46aa335 Compare January 24, 2023 06:48
@espressif-bot espressif-bot added the Status: Opened Issue is new label Jan 24, 2023
@github-actions github-actions bot changed the title [HTTPD] support for multiple simultaneous requests [HTTPD] support for multiple simultaneous requests (IDFGH-9214) Jan 24, 2023
@chipweinberger chipweinberger force-pushed the user/chip/http-multiple-requests branch 4 times, most recently from 533ff91 to 90c17f2 Compare January 24, 2023 07:17
@chipweinberger chipweinberger force-pushed the user/chip/http-multiple-requests branch from 90c17f2 to dc0d40e Compare January 24, 2023 07:24
@chipweinberger
Copy link
Contributor Author

This PR is more of an experiment, that I wanted to share.

If we merge something like this, I like the ESP_ERR_NOT_FINISHED approach with a httpd_server.new_request_per_handler flag.

@chipweinberger
Copy link
Contributor Author

chipweinberger commented Jan 31, 2023

Actually thought of an even better idea.

Instead of httpd_server.new_request_per_handler, a new req obj will be allocated after ESP_ERR_NOT_FINISHED is returned. So the req obj can still be used by the handler, but the next request will use a new object.

@chipweinberger
Copy link
Contributor Author

new PR: #10669

@chipweinberger chipweinberger deleted the user/chip/http-multiple-requests branch May 20, 2023 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Opened Issue is new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants