ESP32 ECMA-419 TCP: fix two lwIP/XS task races under connection churn#1656
Open
xymeow wants to merge 1 commit into
Open
ESP32 ECMA-419 TCP: fix two lwIP/XS task races under connection churn#1656xymeow wants to merge 1 commit into
xymeow wants to merge 1 commit into
Conversation
… churn Fixes two use-after-free races behind the LoadProhibited panics reported in Moddable-OpenSource#1655, both between the lwIP tcpip task and the XS task: 1. Socket teardown: removeTCPCallbacks now clears the callback argument (tcp_arg) before clearing the callbacks, and tcpReceive/tcpSent/ tcpError bail out on a NULL argument, so a late delivery - including lwIP refused-data redelivery - cannot touch a freed TCP record. 2. Receive-buffer list: tcpReceive (lwIP task) walks to the tail of tcp->buffers to append while xs_tcp_read (XS task) pops and frees head nodes; the unsynchronized tail walk could traverse a node mid-free. Both sides now do their pointer surgery inside builtinCriticalSection, with heap operations kept outside. Verified on ESP32-S3 (M5Stack CoreS3) under the reproducing workload - an HTTP server polled by a LAN client, previously crashing every 15-25 minutes: hours of clean uptime with both fixes applied, instrumented to exclude unrelated app-level restarts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #1655.
Two independent races between the lwIP
tcpiptask and the XS task producedLoadProhibitedpanics (rst:0xc) under ordinary HTTP connection churn on ESP32-S3:Socket teardown.
removeTCPCallbacks()now clears the callback argument first (tcp_arg(pcb, NULL)) and then the callbacks, per @phoddie's suggestion in the issue — plain pointer stores, no marshaling.tcpReceive/tcpSent/tcpErrorbail out on a NULL argument (withtcpReceivefreeing the pbuf), so late deliveries — including lwIP refused-data redelivery, which appears in one captured backtrace viatcp_process_refused_data— cannot touch a freedTCPrecord.Receive-buffer list.
tcpReceive(lwIP task) walks to the tail oftcp->buffersto append whilexs_tcp_read(XS task) pops and frees head nodes. The unsynchronized tail walk could traverse a node mid-free — this matches the two captured backtraces pointing at thewalker->nextwalk. Both sides now do their pointer surgery insidebuiltinCriticalSection, with heap operations (c_free,pbuf_free,tcp_recved_safe) kept outside the critical section.Verification: ESP32-S3 (M5Stack CoreS3), ESP-IDF 6.0, release build, serving HTTP to a LAN poller — the workload that previously crashed every 15–25 minutes (24 resets captured in one evening). With both fixes: hours of continuous clean uptime, instrumented via an abort hook so unrelated app-level restarts are excluded from the count. Soak details and symbolicated backtraces are in #1655.
CLA: the completed individual CLA has been emailed to info@moddable.com.
🤖 Generated with Claude Code