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

Fix TCP race condition, remove fixed delay in CC #5167

Merged
merged 2 commits into from
Sep 26, 2018

Conversation

earlephilhower
Copy link
Collaborator

ClientContext::_wait_for_sent() could dereference a TCP's _pcb after
the connection was dropped by the OS, resulting in a crash.

Move the connection dropped check to catch this case, and replace
a fixed millisecond delay() with a yield and timeout value to minimize
wasted time when transmission completes.

ClientContext::_wait_for_sent9) could dereference a TCP's _pcb after
the connection was dropped by the OS, resulting in a crash.

Move the connection dropped check to catch this case, and replace
a fixed millisecond delay() with a yield and timeout value to minimize
wasted time when transmission completes.
@@ -308,37 +308,37 @@ class ClientContext
if (!_pcb)
return true;

int loop = -1;
int prevsndbuf = -1;
max_wait_ms++;
Copy link
Collaborator

Choose a reason for hiding this comment

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

max_wait_ms++; no more necessary


// force lwIP to send what can be sent
tcp_output(_pcb);

int sndbuf = tcp_sndbuf(_pcb);
if (sndbuf != prevsndbuf) {
// send buffer has changed (or first iteration)
// we received an ack: restart the loop counter
// we received an ack: restart the timeout
Copy link
Collaborator

Choose a reason for hiding this comment

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

just remove this comment, your new one is better

// wait until sent: timeout
DEBUGV(":wustmo\n");
#endif
break;
Copy link
Collaborator

@d-a-v d-a-v Sep 25, 2018

Choose a reason for hiding this comment

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

this break would be return false;

}
#endif

return max_wait_ms > 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

return true; here

@earlephilhower earlephilhower merged commit d017157 into esp8266:master Sep 26, 2018
@earlephilhower earlephilhower deleted the clientctxnull branch September 30, 2018 17:41
earlephilhower pushed a commit that referenced this pull request Oct 17, 2018
Fix bug introduced by #5167 which replaced delay() by yield().
That should have been esp_yield() which is the one delay()
calls and is safe from either SYS or CONT contexts.

Fixes #5237.
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.

2 participants