Fix management of the mNumReportsInFlight count in reporting engine. #24093
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
pullapprove
bot
requested review from
andy31415,
anush-apple,
arkq,
Byungjoo-Lee,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis,
franck-apple,
gjc13,
harimau-qirex,
harsha-rajendran,
hawk248,
jelderton,
jepenven-silabs,
jmartinez-silabs,
jtung-apple,
ksperling-apple,
lazarkov,
lpbeliveau-silabs,
LuDuda,
msandstedt,
mspang,
rgoliver,
robszewczyk and
saurabhst
pullapprove
bot
requested review from
woody-apple,
xylophone21,
yufengwangca and
yunhanw-google
|
PR #24093: Size comparison from 7fbf6da to 9541715 Increases (32 builds for bl602, bl702, cc13x2_26x2, cyw30739, k32w, linux, nrfconnect, psoc6, qpg, telink)
Decreases (7 builds for bl702, cc13x2_26x2)
Full report (35 builds for bl602, bl702, cc13x2_26x2, cyw30739, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
If a ReadHandler failed out of SendReportData (e.g. because the session it's on had been marked as defunct), we would increment mNumReportsInFlight and never decrement it. After this happened CHIP_IM_MAX_REPORTS_IN_FLIGHT times (4 by default), we would stop being able to send out any more data reports. This situation is pretty easy to trigger as follows: 1. Use chip-tool to commission a device with node id 17. 2. Start chip-tool interactive mode. 3. Run the following commands in interactive mode: onoff subscribe on-off 0 60 17 1 --keepSubscriptions true onoff subscribe on-off 0 60 17 1 --keepSubscriptions true onoff subscribe on-off 0 60 17 1 --keepSubscriptions true onoff subscribe on-off 0 60 17 1 --keepSubscriptions true onoff subscribe on-off 0 2 17 1 --keepSubscriptions true 4. quit interactive mode (Ctrl-C or quit() command). 5. Wait 60 seconds for all the subscriptions to error out. After this the device will no longer respond with data reports to any read or subscribe requests.
bzbarsky-apple
force-pushed
the
fix-readhandler-count-management
branch
from
9541715
to
549b840
Compare
tcarmelveilleux
approved these changes
Dec 15, 2022
|
PR #24093: Size comparison from 1c87f62 to 549b840 Increases (47 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, nrfconnect, psoc6, qpg, telink)
Decreases (7 builds for bl702, cc13x2_26x2)
Full report (53 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
woody-apple
approved these changes
Dec 16, 2022
bzbarsky-apple
added a commit
to bzbarsky-apple/connectedhomeip
that referenced
this pull request
Dec 16, 2022
…sion. This is testing the situation described in project-chip#24093
bzbarsky-apple
added a commit
to bzbarsky-apple/connectedhomeip
that referenced
this pull request
Dec 16, 2022
…roject-chip#24093) If a ReadHandler failed out of SendReportData (e.g. because the session it's on had been marked as defunct), we would increment mNumReportsInFlight and never decrement it. After this happened CHIP_IM_MAX_REPORTS_IN_FLIGHT times (4 by default), we would stop being able to send out any more data reports. This situation is pretty easy to trigger as follows: 1. Use chip-tool to commission a device with node id 17. 2. Start chip-tool interactive mode. 3. Run the following commands in interactive mode: onoff subscribe on-off 0 60 17 1 --keepSubscriptions true onoff subscribe on-off 0 60 17 1 --keepSubscriptions true onoff subscribe on-off 0 60 17 1 --keepSubscriptions true onoff subscribe on-off 0 60 17 1 --keepSubscriptions true onoff subscribe on-off 0 2 17 1 --keepSubscriptions true 4. quit interactive mode (Ctrl-C or quit() command). 5. Wait 60 seconds for all the subscriptions to error out. After this the device will no longer respond with data reports to any read or subscribe requests.
bzbarsky-apple
added a commit
to bzbarsky-apple/connectedhomeip
that referenced
this pull request
Dec 16, 2022
…sion. This is testing the situation described in project-chip#24093
This was referenced Dec 16, 2022
woody-apple
pushed a commit
that referenced
this pull request
Dec 16, 2022
…24093) (#24114) * Fix management of the mNumReportsInFlight count in reporting engine. (#24093) If a ReadHandler failed out of SendReportData (e.g. because the session it's on had been marked as defunct), we would increment mNumReportsInFlight and never decrement it. After this happened CHIP_IM_MAX_REPORTS_IN_FLIGHT times (4 by default), we would stop being able to send out any more data reports. This situation is pretty easy to trigger as follows: 1. Use chip-tool to commission a device with node id 17. 2. Start chip-tool interactive mode. 3. Run the following commands in interactive mode: onoff subscribe on-off 0 60 17 1 --keepSubscriptions true onoff subscribe on-off 0 60 17 1 --keepSubscriptions true onoff subscribe on-off 0 60 17 1 --keepSubscriptions true onoff subscribe on-off 0 60 17 1 --keepSubscriptions true onoff subscribe on-off 0 2 17 1 --keepSubscriptions true 4. quit interactive mode (Ctrl-C or quit() command). 5. Wait 60 seconds for all the subscriptions to error out. After this the device will no longer respond with data reports to any read or subscribe requests. * Add a test for a subscription trying to send reports on a defunct session. This is testing the situation described in #24093 * add mNumReportsInFlight check test when failing to send report Co-authored-by: yunhanw <yunhanw@google.com>
bzbarsky-apple
added a commit
to bzbarsky-apple/connectedhomeip
that referenced
this pull request
Dec 16, 2022
…sion. This is testing the situation described in project-chip#24093
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.
If a ReadHandler failed out of SendReportData (e.g. because the session it's on had been marked as defunct), we would increment mNumReportsInFlight and never decrement it. After this happened CHIP_IM_MAX_REPORTS_IN_FLIGHT times (4 by default), we would stop being able to send out any more data reports.
This situation is pretty easy to trigger as follows:
After this the device will no longer respond with data reports to any read or subscribe requests.