Skip to content

Fix WiFiClientSecure::available() blocking on dropped connections #6449

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

Merged
merged 13 commits into from
Sep 17, 2019

Conversation

johnm545
Copy link
Contributor

When a secure socket connection is dropped somewhere between the access point and the server, write attempts eventually fill the WiFiClient's tx buffer. At this point, calls to WiFiClientSecure::available() block for 5 (default) seconds. See here for full description and example: #6434

This happens because WiFiClientSecure::available() calls _run_until(BR_SSL_RECVAPP, false), and this also triggers a blocking write attempt if there is still record data hanging around from previous write attempts (i.e. state & BR_SSL_SENDREC == true). The fix is to avoid writing to the WiFiClient more data than will fit in its tx buffer when the blocking flag == false, by first checking WiFiClient::availableForWrite(). This also works fine when availableForWrite() == 0 (WiFiClient::write just returns 0 instantly if the size argument is 0).

I have also removed the if (blocking) guard from the optimistic_yield call here. A yield is required to avoid WDT reset if available() is called in a busy loop (as is common). The WiFiClient::available() method includes a yield, so including one in the WiFiClientSecure method is consistent.

Added a check of WiFiClient::availableForWrite to prevent blocking writes when the _run_until blocking flag is false
Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks

@johnm545
Copy link
Contributor Author

I've just pushed a fix for #6464 here too, as it's also a dropped connection issue and changes are in the same method

@earlephilhower earlephilhower added this to the 2.6.0 milestone Sep 5, 2019
@d-a-v d-a-v added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Sep 11, 2019
Copy link
Collaborator

@d-a-v d-a-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@d-a-v d-a-v removed the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Sep 16, 2019
Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM now.

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.

4 participants