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

Clarify the logic in ExchangeContext::OnSessionReleased. #19383

Merged

Conversation

bzbarsky-apple
Copy link
Contributor

It was not clear who was responsible for releasing various refcounts.
Improve the code and documentation to make it clearer what's going on,
and to avoid the two calls into DoClose in the "waiting for response"
case.

Addresses a problem that was mentioned in
#19359

Problem

See above.

Change overview

See above.

Testing

No actual behavior changes in practice, just more clarity about what's going on.

@github-actions
Copy link

github-actions bot commented Jun 9, 2022

PR #19383: Size comparison from 47a51ce to a7d5115

Increases (27 builds for cc13x2_26x2, cyw30739, efr32, k32w, linux, nrfconnect, p6, telink)
platform target config section 47a51ce a7d5115 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 689951 689975 24 0.0
.text 577312 577336 24 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 640567 640591 24 0.0
.text 550880 550904 24 0.0
lock-ftd LP_CC2652R7 (read only) 683455 683479 24 0.0
.text 584788 584812 24 0.0
lock-mtd LP_CC2652R7 (read only) 632871 632895 24 0.0
.text 534308 534332 24 0.0
pump-app LP_CC2652R7 (read only) 664899 664923 24 0.0
.text 578052 578076 24 0.0
pump-controller-app LP_CC2652R7 (read only) 655483 655499 16 0.0
.text 570748 570764 16 0.0
shell LP_CC2652R7 (read only) 682230 682246 16 0.0
.text 572992 573008 16 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 602562 602578 16 0.0
.app_xip_area 461508 461524 16 0.0
lock cyw930739m2evb_01 (read/write) 599646 599662 16 0.0
.app_xip_area 458456 458472 16 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 599438 599462 24 0.0
.app_xip_area 459364 459388 24 0.0
efr32 lighting-app BRD4161A (read only) 915032 915048 16 0.0
.text 915024 915040 16 0.0
BRD4161A+rpc (read only) 949236 949252 16 0.0
.text 949228 949244 16 0.0
BRD4161A+rs911x (read only) 790228 790244 16 0.0
.text 790220 790236 16 0.0
lock-app BRD4161A+wf200 (read only) 958468 958500 32 0.0
.text 958460 958492 32 0.0
window-app BRD4161A (read only) 900088 900120 32 0.0
.text 900080 900112 32 0.0
k32w light k32w061+release (read/write) 653768 653784 16 0.0
.text 575920 575936 16 0.0
lock k32w061+release (read/write) 714876 714892 16 0.0
.text 636616 636632 16 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 9499468 9499548 80 0.0
.text 7483220 7483300 80 0.0
thermostat-no-ble arm64 (read only) 2544060 2544124 64 0.0
.text 2147040 2147104 64 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1192051 1192067 16 0.0
text 817112 817132 20 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1139759 1139775 16 0.0
text 789160 789184 24 0.0
p6 all-clusters-app default (read/write) 2544520 2544552 32 0.0
.text 1502784 1502816 32 0.0
all-clusters-minimal-app default (read/write) 2489392 2489424 32 0.0
.text 1447656 1447688 32 0.0
light-app default (read/write) 2421312 2421328 16 0.0
.text 1379576 1379592 16 0.0
lock-app default (read/write) 2441648 2441664 16 0.0
.text 1399912 1399928 16 0.0
telink light-switch-app tlsr9518adk80d (read/write) 781696 781736 40 0.0
text 552570 552610 40 0.0
lighting-app tlsr9518adk80d (read/write) 801708 801740 32 0.0
text 569292 569328 36 0.0
Decreases (5 builds for cc13x2_26x2)
platform target config section 47a51ce a7d5115 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read/write) 161816 161792 -24 -0.0
lock-ftd LP_CC2652R7 (read/write) 159376 159352 -24 -0.0
pump-app LP_CC2652R7 (read/write) 178844 178820 -24 -0.0
pump-controller-app LP_CC2652R7 (read/write) 188364 188348 -16 -0.0
shell LP_CC2652R7 (read/write) 165032 165016 -16 -0.0
Full report (28 builds for cc13x2_26x2, cyw30739, efr32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 47a51ce a7d5115 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 689951 689975 24 0.0
(read/write) 161816 161792 -24 -0.0
.bss 74660 74660 0 0.0
.data 3392 3392 0 0.0
.rodata 112327 112327 0 0.0
.text 577312 577336 24 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 640567 640591 24 0.0
(read/write) 158132 158132 0 0.0
.bss 73884 73884 0 0.0
.data 3332 3332 0 0.0
.rodata 89367 89367 0 0.0
.text 550880 550904 24 0.0
lock-ftd LP_CC2652R7 (read only) 683455 683479 24 0.0
(read/write) 159376 159352 -24 -0.0
.bss 72612 72612 0 0.0
.data 3256 3256 0 0.0
.rodata 98183 98183 0 0.0
.text 584788 584812 24 0.0
lock-mtd LP_CC2652R7 (read only) 632871 632895 24 0.0
(read/write) 145720 145720 0 0.0
.bss 68348 68348 0 0.0
.data 3256 3256 0 0.0
.rodata 98071 98071 0 0.0
.text 534308 534332 24 0.0
pump-app LP_CC2652R7 (read only) 664899 664923 24 0.0
(read/write) 178844 178820 -24 -0.0
.bss 72756 72756 0 0.0
.data 3292 3292 0 0.0
.rodata 86363 86363 0 0.0
.text 578052 578076 24 0.0
pump-controller-app LP_CC2652R7 (read only) 655483 655499 16 0.0
(read/write) 188364 188348 -16 -0.0
.bss 72860 72860 0 0.0
.data 3252 3252 0 0.0
.rodata 84251 84251 0 0.0
.text 570748 570764 16 0.0
shell LP_CC2652R7 (read only) 682230 682246 16 0.0
(read/write) 165032 165016 -16 -0.0
.bss 76956 76956 0 0.0
.data 3396 3396 0 0.0
.rodata 108926 108926 0 0.0
.text 572992 573008 16 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 602562 602578 16 0.0
.app_xip_area 461508 461524 16 0.0
.bss 84008 84008 0 0.0
.data 732 732 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 599646 599662 16 0.0
.app_xip_area 458456 458472 16 0.0
.bss 84176 84176 0 0.0
.data 700 700 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 599438 599462 24 0.0
.app_xip_area 459364 459388 24 0.0
.bss 83140 83140 0 0.0
.data 616 616 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 915032 915048 16 0.0
(read/write) 133176 133176 0 0.0
.bss 131088 131088 0 0.0
.data 2088 2088 0 0.0
.text 915024 915040 16 0.0
BRD4161A+rpc (read only) 949236 949252 16 0.0
(read/write) 149868 149868 0 0.0
.bss 147576 147576 0 0.0
.data 2292 2292 0 0.0
.text 949228 949244 16 0.0
BRD4161A+rs911x (read only) 790228 790244 16 0.0
(read/write) 129460 129460 0 0.0
.bss 127364 127364 0 0.0
.data 2096 2096 0 0.0
.text 790220 790236 16 0.0
lock-app BRD4161A+wf200 (read only) 958468 958500 32 0.0
(read/write) 128252 128252 0 0.0
.bss 126188 126188 0 0.0
.data 2064 2064 0 0.0
.text 958460 958492 32 0.0
window-app BRD4161A (read only) 900088 900120 32 0.0
(read/write) 133264 133264 0 0.0
.bss 131176 131176 0 0.0
.data 2084 2084 0 0.0
.text 900080 900112 32 0.0
k32w light k32w061+release (read/write) 653768 653784 16 0.0
.bss 70044 70044 0 0.0
.data 2004 2004 0 0.0
.text 575920 575936 16 0.0
lock k32w061+release (read/write) 714876 714892 16 0.0
.bss 70484 70484 0 0.0
.data 1976 1976 0 0.0
.text 636616 636632 16 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 9499468 9499548 80 0.0
(read/write) 678033 678033 0 0.0
.bss 43681 43681 0 0.0
.data 1152 1152 0 0.0
.data.rel.ro 614464 614464 0 0.0
.dynamic 528 528 0 0.0
.got 14936 14936 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 467916 467916 0 0.0
.text 7483220 7483300 80 0.0
thermostat-no-ble arm64 (read only) 2544060 2544124 64 0.0
(read/write) 183057 183057 0 0.0
.bss 91409 91409 0 0.0
.data 1512 1512 0 0.0
.data.rel.ro 82120 82120 0 0.0
.dynamic 528 528 0 0.0
.got 4992 4992 0 0.0
.init 24 24 0 0.0
.init_array 400 400 0 0.0
.rodata 160164 160164 0 0.0
.text 2147040 2147104 64 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2429656 2429656 0 0.0
.bss 202692 202692 0 0.0
.data 5872 5872 0 0.0
.text 1392300 1392300 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1192051 1192067 16 0.0
bss 141362 141362 0 0.0
rodata 154644 154644 0 0.0
text 817112 817132 20 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1139759 1139775 16 0.0
bss 140579 140579 0 0.0
rodata 131136 131136 0 0.0
text 789160 789184 24 0.0
p6 all-clusters-app default (read/write) 2544520 2544552 32 0.0
.bss 137120 137120 0 0.0
.data 2808 2808 0 0.0
.text 1502784 1502816 32 0.0
all-clusters-minimal-app default (read/write) 2489392 2489424 32 0.0
.bss 136328 136328 0 0.0
.data 2752 2752 0 0.0
.text 1447656 1447688 32 0.0
light-app default (read/write) 2421312 2421328 16 0.0
.bss 129432 129432 0 0.0
.data 2600 2600 0 0.0
.text 1379576 1379592 16 0.0
lock-app default (read/write) 2441648 2441664 16 0.0
.bss 129256 129256 0 0.0
.data 2576 2576 0 0.0
.text 1399912 1399928 16 0.0
telink light-switch-app tlsr9518adk80d (read/write) 781696 781736 40 0.0
bss 70636 70636 0 0.0
noinit 40416 40416 0 0.0
text 552570 552610 40 0.0
lighting-app tlsr9518adk80d (read/write) 801708 801740 32 0.0
bss 70888 70888 0 0.0
noinit 40416 40416 0 0.0
text 569292 569328 36 0.0

It was not clear who was responsible for releasing various refcounts.
Improve the code and documentation to make it clearer what's going on,
and to avoid the two calls into DoClose in the "waiting for response"
case.

Addresses a problem that was mentioned in
project-chip#19359
@github-actions
Copy link

github-actions bot commented Jun 9, 2022

PR #19383: Size comparison from ac7e2f7 to 4f268f3

Increases (24 builds for cc13x2_26x2, cyw30739, esp32, k32w, linux, nrfconnect, p6, telink)
platform target config section ac7e2f7 4f268f3 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 689943 689967 24 0.0
.text 577304 577328 24 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 640599 640623 24 0.0
.text 550912 550936 24 0.0
lock-ftd LP_CC2652R7 (read only) 683455 683487 32 0.0
.text 584788 584820 32 0.0
lock-mtd LP_CC2652R7 (read only) 632879 632903 24 0.0
.text 534316 534340 24 0.0
pump-app LP_CC2652R7 (read only) 664899 664931 32 0.0
.text 578052 578084 32 0.0
pump-controller-app LP_CC2652R7 (read only) 655483 655515 32 0.0
.text 570748 570780 32 0.0
shell LP_CC2652R7 (read only) 682230 682254 24 0.0
.text 572992 573016 24 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 602562 602578 16 0.0
.app_xip_area 461508 461524 16 0.0
lock cyw930739m2evb_01 (read/write) 599646 599662 16 0.0
.app_xip_area 458456 458472 16 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 599446 599470 24 0.0
.app_xip_area 459372 459396 24 0.0
esp32 all-clusters-app c3devkit (read only) 1007740 1007776 36 0.0
.flash.text 1007740 1007776 36 0.0
m5stack (read only) 1062491 1062511 20 0.0
.flash.text 1057107 1057127 20 0.0
k32w light k32w061+release (read/write) 653784 653800 16 0.0
.text 575936 575952 16 0.0
lock k32w061+release (read/write) 714876 714892 16 0.0
.text 636616 636632 16 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 9502940 9503020 80 0.0
.text 7486692 7486772 80 0.0
thermostat-no-ble arm64 (read only) 2544060 2544140 80 0.0
.text 2147040 2147120 80 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1192051 1192067 16 0.0
text 817112 817132 20 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1139775 1139807 32 0.0
text 789188 789212 24 0.0
p6 all-clusters-app default (read/write) 2544632 2544648 16 0.0
.text 1502896 1502912 16 0.0
all-clusters-minimal-app default (read/write) 2489440 2489456 16 0.0
.text 1447704 1447720 16 0.0
light-app default (read/write) 2421312 2421328 16 0.0
.text 1379576 1379592 16 0.0
lock-app default (read/write) 2441648 2441664 16 0.0
.text 1399912 1399928 16 0.0
telink light-switch-app tlsr9518adk80d (read/write) 781696 781736 40 0.0
text 552576 552612 36 0.0
lighting-app tlsr9518adk80d (read/write) 801716 801748 32 0.0
text 569298 569332 34 0.0
Decreases (5 builds for cc13x2_26x2)
platform target config section ac7e2f7 4f268f3 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read/write) 161824 161800 -24 -0.0
lock-ftd LP_CC2652R7 (read/write) 159376 159344 -32 -0.0
pump-app LP_CC2652R7 (read/write) 178844 178812 -32 -0.0
pump-controller-app LP_CC2652R7 (read/write) 188364 188332 -32 -0.0
shell LP_CC2652R7 (read/write) 165032 165008 -24 -0.0
Full report (25 builds for cc13x2_26x2, cyw30739, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section ac7e2f7 4f268f3 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 689943 689967 24 0.0
(read/write) 161824 161800 -24 -0.0
.bss 74660 74660 0 0.0
.data 3392 3392 0 0.0
.rodata 112327 112327 0 0.0
.text 577304 577328 24 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 640599 640623 24 0.0
(read/write) 158132 158132 0 0.0
.bss 73884 73884 0 0.0
.data 3332 3332 0 0.0
.rodata 89367 89367 0 0.0
.text 550912 550936 24 0.0
lock-ftd LP_CC2652R7 (read only) 683455 683487 32 0.0
(read/write) 159376 159344 -32 -0.0
.bss 72612 72612 0 0.0
.data 3256 3256 0 0.0
.rodata 98183 98183 0 0.0
.text 584788 584820 32 0.0
lock-mtd LP_CC2652R7 (read only) 632879 632903 24 0.0
(read/write) 145720 145720 0 0.0
.bss 68348 68348 0 0.0
.data 3256 3256 0 0.0
.rodata 98071 98071 0 0.0
.text 534316 534340 24 0.0
pump-app LP_CC2652R7 (read only) 664899 664931 32 0.0
(read/write) 178844 178812 -32 -0.0
.bss 72756 72756 0 0.0
.data 3292 3292 0 0.0
.rodata 86363 86363 0 0.0
.text 578052 578084 32 0.0
pump-controller-app LP_CC2652R7 (read only) 655483 655515 32 0.0
(read/write) 188364 188332 -32 -0.0
.bss 72860 72860 0 0.0
.data 3252 3252 0 0.0
.rodata 84251 84251 0 0.0
.text 570748 570780 32 0.0
shell LP_CC2652R7 (read only) 682230 682254 24 0.0
(read/write) 165032 165008 -24 -0.0
.bss 76956 76956 0 0.0
.data 3396 3396 0 0.0
.rodata 108926 108926 0 0.0
.text 572992 573016 24 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 602562 602578 16 0.0
.app_xip_area 461508 461524 16 0.0
.bss 84008 84008 0 0.0
.data 732 732 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 599646 599662 16 0.0
.app_xip_area 458456 458472 16 0.0
.bss 84176 84176 0 0.0
.data 700 700 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 599446 599470 24 0.0
.app_xip_area 459372 459396 24 0.0
.bss 83140 83140 0 0.0
.data 616 616 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
esp32 all-clusters-app c3devkit (read only) 1007740 1007776 36 0.0
(read/write) 1481834 1481834 0 0.0
.dram0.bss 69168 69168 0 0.0
.dram0.data 14656 14656 0 0.0
.flash.rodata 212584 212584 0 0.0
.flash.text 1007740 1007776 36 0.0
.iram0.text 62902 62902 0 0.0
m5stack (read only) 1062491 1062511 20 0.0
(read/write) 483968 483968 0 0.0
.dram0.bss 74688 74688 0 0.0
.dram0.data 34200 34200 0 0.0
.flash.rodata 243084 243084 0 0.0
.flash.text 1057107 1057127 20 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w061+release (read/write) 653784 653800 16 0.0
.bss 70044 70044 0 0.0
.data 2004 2004 0 0.0
.text 575936 575952 16 0.0
lock k32w061+release (read/write) 714876 714892 16 0.0
.bss 70484 70484 0 0.0
.data 1976 1976 0 0.0
.text 636616 636632 16 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 9502940 9503020 80 0.0
(read/write) 678033 678033 0 0.0
.bss 43681 43681 0 0.0
.data 1152 1152 0 0.0
.data.rel.ro 614464 614464 0 0.0
.dynamic 528 528 0 0.0
.got 14936 14936 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 467916 467916 0 0.0
.text 7486692 7486772 80 0.0
thermostat-no-ble arm64 (read only) 2544060 2544140 80 0.0
(read/write) 183057 183057 0 0.0
.bss 91409 91409 0 0.0
.data 1512 1512 0 0.0
.data.rel.ro 82120 82120 0 0.0
.dynamic 528 528 0 0.0
.got 4992 4992 0 0.0
.init 24 24 0 0.0
.init_array 400 400 0 0.0
.rodata 160164 160164 0 0.0
.text 2147040 2147120 80 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2429656 2429656 0 0.0
.bss 202692 202692 0 0.0
.data 5872 5872 0 0.0
.text 1392300 1392300 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1192051 1192067 16 0.0
bss 141362 141362 0 0.0
rodata 154644 154644 0 0.0
text 817112 817132 20 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1139775 1139807 32 0.0
bss 140579 140579 0 0.0
rodata 131136 131136 0 0.0
text 789188 789212 24 0.0
p6 all-clusters-app default (read/write) 2544632 2544648 16 0.0
.bss 137120 137120 0 0.0
.data 2808 2808 0 0.0
.text 1502896 1502912 16 0.0
all-clusters-minimal-app default (read/write) 2489440 2489456 16 0.0
.bss 136328 136328 0 0.0
.data 2752 2752 0 0.0
.text 1447704 1447720 16 0.0
light-app default (read/write) 2421312 2421328 16 0.0
.bss 129432 129432 0 0.0
.data 2600 2600 0 0.0
.text 1379576 1379592 16 0.0
lock-app default (read/write) 2441648 2441664 16 0.0
.bss 129256 129256 0 0.0
.data 2576 2576 0 0.0
.text 1399912 1399928 16 0.0
telink light-switch-app tlsr9518adk80d (read/write) 781696 781736 40 0.0
bss 70636 70636 0 0.0
noinit 40416 40416 0 0.0
text 552576 552612 36 0.0
lighting-app tlsr9518adk80d (read/write) 801716 801748 32 0.0
bss 70888 70888 0 0.0
noinit 40416 40416 0 0.0
text 569298 569332 34 0.0

src/messaging/ExchangeDelegate.h Show resolved Hide resolved
src/messaging/ExchangeDelegate.h Outdated Show resolved Hide resolved
@bzbarsky-apple bzbarsky-apple merged commit e5bad6a into project-chip:master Jun 10, 2022
@bzbarsky-apple bzbarsky-apple deleted the fix-exchange-session-release branch June 10, 2022 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants