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

fixed http.end taking too long #6277

Merged
merged 1 commit into from
Feb 21, 2022
Merged

Conversation

yigitabi5444
Copy link
Contributor

http end takes 20-30 seconds if there is a large amount of data
replacing this read loop with flush fixes that problem

http end takes 20-30 seconds if there is a large amount of data
replacing this read loop with flush fixes that problem
@CLAassistant
Copy link

CLAassistant commented Feb 13, 2022

CLA assistant check
All committers have signed the CLA.

@mrengineer7777
Copy link
Collaborator

Discussion: Does flushing the buffer rather than reading it mean the final bytes aren't processed? Does the current read() perform any processing? I see read() is part of WiFiClient.cpp, and followed it to WiFiClientRxBuffer->read(). I don't see any processing events, but I can't be certain from a quick glance at the code.

@yigitabi5444
Copy link
Contributor Author

Im not sure if read does any processing but the same fix was implemented over here: #828

@mrengineer7777
Copy link
Collaborator

@yigitabi5444 So you are making the HTTPClient.disconnect() behavior consistent with HTTPClient.end().
BTW, the change for end() is here: e3a5ae4

Seems reasonable. I reviewed the commit history for HTTPClient.cpp and see no regressions for end() or disconnect() since 2018. This PR seems like a good fix. Just not sure if read() does any processing. Hopefully Espressif employees can clarify.

@me-no-dev
Copy link
Member

thanks @yigitabi5444

@lemonkey
Copy link

lemonkey commented Aug 27, 2022

WARNING: changing from looping while available() > 0 and reading the bytes on the client one at a time to calling flush() can run into issues when available() returns a value > 0 but the socket is no longer connected. WiFiClient::flush( ) currently gets stuck in a loop and never returns for this scenario.

This also impacts HTTPClient.disconnect() which was similarly updated recently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants