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

Fix failing TestTimedHandler unit test. #12470

Merged
merged 1 commit into from
Dec 2, 2021

Conversation

bzbarsky-apple
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple commented Dec 2, 2021

This is failing due to a merge conflict between
#11988 and
#12389: the latter
ends up in an error state as described in
#12466 (comment)
and the former makes our code a lot more sensitive to being in that
error state.

The fix for the test is to not use the sync mode of loopback
transport, which allows the stack for sending a message to unwind
before responses are delivered and avoids the "object deleted by
response while we are still working with it" problem described in
#12466 (comment).

When the responses were made async, it turned out the test was missing
some "expect response" flags that should have been there all along and
it was only passing because the response happened before the send
could get to the "close the exchange" stage. With async responses,
exchanges were closing too early without the "expect response" flags.

Separately we should figure out which parts of
#12466 we should
do.

This is failing due to a merge conflict between
project-chip#11988 and
project-chip#12389: the latter
ends up in an error state as described in
project-chip#12466 (comment)
and the former makes our code a lot more sensitive to being in that
error state.

The fix for the test is to not use the sync mode of loopback
transport, which allows the stack for sending a message to unwind
before responses are delivered and avoids the "object deleted by
response while we are still working with it" problem described in
project-chip#12466 (comment).

When the responses were made async, it turned out the test was missing
some "expect response" flags that should have been there all along and
it was only passing because the response happened before the send
could get to the "close the exchange" stage.  With async responses,
exchanges were closing too early without the "expect response" flags.

Separately we should figure out which parts of
project-chip#12466 we should
do.
@github-actions
Copy link

github-actions bot commented Dec 2, 2021

PR #12470: Size comparison from 7b214c9 to 6408b68

Increases (25 builds for efr32, esp32, k32w, linux, mbed, p6, qpg, telink)
platform target config section 7b214c9 6408b68 change % change
efr32 lighting-app BRD4161A (read only) 730024 730064 40 0.0
.text 730016 730056 40 0.0
BRD4161A+rpc (read only) 758672 758704 32 0.0
.text 758664 758696 32 0.0
lock-app BRD4161A (read only) 703900 703924 24 0.0
.text 703892 703916 24 0.0
window-app BRD4161A (read only) 706996 707020 24 0.0
.text 706988 707012 24 0.0
esp32 all-clusters-app c3devkit (read only) 839092 839114 22 0.0
(read/write) 1225074 1225090 16 0.0
.dram0.data 14028 14036 8 0.1
.flash.text 839092 839114 22 0.0
m5stack (read only) 910767 910791 24 0.0
(read/write) 424304 424312 8 0.0
.flash.rodata 194800 194808 8 0.0
.flash.text 905383 905407 24 0.0
k32w lighting-app k32w061+se05x+release (read/write) 727908 727928 20 0.0
.text 641252 641272 20 0.0
lock-app k32w061+debug (read/write) 616896 616916 20 0.0
.text 539836 539856 20 0.0
shell k32w061+debug (read/write) 682784 682804 20 0.0
.text 594096 594116 20 0.0
linux all-clusters-app debug (read only) 1796985 1797065 80 0.0
.text 1510738 1510818 80 0.0
bridge-app debug+rpc (read only) 1367205 1367285 80 0.0
.text 1147509 1147589 80 0.0
chip-tool debug (read only) 6528205 6528285 80 0.0
.text 5816613 5816693 80 0.0
lighting-app debug+rpc (read only) 1651033 1651097 64 0.0
.text 1373842 1373906 64 0.0
ota-provider-app debug (read only) 1327801 1327881 80 0.0
.text 1106434 1106514 80 0.0
ota-requestor-app debug (read only) 1427857 1427937 80 0.0
.text 1191106 1191186 80 0.0
tv-app debug (read only) 1944721 1944817 96 0.0
.rodata 168680 168712 32 0.0
.text 1626882 1626946 64 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read/write) 2299368 2299376 8 0.0
.text 1261968 1261976 8 0.0
lighting-app CY8CPROTO_062_4343W+release (read/write) 2284096 2284104 8 0.0
.text 1246696 1246704 8 0.0
lock-app CY8CPROTO_062_4343W+release (read/write) 2257128 2257200 72 0.0
.text 1219728 1219800 72 0.0
p6 all-clusters-app default (read/write) 2320800 2320840 40 0.0
.text 1279064 1279104 40 0.0
light-app default (read/write) 2256600 2256624 24 0.0
.text 1214864 1214888 24 0.0
lock-app default (read/write) 2231944 2231984 40 0.0
.text 1190208 1190248 40 0.0
qpg lighting-app qpg6100+debug (read only) 499780 499800 20 0.0
.text 494460 494480 20 0.0
lock-app qpg6100+debug (read only) 472592 472612 20 0.0
.text 467272 467292 20 0.0
telink lighting-app tlsr9518adk80d (read/write) 782718 782734 16 0.0
text 543954 543976 22 0.0
Full report (29 builds for efr32, esp32, k32w, linux, mbed, p6, qpg, telink)
platform target config section 7b214c9 6408b68 change % change
efr32 lighting-app BRD4161A (read only) 730024 730064 40 0.0
(read/write) 119660 119660 0 0.0
.bss 117844 117844 0 0.0
.data 1812 1812 0 0.0
.text 730016 730056 40 0.0
BRD4161A+rpc (read only) 758672 758704 32 0.0
(read/write) 137964 137964 0 0.0
.bss 136044 136044 0 0.0
.data 1920 1920 0 0.0
.text 758664 758696 32 0.0
lock-app BRD4161A (read only) 703900 703924 24 0.0
(read/write) 117364 117364 0 0.0
.bss 115596 115596 0 0.0
.data 1768 1768 0 0.0
.text 703892 703916 24 0.0
window-app BRD4161A (read only) 706996 707020 24 0.0
(read/write) 117788 117788 0 0.0
.bss 116012 116012 0 0.0
.data 1776 1776 0 0.0
.text 706988 707012 24 0.0
esp32 all-clusters-app c3devkit (read only) 839092 839114 22 0.0
(read/write) 1225074 1225090 16 0.0
.dram0.bss 58824 58824 0 0.0
.dram0.data 14028 14036 8 0.1
.flash.rodata 166968 166968 0 0.0
.flash.text 839092 839114 22 0.0
.iram0.text 61394 61394 0 0.0
m5stack (read only) 910767 910791 24 0.0
(read/write) 424304 424312 8 0.0
.dram0.bss 64224 64224 0 0.0
.dram0.data 34000 34000 0 0.0
.flash.rodata 194800 194808 8 0.0
.flash.text 905383 905407 24 0.0
.iram0.text 122943 122943 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 727908 727928 20 0.0
.bss 79012 79012 0 0.0
.data 1844 1844 0 0.0
.text 641252 641272 20 0.0
lock-app k32w061+debug (read/write) 616896 616916 20 0.0
.bss 69452 69452 0 0.0
.data 1808 1808 0 0.0
.text 539836 539856 20 0.0
shell k32w061+debug (read/write) 682784 682804 20 0.0
.bss 81108 81108 0 0.0
.data 1780 1780 0 0.0
.text 594096 594116 20 0.0
linux all-clusters-app debug (read only) 1796985 1797065 80 0.0
(read/write) 125992 125992 0 0.0
.bss 53456 53456 0 0.0
.data 1104 1104 0 0.0
.data.rel.ro 66128 66128 0 0.0
.dynamic 592 592 0 0.0
.got 4112 4112 0 0.0
.init 27 27 0 0.0
.init_array 560 560 0 0.0
.rodata 145909 145909 0 0.0
.text 1510738 1510818 80 0.0
bridge-app debug+rpc (read only) 1367205 1367285 80 0.0
(read/write) 71920 71920 0 0.0
.bss 35440 35440 0 0.0
.data 1680 1680 0 0.0
.data.rel.ro 29792 29792 0 0.0
.dynamic 592 592 0 0.0
.got 3984 3984 0 0.0
.init 27 27 0 0.0
.init_array 408 408 0 0.0
.rodata 118860 118860 0 0.0
.text 1147509 1147589 80 0.0
chip-tool debug (read only) 6528205 6528285 80 0.0
(read/write) 199760 199760 0 0.0
.bss 33736 33736 0 0.0
.data 1008 1008 0 0.0
.data.rel.ro 159432 159432 0 0.0
.dynamic 592 592 0 0.0
.got 4464 4464 0 0.0
.init 27 27 0 0.0
.init_array 480 480 0 0.0
.rodata 304984 304984 0 0.0
.text 5816613 5816693 80 0.0
lighting-app debug+rpc (read only) 1651033 1651097 64 0.0
(read/write) 105008 105008 0 0.0
.bss 41136 41136 0 0.0
.data 1264 1264 0 0.0
.data.rel.ro 57296 57296 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 536 536 0 0.0
.rodata 138609 138609 0 0.0
.text 1373842 1373906 64 0.0
ota-provider-app debug (read only) 1327801 1327881 80 0.0
(read/write) 70376 70376 0 0.0
.bss 38016 38016 0 0.0
.data 912 912 0 0.0
.data.rel.ro 26328 26328 0 0.0
.dynamic 592 592 0 0.0
.got 4048 4048 0 0.0
.init 27 27 0 0.0
.init_array 448 448 0 0.0
.rodata 120296 120296 0 0.0
.text 1106434 1106514 80 0.0
ota-requestor-app debug (read only) 1427857 1427937 80 0.0
(read/write) 74272 74272 0 0.0
.bss 40128 40128 0 0.0
.data 976 976 0 0.0
.data.rel.ro 28040 28040 0 0.0
.dynamic 592 592 0 0.0
.got 4032 4032 0 0.0
.init 27 27 0 0.0
.init_array 472 472 0 0.0
.rodata 132240 132240 0 0.0
.text 1191106 1191186 80 0.0
shell debug (read only) 812841 812841 0 0.0
(read/write) 60264 60264 0 0.0
.bss 16904 16904 0 0.0
.data 240 240 0 0.0
.data.rel.ro 38656 38656 0 0.0
.dynamic 592 592 0 0.0
.got 3504 3504 0 0.0
.init 27 27 0 0.0
.init_array 344 344 0 0.0
.rodata 83506 83506 0 0.0
.text 623250 623250 0 0.0
tv-app debug (read only) 1944721 1944817 96 0.0
(read/write) 314640 314640 0 0.0
.bss 245496 245496 0 0.0
.data 1504 1504 0 0.0
.data.rel.ro 61984 61984 0 0.0
.dynamic 592 592 0 0.0
.got 4424 4424 0 0.0
.init 27 27 0 0.0
.init_array 624 624 0 0.0
.rodata 168680 168712 32 0.0
.text 1626882 1626946 64 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2299368 2299376 8 0.0
.bss 181892 181892 0 0.0
.data 5168 5168 0 0.0
.heap 849384 849384 0 0.0
.text 1261968 1261976 8 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2284096 2284104 8 0.0
.bss 172896 172896 0 0.0
.data 5480 5480 0 0.0
.heap 858072 858072 0 0.0
.text 1246696 1246704 8 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2257128 2257200 72 0.0
.bss 171712 171712 0 0.0
.data 5480 5480 0 0.0
.heap 859256 859256 0 0.0
.text 1219728 1219800 72 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1140008 1140008 0 0.0
.bss 11756 11756 0 0.0
.data 4376 4376 0 0.0
.heap 1020312 1020312 0 0.0
.text 103392 103392 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2046576 2046576 0 0.0
.bss 156704 156704 0 0.0
.data 4864 4864 0 0.0
.heap 874880 874880 0 0.0
.text 1009176 1009176 0 0.0
p6 all-clusters-app default (read/write) 2320800 2320840 40 0.0
.bss 110072 110072 0 0.0
.data 2464 2464 0 0.0
.heap 920808 920808 0 0.0
.text 1279064 1279104 40 0.0
light-app default (read/write) 2256600 2256624 24 0.0
.bss 98112 98112 0 0.0
.data 2328 2328 0 0.0
.heap 932904 932904 0 0.0
.text 1214864 1214888 24 0.0
lock-app default (read/write) 2231944 2231984 40 0.0
.bss 96768 96768 0 0.0
.data 2288 2288 0 0.0
.heap 934288 934288 0 0.0
.text 1190208 1190248 40 0.0
qpg lighting-app qpg6100+debug (read only) 499780 499800 20 0.0
(read/write) 114140 114140 0 0.0
.bss 79904 79904 0 0.0
.data 948 948 0 0.0
.text 494460 494480 20 0.0
lock-app qpg6100+debug (read only) 472592 472612 20 0.0
(read/write) 114140 114140 0 0.0
.bss 78816 78816 0 0.0
.data 900 900 0 0.0
.text 467272 467292 20 0.0
persistent-storage-app qpg6100+debug (read only) 108208 108208 0 0.0
(read/write) 114140 114140 0 0.0
.bss 36696 36696 0 0.0
.data 292 292 0 0.0
.text 102888 102888 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 782718 782734 16 0.0
bss 79976 79976 0 0.0
noinit 37160 37160 0 0.0
text 543954 543976 22 0.0

@kghost kghost added the hotfix urgent fix needed, can bypass review label Dec 2, 2021
Copy link
Contributor

@kghost kghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The non-async loopback transport is problematic. The message can be handled completely as soon as send message is called, we should deprecate it and prefer async loopback transport.

@kghost kghost merged commit dc10c82 into project-chip:master Dec 2, 2021
@bzbarsky-apple bzbarsky-apple deleted the fix-ci branch December 2, 2021 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app hotfix urgent fix needed, can bypass review review - pending
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants