lwIP: avoid stale PCB use after remote close

[meganetaaan]

Summary

Prevent a delayed receive-window notification from calling tcp_recved() with a freed lwIP tcp_pcb after the remote peer closes a connection while received data remains buffered.

Root cause

When tcpReceive() receives a terminal notification (pb == NULL or a receive error), it currently clears the receive, sent, and error callbacks. Buffered data may still be owned by the socket and consumed later from the XS task.

When that data is consumed, tcp_recved_safe() stores the current raw tcp_pcb * in a heap-allocated message and posts it with tcpip_callback_with_block(). The block argument only blocks until the message can be posted; it does not wait for the callback to run.

The PCB can therefore be destroyed before tcp_recved_INLWIP() executes. Because the terminal receive path already removed tcp_err, tcpError() cannot clear the owner's socket pointer. The queued callback then calls tcp_recved() with a stale PCB.

The observed ESP32-S3 panic ended in:

tcp_update_rcv_ann_wnd
tcp_recved
tcp_recved_INLWIP
tcpip_thread_handle_msg
tcpip_thread

GDB showed that the PCB contents had already been overwritten and that it was no longer present in lwIP's active PCB lists, while the socket owner still retained the old pointer.

Changes

• On terminal receive, disable the receive and sent callbacks but retain the error callback and callback argument until final teardown.
• Change tcp_recved_safe() to receive a pointer to the owner's PCB pointer.
• Use synchronous tcpip_api_call() and re-read that pointer after earlier queued lwIP work has run.
• Skip tcp_recved() if tcpError() has already invalidated the owner's pointer.
• Update both the ECMA-419 io/socket implementation and the legacy network/socket implementation.

The synchronous call also removes the heap allocation and deferred callback previously used by tcp_recved_safe().

Relationship to #1655 and #1656

PR #1656 prevents local XS-side socket teardown from racing an lwIP callback that is already being dispatched.

This change addresses a different lifetime direction: after a remote close, an XS-side receive notification can retain a PCB that is destroyed before the queued notification executes.

This PR does not attempt to resolve the separate tcp->buffers node corruption still discussed in #1655.

Validation

Tested on an M5Stack CoreS3 with Moddable 9.0.0 and ESP-IDF 6.0.2.

The reproducer streams data to the device and closes the server side immediately after the response, leaving unread socket buffers. The unpatched build reproduced the tcp_recved() panic and reboot.

With this change:

• One real-time-paced fixture run terminated normally.
• Three unpaced, higher-load fixture runs terminated normally.
• No tcp_recved() panic, task deadlock, or device reboot was observed.
• The complete CoreS3 application builds successfully.

No automated test is included because the failure depends on the ESP32 lwIP tcpip task and native PCB destruction order; a JavaScript fake socket does not exercise that lifetime.

[phoddie]

Thank you for the report and proposed fix.

I've been aware of this as a potential issue for some time, but never seen it happen in the real world. I'd prefer to have a way to reproduce the behavior to be able to evaluate any change's runtime behavior. But, maybe that is impractical here? How are you seeing this happen?

Additionally, the proposed patch changes the prototype of the lwip APIs. That may be inescapable, but we should consider alternatives (maybe you did?), such as passing the TCP record.

The original implementation puts the marshaled parameters into a memory block created with malloc and frees that after completion. The change puts that record on the stack, which seems dangerous for a potential cross-thread call. My understanding is that tcpip_callback_with_block() runs the callback asynchronously.

If this issue happens with tcp_recved_safe() it seems possible other APIs could be impacted. A quick scan suggests all other asynchronous calls to lwip don't have the same possible issue. Did you review for that?