Conversation
|
Hi @ zyk6271, Thanks for the PR proposal. Unfortunately, I don't have time to review and test the related cases for now. I will take a look once have time for this. Please consider to extend your PR with the logs of stress testing with fault injection. |
|
Implemented the requested stress/fault-injection follow-up in commit Key results on ESP32-D0WD-V3 / ESP-IDF v6.0.2: No The follow-up also fixes the accepted-socket/address leak itself, preserves TCP master reconnect definitions during fatal-error recovery, and removes negative-fd |
|
@zyk6271 , Thanks. for the update. I just reviewed it quickly.
I have some related fixes in my other opened MR. So, your PR needs to be rebased to main. I will check all aspects later. This PR will follow the formal process and I will inform you about status. |
Release accepted sockets and address metadata on all failure paths, close stale slave clients after fatal network-stack errors, and keep the master node table intact for reconnects. Also fix sparse-node timeout scanning and avoid using negative socket descriptors as fd_set indexes.
466cdb5 to
73a67f8
Compare
|
Rebased this PR onto the latest The two patch commits were preserved unchanged according to
I also reviewed the related changes already present on |
|
I have a lot of fixes in the separate MRs that are not yet merged into main. You can check and keep your PR with minimal changes. Once possible to continue, I will verify and take your original commit and adopt it with additional fix commit in my branch as required. This will take time as per formal process. I will inform you if I have any further questions or information. Thanks for contribution. |
|
A production Modbus TCP slave of mine stopped answering: port 502 completed the TCP handshake and reset the connection on the first request, and the web server on the same board stopped answering too. Nothing remote could recover it; bouncing its switch port did. I reproduced it on a spare board of the same model, and it is the same Setup: Waveshare ESP32-S3-POE-ETH-8DI-8RO (W5500 Ethernet), ESP-IDF 5.5.4, esp-modbus 2.1.3 as a TCP slave, Trigger: a host holds 6 HTTP connections and opens 8 Modbus connections at the same time, which runs lwIP out of sockets. Unpatched 2.1.3 (2 of 2 runs): The backtrace is A side note on the #127 suggestion: zero-initialising With this PR: I tested two builds on the same board, with the same trigger and application code:
In both builds the uptime kept advancing, and the HTTP server answered throughout. 30 rounds each of connect/read/close, connect/read/RST and connect/close without a request left all 5 slots free. The Short test, not a long soak. I did not test a peer that disappears without a FIN or RST, or the heap-pressure path from your description. |
|
I don't think it is reliable to open maximum number of sockets in the production project with limited resources. However, your stress test results show the error handling issue clearly and are useful. I will consider increasing the priority of this PR to merge it as soon as possible and merge other related MRs with related fixes later.
I tried to verify some aspects for this PR and the socket accept limitation for the TCP Slave from LWIP side. It looks like the TCP Slave can not accept more than 7 connections, and resets all following connections, even if Thanks for test report. |
Summary
fd_seterrnologging whenselect()repeatedly failsRoot cause
Two failure modes combined in the reported long-running failure:
accept()/select()returnedENOMEM, Wi-Fi allocation failed, and connectivity was lost.select()immediately on persistent errors. The high-prioritymb_drv_tcp_taskcould therefore starve CPU0's IDLE task and trigger the task watchdog.The previous code also copied the full read set into
exceptfds, including non-socket descriptors, and did not consume exceptional client conditions.Changes
accept()errno, avoidclose(-1), and handle address/strdup()failuresopen_setselect()failuresHardware validation
Target: ESP32-D0WD-V3, ESP-IDF v6.0.2, esp-modbus v2.1.3 component sources, Modbus TCP slave with five client slots.
A/B resource-pressure fault injection
The fault injector holds all five clients open, then repeatedly creates extra TCP connections.
Unpatched firmware:
Patched firmware, with the same temporary heap telemetry:
A fresh run after the final source-review fixes:
Additional fault injection/regression
During the patched test windows there was no
errno=12,wifi:mem fail,wifi:m f null, select busy loop, or task WDT. A singleerrno=113while Wi-Fi was physically unavailable was expected and recovery completed after reassociation.Build validation
0xfdb30; smallest app partition remained 32% freegit diff --checkpassed for all five changed library files