-
Notifications
You must be signed in to change notification settings - Fork 41
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
ESP8266 TCP - connection closure error #4
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that ERR_OK is appropriate here. This is not a premature abort, but rather a normal shutdown.
That said, looking at the code in lwip/core/tcp_in.c, there is very little difference between the code paths in the two cases. The only difference is that, in the OK case, tcp_output() is called before falling through to the aborted: label. I think that is a good thing, as it permits the stack to proceed apace with the final stages of the connection close dance.
Regarding the timing of tcp_recved versus handle-data, it's unclear to me what is correct. On the one hand, calling tcp_recved early would potentially speed things up by getting the TCP ACK out sooner. On the other hand, it seems prudent to make sure the data is safe before proceeding. On the other other hand, the data is probably safe until the pbuf is freed.
Analysis of tcp_in.c suggests that, before the pbuf is passed to the application, it is disconnected from the pcb, so it should be safe to call tcp_recved before handling the data. That being the case, I have moved the tcp-recved call up to before handle-data. We can keep our fingers crossed.