-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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 race condition in WiFiGenericClass::hostByName #8672
Conversation
This is good to have! @MattiasTF could you please fix the merge conflicts, so that we can merge this? |
What would be your preferred fix? I can rebase the branch to give you a single patch. |
@MattiasTF that would be great. We just can't merge it right now and can not rebase it for you, because you did not select to allow us to do that when you created the PR. |
dns_gethostbyname, as used in hostByName, is required to run in lwIP's TCP/IP context. This can be verified by enabling LWIP_CHECK_THREAD_SAFETY in the sdkconfig. Calling dns_gethostbyname from the Arduino task can trigger race conditions in lwIP or lower layers. One possibility is a corruption of IDF's Ethernet buffers, causing an unstoppable flood of "insufficient TX buffer size" errors, effectively severing all Ethernet connectivity. This patch makes sure to call dns_gethostbyname from lwIP's TCP/IP context.
efaa628
to
e64beb1
Compare
There we go.
Thanks for the info. Can’t seem to find that option at the moment when creating a new PR, though. |
Thanks, but I just found out that that option only exists when creating PRs from user-owned forks. It’s not possible to use that option with forks living in company repos, which is why I don’t have it. |
That is good to know! Thanks for researching it :) |
Description of Change
dns_gethostbyname
, as used inhostByName
, is required to run in lwIP’s TCP/IP context. This can be verified by enablingLWIP_CHECK_THREAD_SAFETY
in the sdkconfig.Calling
dns_gethostbyname
from the Arduino task can trigger race conditions in lwIP or lower layers. One possibility is a corruption of IDF's Ethernet buffers, causing an unstoppable flood of "insufficient TX buffer size" errors, effectively severing all Ethernet connectivity.hostByName
cannot be executed in the TCP/IP context because it blocks until completion, preventing the TCP/IP task from receiving the DNS response to unblock itself, causing a deadlock. Therefore, this PR patcheshostByName
to executedns_gethostbyname
in the TCP/IP context while still blocking only the Arduino task.Tests scenarios
I have tested my PR on Arduino-esp32 master as of 02e31b4 on a custom ESP32 board.