Add async message dispatch to loopback #11461
Merged
Conversation
This file contains 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
This PR was triggered by some test failures in some of the end-to-end IM unit tests that utilized the loopback transport to send/receive payloads from client to server and back. Since the current loopback transport processes 'transmitted' messages synchronously without completing the execution of the original context, it results in call flows that are not typical of actual devices interacting with each other. This resulted in a use-after-free error where the upon calling SendMessage() within the CommandSender, the synchronous execution resulted in the eventual destruction of the original CommandSender object immediately after SendMessage() was called. This PR adds support for asynchronous dispatch and handling of transmitted messages that is more representative of real-world CHIP node interactions to the existing loopback interface. It utilizes SystemLayer::ScheduleWork to handle the processing of the sent message as a bottom half handler. It also adds a DrainAndServiceIO method on the AppContext that will automatically drain and service the IO till all messages have been handled. Tests: - Ensured the TestCommand failure doesn't happen again.
pullapprove
bot
requested review from
anush-apple,
austinh0,
balducci-apple,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chulspro,
Damian-Nordic,
electrocucaracha,
erjiaqing,
franck-apple,
harimau-qirex,
hawk248,
jelderton,
jepenven-silabs,
jmartinez-silabs,
kghost,
kpschoedel,
LuDuda,
mlepage-google,
msandstedt,
pan-apple,
sagar-apple,
saurabhst and
selissia
pullapprove
bot
requested review from
tecimovic,
turon,
vivien-apple,
wbschiller,
woody-apple,
yufengwangca and
yunhanw-google
|
PR #11461: Size comparison from 39ff9bf to 9de3aee Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
Co-authored-by: Boris Zbarsky <[email protected]>
andy31415
approved these changes
Nov 5, 2021
|
Fast track: this updates tests. @bzbarsky-apple @kghost - could you review timeout comments? I believe for unit tests (which these cpp files seem to be using) 5 seconds should be a very long time. |
bzbarsky-apple
approved these changes
Nov 5, 2021
PSONALl
pushed a commit
to PSONALl/connectedhomeip
that referenced
this pull request
Dec 3, 2021
* Add async message dispatch to loopback This PR was triggered by some test failures in some of the end-to-end IM unit tests that utilized the loopback transport to send/receive payloads from client to server and back. Since the current loopback transport processes 'transmitted' messages synchronously without completing the execution of the original context, it results in call flows that are not typical of actual devices interacting with each other. This resulted in a use-after-free error where the upon calling SendMessage() within the CommandSender, the synchronous execution resulted in the eventual destruction of the original CommandSender object immediately after SendMessage() was called. This PR adds support for asynchronous dispatch and handling of transmitted messages that is more representative of real-world CHIP node interactions to the existing loopback interface. It utilizes SystemLayer::ScheduleWork to handle the processing of the sent message as a bottom half handler. It also adds a DrainAndServiceIO method on the AppContext that will automatically drain and service the IO till all messages have been handled. Tests: - Ensured the TestCommand failure doesn't happen again. * Apply suggestions from code review Co-authored-by: Boris Zbarsky <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]>
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.
This PR was triggered by some test failures in some of the end-to-end IM unit tests that utilized the loopback transport to send/receive payloads from client to server and back. Since the current loopback transport
processes 'transmitted' messages synchronously without completing the execution of the original context, it results in call flows that are not typical of actual devices interacting with each other. This resulted in a use-after-free error where the upon calling
SendMessage()within theCommandSender, the synchronous execution resulted in the eventual destruction of the original CommandSender object immediately afterSendMessage()was called.This PR adds support for asynchronous dispatch and handling of transmitted messages that is more representative of real-world CHIP node interactions to the existing loopback interface. It utilizes
SystemLayer::ScheduleWorkto handle the processing of sent messages as bottom half handlers.It also adds a
DrainAndServiceIO()method on the AppContext that will automatically drain and service the IO till all messages have been handled, making it easy to drive the system to quiescence.Tests: