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.

Sign up for GitHub

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

Jump to bottom

Don't touch member variables from async dispatch. #21456

Merged
merged 1 commit into from
Aug 1, 2022
Merged

Don't touch member variables from async dispatch. #21456

merged 1 commit into from
Aug 1, 2022

Conversation

bzbarsky-apple
Copy link
Contributor

We were null-checking members, then calling them from an async block,
by which point they might be null.

Fixes #21453

Problem

Crashes.

Change overview

See above.

Testing

Need to do stress-testing to see how this does on the crash stacks we are seeing...

@bzbarsky-apple
Don't touch member variables from async dispatch.
4317d6c 
We were null-checking members, then calling them from an async block,
by which point they might be null.

Fixes project-chip#21453
@bzbarsky-apple
Copy link
Contributor Author

@jtung-apple Please review next week?

@woody-apple woody-apple enabled auto-merge (squash) July 30, 2022 01:24
@github-actions
Copy link

github-actions bot commented Jul 30, 2022 •
edited
Loading

PR #21456: Size comparison from 390e391 to 4317d6c

Increases (4 builds for bl602, cc13x2_26x2, telink)
platform target config section 390e391 4317d6c change % change
bl602 lighting-app bl602 .text 1052456 1052460 4 0.0
cc13x2_26x2 lock-ftd LP_CC2652R7 (read only) 672095 672103 8 0.0
.text 595264 595272 8 0.0
pump-app LP_CC2652R7 (read/write) 161200 161208 8 0.0
telink light-switch-app tlsr9518adk80d text 569050 569052 2 0.0
Decreases (4 builds for cc13x2_26x2, esp32, nrfconnect)
platform target config section 390e391 4317d6c change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read/write) 169448 169440 -8 -0.0
pump-app LP_CC2652R7 (read only) 681167 681159 -8 -0.0
.text 591764 591756 -8 -0.0
esp32 all-clusters-app c3devkit (read only) 1023562 1023560 -2 -0.0
.flash.text 1023562 1023560 -2 -0.0
nrfconnect all-clusters-minimal-app nrf52840dk_nrf52840 text 798892 798888 -4 -0.0
Full report (43 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 390e391 4317d6c change % change
bl602 lighting-app bl602 (read/write) 1382346 1382346 0 0.0
.bss 117626 117626 0 0.0
.data 4480 4480 0 0.0
.text 1052456 1052460 4 0.0
bl602+rpc (read/write) 1427746 1427746 0 0.0
.bss 125066 125066 0 0.0
.data 4600 4600 0 0.0
.text 1084124 1084124 0 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 669131 669131 0 0.0
(read/write) 182220 182220 0 0.0
.bss 74244 74244 0 0.0
.data 3372 3372 0 0.0
.rodata 88283 88283 0 0.0
.text 580532 580532 0 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 634675 634675 0 0.0
(read/write) 157828 157828 0 0.0
.bss 73540 73540 0 0.0
.data 3372 3372 0 0.0
.rodata 77507 77507 0 0.0
.text 556844 556844 0 0.0
lock-ftd LP_CC2652R7 (read only) 672095 672103 8 0.0
(read/write) 169448 169440 -8 -0.0
.bss 71324 71324 0 0.0
.data 3296 3296 0 0.0
.rodata 76351 76351 0 0.0
.text 595264 595272 8 0.0
lock-mtd LP_CC2652R7 (read only) 654379 654379 0 0.0
(read/write) 182852 182852 0 0.0
.bss 67012 67012 0 0.0
.data 3296 3296 0 0.0
.rodata 101107 101107 0 0.0
.text 552792 552792 0 0.0
pump-app LP_CC2652R7 (read only) 681167 681159 -8 -0.0
(read/write) 161200 161208 8 0.0
.bss 71380 71380 0 0.0
.data 3296 3296 0 0.0
.rodata 88919 88919 0 0.0
.text 591764 591756 -8 -0.0
pump-controller-app LP_CC2652R7 (read only) 666743 666743 0 0.0
(read/write) 175760 175760 0 0.0
.bss 71516 71516 0 0.0
.data 3292 3292 0 0.0
.rodata 84743 84743 0 0.0
.text 581520 581520 0 0.0
shell LP_CC2652R7 (read only) 661822 661822 0 0.0
(read/write) 185048 185048 0 0.0
.bss 76564 76564 0 0.0
.data 3376 3376 0 0.0
.rodata 85262 85262 0 0.0
.text 576244 576244 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 583870 583870 0 0.0
.app_xip_area 460680 460680 0 0.0
.bss 65632 65632 0 0.0
.data 744 744 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 589782 589782 0 0.0
.app_xip_area 461864 461864 0 0.0
.bss 70360 70360 0 0.0
.data 748 748 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 589570 589570 0 0.0
.app_xip_area 467196 467196 0 0.0
.bss 64872 64872 0 0.0
.data 688 688 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read/write) 1088816 1088816 0 0.0
.bss 133268 133268 0 0.0
.data 2064 2064 0 0.0
.text 953464 953464 0 0.0
BRD4161A+rpc (read/write) 1143124 1143124 0 0.0
.bss 149948 149948 0 0.0
.data 2276 2276 0 0.0
.text 990880 990880 0 0.0
BRD4161A+rs911x (read/write) 974768 974768 0 0.0
.bss 161744 161744 0 0.0
.data 2048 2048 0 0.0
.text 810956 810956 0 0.0
lock-app BRD4161A+wf200 (read/write) 1129384 1129384 0 0.0
.bss 144376 144376 0 0.0
.data 2056 2056 0 0.0
.text 982932 982932 0 0.0
window-app BRD4161A (read/write) 1082100 1082100 0 0.0
.bss 134740 134740 0 0.0
.data 2092 2092 0 0.0
.text 945244 945244 0 0.0
esp32 all-clusters-app c3devkit (read only) 1023562 1023560 -2 -0.0
(read/write) 1486586 1486586 0 0.0
.dram0.bss 70304 70304 0 0.0
.dram0.data 14600 14600 0 0.0
.flash.rodata 216248 216248 0 0.0
.flash.text 1023562 1023560 -2 -0.0
.iram0.text 62902 62902 0 0.0
m5stack (read only) 1076939 1076939 0 0.0
(read/write) 488616 488616 0 0.0
.dram0.bss 75816 75816 0 0.0
.dram0.data 34144 34144 0 0.0
.flash.rodata 246660 246660 0 0.0
.flash.text 1071555 1071555 0 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w0+release (read/write) 643240 643240 0 0.0
.bss 69720 69720 0 0.0
.data 2044 2044 0 0.0
.text 568748 568748 0 0.0
lock k32w0+release (read/write) 700704 700704 0 0.0
.bss 70160 70160 0 0.0
.data 2052 2052 0 0.0
.text 625764 625764 0 0.0
linux all-clusters-app debug (read only) 3008177 3008177 0 0.0
(read/write) 155712 155712 0 0.0
.bss 61920 61920 0 0.0
.data 2064 2064 0 0.0
.data.rel.ro 85384 85384 0 0.0
.dynamic 608 608 0 0.0
.got 4568 4568 0 0.0
.init 27 27 0 0.0
.init_array 1144 1144 0 0.0
.rodata 270571 270571 0 0.0
.text 2558770 2558770 0 0.0
all-clusters-minimal-app debug (read only) 2850673 2850673 0 0.0
(read/write) 147448 147448 0 0.0
.bss 61120 61120 0 0.0
.data 2064 2064 0 0.0
.data.rel.ro 77992 77992 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 1136 1136 0 0.0
.rodata 270731 270731 0 0.0
.text 2403714 2403714 0 0.0
bridge-app debug+rpc (read only) 2358209 2358209 0 0.0
(read/write) 127576 127576 0 0.0
.bss 50592 50592 0 0.0
.data 3600 3600 0 0.0
.data.rel.ro 67544 67544 0 0.0
.dynamic 608 608 0 0.0
.got 4392 4392 0 0.0
.init 27 27 0 0.0
.init_array 816 816 0 0.0
.rodata 201960 201960 0 0.0
.text 1993266 1993266 0 0.0
chip-tool debug (read only) 10477841 10477841 0 0.0
(read/write) 641464 641464 0 0.0
.bss 24856 24856 0 0.0
.data 3266 3266 0 0.0
.data.rel.ro 606824 606824 0 0.0
.dynamic 608 608 0 0.0
.got 5088 5088 0 0.0
.init 27 27 0 0.0
.init_array 760 760 0 0.0
.rodata 536821 536821 0 0.0
.text 8456612 8456612 0 0.0
chip-tool-ipv6only arm64 (read only) 9888180 9888180 0 0.0
(read/write) 689073 689073 0 0.0
.bss 32897 32897 0 0.0
.data 3272 3272 0 0.0
.data.rel.ro 634360 634360 0 0.0
.dynamic 560 560 0 0.0
.got 13584 13584 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 468948 468948 0 0.0
.text 7804068 7804068 0 0.0
lighting-app debug+rpc (read only) 2580537 2580537 0 0.0
(read/write) 130256 130256 0 0.0
.bss 49760 49760 0 0.0
.data 2096 2096 0 0.0
.data.rel.ro 72456 72456 0 0.0
.dynamic 608 608 0 0.0
.got 4392 4392 0 0.0
.init 27 27 0 0.0
.init_array 904 904 0 0.0
.rodata 217616 217616 0 0.0
.text 2191874 2191874 0 0.0
lock-app debug (read only) 2566961 2566961 0 0.0
(read/write) 125528 125528 0 0.0
.bss 48224 48224 0 0.0
.data 1712 1712 0 0.0
.data.rel.ro 69592 69592 0 0.0
.dynamic 608 608 0 0.0
.got 4464 4464 0 0.0
.init 27 27 0 0.0
.init_array 880 880 0 0.0
.rodata 234800 234800 0 0.0
.text 2165170 2165170 0 0.0
ota-provider-app debug (read only) 2348873 2348873 0 0.0
(read/write) 119040 119040 0 0.0
.bss 47808 47808 0 0.0
.data 1936 1936 0 0.0
.data.rel.ro 63416 63416 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 760 760 0 0.0
.rodata 208088 208088 0 0.0
.text 1976930 1976930 0 0.0
ota-requestor-app debug (read only) 2469769 2469769 0 0.0
(read/write) 126432 126432 0 0.0
.bss 50176 50176 0 0.0
.data 2240 2240 0 0.0
.data.rel.ro 68072 68072 0 0.0
.dynamic 608 608 0 0.0
.got 4480 4480 0 0.0
.init 27 27 0 0.0
.init_array 824 824 0 0.0
.rodata 211648 211648 0 0.0
.text 2085650 2085650 0 0.0
shell debug (read only) 2580353 2580353 0 0.0
(read/write) 141832 141832 0 0.0
.bss 57768 57768 0 0.0
.data 1264 1264 0 0.0
.data.rel.ro 77008 77008 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 1016 1016 0 0.0
.rodata 231794 231794 0 0.0
.text 2190354 2190354 0 0.0
thermostat-no-ble arm64 (read only) 2347564 2347564 0 0.0
(read/write) 141761 141761 0 0.0
.bss 55329 55329 0 0.0
.data 1672 1672 0 0.0
.data.rel.ro 75976 75976 0 0.0
.dynamic 560 560 0 0.0
.got 5016 5016 0 0.0
.init 24 24 0 0.0
.init_array 408 408 0 0.0
.rodata 139316 139316 0 0.0
.text 1970736 1970736 0 0.0
tv-app debug (read only) 3151969 3151969 0 0.0
(read/write) 257736 257736 0 0.0
.bss 167352 167352 0 0.0
.data 4736 4736 0 0.0
.data.rel.ro 79104 79104 0 0.0
.dynamic 608 608 0 0.0
.got 4848 4848 0 0.0
.init 27 27 0 0.0
.init_array 1064 1064 0 0.0
.rodata 255656 255656 0 0.0
.text 2706738 2706738 0 0.0
tv-casting-app debug (read only) 5386985 5386985 0 0.0
(read/write) 158720 158720 0 0.0
.bss 51384 51384 0 0.0
.data 2432 2432 0 0.0
.data.rel.ro 98504 98504 0 0.0
.dynamic 608 608 0 0.0
.got 4736 4736 0 0.0
.init 27 27 0 0.0
.init_array 1024 1024 0 0.0
.rodata 338993 338993 0 0.0
.text 4780722 4780722 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2452184 2452184 0 0.0
.bss 214524 214524 0 0.0
.data 5872 5872 0 0.0
.text 1414828 1414828 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1173899 1173899 0 0.0
bss 143128 143128 0 0.0
rodata 142124 142124 0 0.0
text 809704 809704 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1153823 1153823 0 0.0
bss 142364 142364 0 0.0
rodata 133656 133656 0 0.0
text 798892 798888 -4 -0.0
p6 all-clusters-app default (read only) 881552 881552 0 0.0
(read/write) 1687940 1687940 0 0.0
.bss 149144 149144 0 0.0
.data 2648 2648 0 0.0
.text 1527760 1527760 0 0.0
all-clusters-minimal-app default (read only) 882272 882272 0 0.0
(read/write) 1632004 1632004 0 0.0
.bss 148424 148424 0 0.0
.data 2648 2648 0 0.0
.text 1472544 1472544 0 0.0
light-app default (read only) 890576 890576 0 0.0
(read/write) 1553196 1553196 0 0.0
.bss 140328 140328 0 0.0
.data 2440 2440 0 0.0
.text 1402040 1402040 0 0.0
lock-app default (read only) 886104 886104 0 0.0
(read/write) 1590876 1590876 0 0.0
.bss 144784 144784 0 0.0
.data 2456 2456 0 0.0
.text 1435248 1435248 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 805192 805192 0 0.0
bss 70972 70972 0 0.0
noinit 43488 43488 0 0.0
text 569050 569052 2 0.0
lighting-app tlsr9518adk80d (read/write) 825632 825632 0 0.0
bss 71816 71816 0 0.0
noinit 43488 43488 0 0.0
text 585938 585938 0 0.0

@woody-apple woody-apple merged commit 7a4f84f into project-chip:master Aug 1, 2022
@bzbarsky-apple bzbarsky-apple deleted the fix-async-dispatch-members branch August 1, 2022 22:37
github-actions bot pushed a commit that referenced this pull request Aug 1, 2022
@bzbarsky-apple @web-flow
Don't touch member variables from async dispatch. (#21456)
c39c4c2 
We were null-checking members, then calling them from an async block,
by which point they might be null.

Fixes #21453
woody-apple added a commit that referenced this pull request Aug 2, 2022
@woody-apple @bzbarsky-apple
Don't touch member variables from async dispatch. (#21456) (#21506)
7571349 
We were null-checking members, then calling them from an async block,
by which point they might be null.

Fixes #21453

Co-authored-by: Boris Zbarsky <[email protected]>
This was referenced Aug 2, 2022
Closed
Closed
isiu-apple pushed a commit to isiu-apple/connectedhomeip that referenced this pull request Sep 16, 2022
@bzbarsky-apple @isiu-apple
Don't touch member variables from async dispatch. (project-chip#21456)
3175251 
We were null-checking members, then calling them from an async block,
by which point they might be null.

Fixes project-chip#21453
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@woody-apple woody-apple woody-apple approved these changes

@jtung-apple jtung-apple Awaiting requested review from jtung-apple

@anush-apple anush-apple Awaiting requested review from anush-apple

@arkq arkq Awaiting requested review from arkq

@Byungjoo-Lee Byungjoo-Lee Awaiting requested review from Byungjoo-Lee

@carol-apple carol-apple Awaiting requested review from carol-apple

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@Damian-Nordic Damian-Nordic Awaiting requested review from Damian-Nordic

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@franck-apple franck-apple Awaiting requested review from franck-apple

@gjc13 gjc13 Awaiting requested review from gjc13

@harimau-qirex harimau-qirex Awaiting requested review from harimau-qirex

@harsha-rajendran harsha-rajendran Awaiting requested review from harsha-rajendran

@hawk248 hawk248 Awaiting requested review from hawk248

@isiu-apple isiu-apple Awaiting requested review from isiu-apple

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@kghost kghost Awaiting requested review from kghost

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@mlepage-google mlepage-google Awaiting requested review from mlepage-google

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@tecimovic tecimovic Awaiting requested review from tecimovic

@vijs vijs Awaiting requested review from vijs

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yufengwangca yufengwangca Awaiting requested review from yufengwangca

@yunhanw-google yunhanw-google Awaiting requested review from yunhanw-google

Assignees
No one assigned
Labels
darwin fast track review - pending
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SubscriptionCallback::ReportData needs to capture its callbacks in block vars.
3 participants
@bzbarsky-apple @mrjerryjohns @woody-apple