Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Avoid crash in reporting::Engine::ScheduleRun() #13093

Merged
merged 1 commit into from
Dec 16, 2021
Merged

Avoid crash in reporting::Engine::ScheduleRun() #13093

merged 1 commit into from
Dec 16, 2021

Conversation

kpschoedel
Copy link
Contributor

Problem

reporting::Engine::ScheduleRun() crashes if invoked after SessionManager has shut down.

Change overview

Check pointers.

Testing

Verified using pending PR #13060 with

server start
server stop
exit

which crashes without this change.

@kpschoedel
Avoid crash in reporting::Engine::ScheduleRun()
782b361 
#### Problem

`reporting::Engine::ScheduleRun()` crashes if invoked after
`SessionManager` has shut down.

#### Change overview

Check pointers.

#### Testing

Verified using pending PR project-chip#13060 with
```
server start
server stop
exit
```
which crashes without this change.
@boring-cyborg boring-cyborg bot added the app label Dec 16, 2021
@github-actions
Copy link

github-actions bot commented Dec 16, 2021 •
edited
Loading

PR #13093: Size comparison from 59dd330 to 782b361

Increases (5 builds for efr32, linux, p6)
platform target config section 59dd330 782b361 change % change
efr32 lighting-app BRD4161A+rpc (read only) 817084 817100 16 0.0
.text 817076 817092 16 0.0
window-app BRD4161A (read only) 802848 802864 16 0.0
.text 802840 802856 16 0.0
linux chip-tool-ipv6only arm64 (read only) 6930700 6930876 176 0.0
.text 5863652 5863828 176 0.0
thermostat-no-ble arm64 (read only) 1993780 1993956 176 0.0
.text 1654112 1654288 176 0.0
p6 all-clusters-app default (read/write) 2384624 2384640 16 0.0
.text 1342888 1342904 16 0.0
Decreases (2 builds for esp32, telink)
platform target config section 59dd330 782b361 change % change
esp32 all-clusters-app c3devkit (read only) 877072 877066 -6 -0.0
.flash.text 877072 877066 -6 -0.0
telink lighting-app tlsr9518adk80d (read/write) 830230 830222 -8 -0.0
text 578418 578416 -2 -0.0
Full report (31 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 59dd330 782b361 change % change
efr32 lighting-app BRD4161A (read only) 829456 829456 0 0.0
(read/write) 127352 127352 0 0.0
.bss 125472 125472 0 0.0
.data 1876 1876 0 0.0
.text 829448 829448 0 0.0
BRD4161A+rpc (read only) 817084 817100 16 0.0
(read/write) 144016 144016 0 0.0
.bss 142040 142040 0 0.0
.data 1976 1976 0 0.0
.text 817076 817092 16 0.0
window-app BRD4161A (read only) 802848 802864 16 0.0
(read/write) 126288 126288 0 0.0
.bss 124456 124456 0 0.0
.data 1832 1832 0 0.0
.text 802840 802856 16 0.0
esp32 all-clusters-app c3devkit (read only) 877072 877066 -6 -0.0
(read/write) 1313042 1313042 0 0.0
.dram0.bss 69784 69784 0 0.0
.dram0.data 14220 14220 0 0.0
.flash.rodata 175976 175976 0 0.0
.flash.text 877072 877066 -6 -0.0
.iram0.text 62254 62254 0 0.0
m5stack (read only) 937951 937951 0 0.0
(read/write) 442144 442144 0 0.0
.dram0.bss 74280 74280 0 0.0
.dram0.data 34056 34056 0 0.0
.flash.rodata 202800 202800 0 0.0
.flash.text 932567 932567 0 0.0
.iram0.text 122671 122671 0 0.0
k32w light k32w061+release (read/write) 648312 648312 0 0.0
.bss 76480 76480 0 0.0
.data 1904 1904 0 0.0
.text 564128 564128 0 0.0
lock k32w061+release (read/write) 633028 633028 0 0.0
.bss 76200 76200 0 0.0
.data 1860 1860 0 0.0
.text 549168 549168 0 0.0
linux chip-tool-ipv6only arm64 (read only) 6930700 6930876 176 0.0
(read/write) 323633 323633 0 0.0
.bss 54577 54577 0 0.0
.data 1096 1096 0 0.0
.data.rel.ro 208096 208096 0 0.0
.dynamic 560 560 0 0.0
.got 56160 56160 0 0.0
.init 24 24 0 0.0
.init_array 168 168 0 0.0
.rodata 379476 379476 0 0.0
.text 5863652 5863828 176 0.0
thermostat-no-ble arm64 (read only) 1993780 1993956 176 0.0
(read/write) 143937 143937 0 0.0
.bss 64321 64321 0 0.0
.data 880 880 0 0.0
.data.rel.ro 72000 72000 0 0.0
.dynamic 560 560 0 0.0
.got 3840 3840 0 0.0
.init 24 24 0 0.0
.init_array 288 288 0 0.0
.rodata 128196 128196 0 0.0
.text 1654112 1654288 176 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2334104 2334104 0 0.0
.bss 189068 189068 0 0.0
.data 5264 5264 0 0.0
.text 1296680 1296680 0 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2328616 2328616 0 0.0
.bss 180896 180896 0 0.0
.data 5544 5544 0 0.0
.text 1291216 1291216 0 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2301712 2301712 0 0.0
.bss 179944 179944 0 0.0
.data 5536 5536 0 0.0
.text 1264312 1264312 0 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 4368 4368 0 0.0
.text 103392 103392 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2053688 2053688 0 0.0
.bss 156972 156972 0 0.0
.data 4864 4864 0 0.0
.text 1016288 1016288 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 935579 935579 0 0.0
bss 118400 118400 0 0.0
rodata 108120 108120 0 0.0
text 631508 631508 0 0.0
nrf52840dk_nrf52840+rpc (read/write) 921979 921979 0 0.0
bss 115444 115444 0 0.0
rodata 101536 101536 0 0.0
text 626820 626820 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 859342 859342 0 0.0
bss 116684 116684 0 0.0
rodata 103044 103044 0 0.0
text 558948 558948 0 0.0
lock-app nrf52840dk_nrf52840 (read/write) 907723 907723 0 0.0
bss 117588 117588 0 0.0
rodata 103424 103424 0 0.0
text 609332 609332 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 831638 831638 0 0.0
bss 115900 115900 0 0.0
rodata 98388 98388 0 0.0
text 536816 536816 0 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 542351 542351 0 0.0
bss 52588 52588 0 0.0
rodata 50668 50668 0 0.0
text 376892 376892 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 912603 912603 0 0.0
bss 117496 117496 0 0.0
rodata 104768 104768 0 0.0
text 612884 612884 0 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 905803 905803 0 0.0
bss 117376 117376 0 0.0
rodata 102896 102896 0 0.0
text 608080 608080 0 0.0
shell nrf52840dk_nrf52840 (read/write) 796079 796079 0 0.0
bss 109464 109464 0 0.0
rodata 78096 78096 0 0.0
text 532048 532048 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 708710 708710 0 0.0
bss 107352 107352 0 0.0
rodata 72396 72396 0 0.0
text 449544 449544 0 0.0
p6 all-clusters-app default (read/write) 2384624 2384640 16 0.0
.bss 117260 117260 0 0.0
.data 2544 2544 0 0.0
.text 1342888 1342904 16 0.0
light-app default (read/write) 2324008 2324008 0 0.0
.bss 106152 106152 0 0.0
.data 2384 2384 0 0.0
.text 1282272 1282272 0 0.0
lock-app default (read/write) 2296216 2296216 0 0.0
.bss 105032 105032 0 0.0
.data 2336 2336 0 0.0
.text 1254480 1254480 0 0.0
qpg lighting-app qpg6105+debug (read only) 531812 531812 0 0.0
(read/write) 146936 146936 0 0.0
.bss 86816 86816 0 0.0
.data 1004 1004 0 0.0
.text 526492 526492 0 0.0
lock-app qpg6105+debug (read only) 503492 503492 0 0.0
(read/write) 146940 146940 0 0.0
.bss 85952 85952 0 0.0
.data 952 952 0 0.0
.text 498172 498172 0 0.0
persistent-storage-app qpg6105+debug (read only) 106448 106448 0 0.0
(read/write) 146938 146938 0 0.0
.bss 36146 36146 0 0.0
.data 288 288 0 0.0
.text 101128 101128 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 830230 830222 -8 -0.0
bss 87040 87040 0 0.0
noinit 37160 37160 0 0.0
text 578418 578416 -2 -0.0

bzbarsky-apple
bzbarsky-apple approved these changes Dec 16, 2021
View reviewed changes
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

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

I guess, but what's calling ScheduleRun so late?

@kpschoedel
Copy link
Contributor Author

I guess, but what's calling ScheduleRun so late?

It's via :app::EventManagement::LogEvent(), called in the identified from PlatformMgrDelegate::OnShutDown(). Yufeng will address that particular call separately, but the underlying problem is that there's no emberAfShutdown() to complement emberAfInit(), and the even-more-underlying problem is that CHIP has no coherent init/shutdown strategy.

@andy31415 andy31415 merged commit 2ddefed into project-chip:master Dec 16, 2021
@kpschoedel kpschoedel deleted the server-shutdown-1 branch December 20, 2021 14:18
@lnikulin lnikulin mentioned this pull request Feb 1, 2022
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@bzbarsky-apple bzbarsky-apple bzbarsky-apple approved these changes

@yufengwangca yufengwangca yufengwangca approved these changes

@msandstedt msandstedt msandstedt approved these changes

@andy31415 andy31415 Awaiting requested review from andy31415

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

@austinh0 austinh0 Awaiting requested review from austinh0

@balducci-apple balducci-apple Awaiting requested review from balducci-apple

@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

@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

@hawk248 hawk248 Awaiting requested review from hawk248

@holbrookt holbrookt Awaiting requested review from holbrookt

@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

@LuDuda LuDuda Awaiting requested review from LuDuda

@lzgrablic02 lzgrablic02 Awaiting requested review from lzgrablic02

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@pan-apple pan-apple Awaiting requested review from pan-apple

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@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

@turon turon Awaiting requested review from turon

@vijs vijs Awaiting requested review from vijs

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

@wbschiller wbschiller Awaiting requested review from wbschiller

@woody-apple woody-apple Awaiting requested review from woody-apple

@xylophone21 xylophone21 Awaiting requested review from xylophone21

Assignees
No one assigned
Labels
app review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kpschoedel @msandstedt @yufengwangca @bzbarsky-apple @andy31415