Skip to content
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

workaround for #3382 #3427

Closed
wants to merge 1 commit into from
Closed

workaround for #3382 #3427

wants to merge 1 commit into from

Conversation

softhack007
Copy link
Collaborator

On esp32, its possible that two threads (async_tcp, looptask) are sending out JSON via AsyncWebServer in parallel

  • serveJson() responds to json/si, and
  • updateInterfaces() is sending JSON state info using sendDataWs().

Assumed root cause: parts of AsyncWebServer are not thread-safe, and the payloads from a second ->send() call may corrupt the previous one if still being sent out. The problem should be solved properly in AsyncWebServer, however until that happens, this PR reduces the frequency of errors.

The workaround uses the INTERFACE_UPDATE_COOLDOWN timer to delay the second webSocket send command so it does not (usually) interfere with the first.

Changes tested on esp32 with the example config & preset from bug report #3382.
Testing on 8266 still to be done.

On esp32, its possible that two threads (async_tcp, looptask) are sending out JSON via AsyncWebServer in parallel 
- serveJson() responds to json/si, and 
- updateInterfaces() is sending JSON state info using sendDataWs(). 

This workaround uses the INTERFACE_UPDATE_COOLDOWN timer  to delay the second webSocket send command so it does not (usually) interfere with the first.

It looks like the root cause that parts of AsyncWebServer are not thread-safe, and the payloads from a second ->send() call may corrupt the previous one if still being sent out.  The problem should be solved properly in AsyncWebServer, however until that happens, this workaround help to reduce the frequency of errors.
@softhack007 softhack007 marked this pull request as draft October 5, 2023 13:24
@softhack007 softhack007 marked this pull request as ready for review October 5, 2023 13:25
@softhack007 softhack007 marked this pull request as draft October 5, 2023 13:25
@blazoncek
Copy link
Collaborator

@softhack007 this has been superseded by JSON buffer guard in 0.15. This PR should be no longer necessary.

@softhack007 softhack007 marked this pull request as ready for review May 1, 2024 19:55
@softhack007 softhack007 closed this May 1, 2024
@softhack007 softhack007 deleted the workaround_3382 branch May 1, 2024 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants