Skip to content

ESP32 ECMA-419 TCP: fix two lwIP/XS task races under connection churn #1656

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.

Sign up for GitHub

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

Open
xymeow wants to merge 1 commit into
base: public
Choose a base branch
from
+23 −0
Open

ESP32 ECMA-419 TCP: fix two lwIP/XS task races under connection churn #1656

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions modules/io/socket/lwip/tcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,10 @@ void xs_tcp_destructor(void *data)
void removeTCPCallbacks(TCP tcp)
{
if (tcp->skt) {
// clear the argument first so a late callback observes NULL
// (the lwIP tcpip task can deliver segments - including refused-data
// redelivery - after the XS task tears this socket down)
tcp_arg(tcp->skt, NULL);
tcp_recv(tcp->skt, NULL);
tcp_sent(tcp->skt, NULL);
tcp_err(tcp->skt, NULL);
Expand Down Expand Up @@ -355,7 +359,9 @@ void xs_tcp_read(xsMachine *the)
buffer->fragment = fragment->next;
buffer->fragmentOffset = 0;
if (NULL == buffer->fragment) {
builtinCriticalSectionBegin();
tcp->buffers = buffer->next;
builtinCriticalSectionEnd();
tcp_recved_safe(tcp->skt, buffer->pb->tot_len);
pbuf_free_safe(buffer->pb);
c_free(buffer);
Expand Down Expand Up @@ -532,6 +538,9 @@ void tcpError(void *arg, err_t err)
{
TCP tcp = arg;

if (NULL == tcp) // callbacks cleared during teardown
return;

removeTCPCallbacks(tcp);
tcp->skt = NULL; // "pcb is already freed when this callback is called"
if (kTCPError & tcp->triggerable)
Expand All @@ -543,6 +552,12 @@ err_t tcpReceive(void *arg, struct tcp_pcb *pcb, struct pbuf *pb, err_t err)
TCP tcp = arg;
TCPBuffer buffer;

if (NULL == tcp) { // callbacks cleared during teardown
if (pb)
pbuf_free(pb);
return ERR_OK;
}

if ((NULL == pb) || (ERR_OK != err)) { //@@ when is err set here?
removeTCPCallbacks(tcp);
#if ESP32
Expand All @@ -569,6 +584,10 @@ err_t tcpReceive(void *arg, struct tcp_pcb *pcb, struct pbuf *pb, err_t err)

modInstrumentationAdjust(NetworkBytesRead, buffer->bytes);

// xs_tcp_read pops and frees head nodes on the XS task while this runs
// on the lwIP task; guard the pointer surgery so the tail walk cannot
// traverse a node being detached and freed
builtinCriticalSectionBegin();
if (tcp->buffers) {
TCPBuffer walker;

Expand All @@ -578,6 +597,7 @@ err_t tcpReceive(void *arg, struct tcp_pcb *pcb, struct pbuf *pb, err_t err)
}
else
tcp->buffers = buffer;
builtinCriticalSectionEnd();

if (tcp->triggerable & kTCPReadable)
tcpTrigger(tcp, kTCPReadable);
Expand All @@ -587,6 +607,9 @@ err_t tcpReceive(void *arg, struct tcp_pcb *pcb, struct pbuf *pb, err_t err)

err_t tcpSent(void *arg, struct tcp_pcb *pcb, u16_t len)
{
if (NULL == arg) // callbacks cleared during teardown
return ERR_OK;

tcpTrigger((TCP)arg, kTCPWritable);
return ERR_OK;
}
Expand Down