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

Incorrect processing in Engine::ScheduleRun() #14697

Closed
wants to merge 1 commit into from
Closed

Incorrect processing in Engine::ScheduleRun() #14697

wants to merge 1 commit into from

Conversation

lnikulin
Copy link
Contributor

@lnikulin lnikulin commented Feb 2, 2022

Problem

Change overview

setup 'mRunScheduled = true' before execute ScheduleWork() to avoid case when scheduled callback has been done before ScheduleWork() returns

@lnikulin
Fix Engine::ScheduleRun() in case callback function done before syste…
00bf71c 
…mLayer->ScheduleWork() returns. (project-chip#14596)

- setup mRunScheduled = true before execute ScheduleWork()
@boring-cyborg boring-cyborg bot added the app label Feb 2, 2022
@CLAassistant
Copy link

CLAassistant commented Feb 2, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@restyled-io restyled-io bot mentioned this pull request Feb 2, 2022
Closed
@github-actions
Copy link

github-actions bot commented Feb 2, 2022 •
edited
Loading

PR #14697: Size comparison from 63a5c52 to 00bf71c

Increases (16 builds for cyw30739, efr32, esp32, nrfconnect, p6, telink)
platform target config section 63a5c52 00bf71c change % change
cyw30739 lock cyw930739m2evb_01 (read/write) 535150 535158 8 0.0
.app_xip_area 443948 443956 8 0.0
ota-requestor cyw930739m2evb_01 (read/write) 559706 559714 8 0.0
.app_xip_area 459192 459200 8 0.0
efr32 lighting-app BRD4161A (read only) 843824 843840 16 0.0
.text 843816 843832 16 0.0
BRD4161A+rpc (read only) 831200 831216 16 0.0
.text 831192 831208 16 0.0
esp32 all-clusters-app c3devkit (read only) 934022 934026 4 0.0
.flash.text 934022 934026 4 0.0
m5stack (read only) 982071 982079 8 0.0
.flash.text 976687 976695 8 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 text 668068 668072 4 0.0
nrf52840dk_nrf52840+rpc text 664508 664512 4 0.0
nrf52840dongle_nrf52840 text 673460 673464 4 0.0
nrf5340dk_nrf5340_cpuapp text 584256 584260 4 0.0
lock-app nrf52840dk_nrf52840 (read/write) 913143 913159 16 0.0
text 614068 614072 4 0.0
nrf5340dk_nrf5340_cpuapp text 531040 531044 4 0.0
pump-app nrf52840dk_nrf52840 (read/write) 915927 915943 16 0.0
text 616524 616528 4 0.0
pump-controller-app nrf52840dk_nrf52840 text 612576 612580 4 0.0
p6 light-app default (read/write) 2340536 2340552 16 0.0
.text 1298800 1298816 16 0.0
telink lighting-app tlsr9518adk80d text 592264 592268 4 0.0
Decreases (10 builds for linux)
platform target config section 63a5c52 00bf71c change % change
linux all-clusters-app debug (read only) 2303353 2303321 -32 -0.0
.text 1936994 1936962 -32 -0.0
bridge-app debug+rpc (read only) 1630733 1630717 -16 -0.0
.text 1377701 1377685 -16 -0.0
chip-tool debug (read only) 7135381 7135349 -32 -0.0
.text 6360597 6360565 -32 -0.0
door-lock-app debug (read only) 1852145 1852129 -16 -0.0
.text 1532818 1532802 -16 -0.0
lighting-app debug+rpc (read only) 1953513 1953481 -32 -0.0
.text 1637794 1637762 -32 -0.0
ota-provider-app debug (read only) 1631425 1631409 -16 -0.0
.text 1366226 1366210 -16 -0.0
ota-requestor-app debug (read only) 1640193 1640161 -32 -0.0
.text 1379746 1379714 -32 -0.0
shell debug (read only) 2277977 2277945 -32 -0.0
.text 1916770 1916738 -32 -0.0
thermostat-no-ble arm64 (read only) 2086380 2086364 -16 -0.0
.text 1737648 1737632 -16 -0.0
tv-app debug (read only) 2407873 2407841 -32 -0.0
.text 2044370 2044338 -32 -0.0
Full report (43 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 63a5c52 00bf71c change % change
cyw30739 light cyw930739m2evb_01 (read/write) 577526 577526 0 0.0
.app_xip_area 484756 484756 0 0.0
.bss 75516 75516 0 0.0
.data 596 596 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 535150 535158 8 0.0
.app_xip_area 443948 443956 8 0.0
.bss 73988 73988 0 0.0
.data 560 560 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor cyw930739m2evb_01 (read/write) 559706 559714 8 0.0
.app_xip_area 459192 459200 8 0.0
.bss 82972 82972 0 0.0
.data 504 504 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 843824 843840 16 0.0
(read/write) 125056 125056 0 0.0
.bss 123160 123160 0 0.0
.data 1892 1892 0 0.0
.text 843816 843832 16 0.0
BRD4161A+rpc (read only) 831200 831216 16 0.0
(read/write) 141712 141712 0 0.0
.bss 139720 139720 0 0.0
.data 1992 1992 0 0.0
.text 831192 831208 16 0.0
window-app BRD4161A (read only) 816076 816076 0 0.0
(read/write) 123684 123684 0 0.0
.bss 121836 121836 0 0.0
.data 1848 1848 0 0.0
.text 816068 816068 0 0.0
esp32 all-clusters-app c3devkit (read only) 934022 934026 4 0.0
(read/write) 1401482 1401482 0 0.0
.dram0.bss 70320 70320 0 0.0
.dram0.data 14268 14268 0 0.0
.flash.rodata 198000 198000 0 0.0
.flash.text 934022 934026 4 0.0
.iram0.text 62056 62056 0 0.0
m5stack (read only) 982071 982079 8 0.0
(read/write) 465832 465832 0 0.0
.dram0.bss 75072 75072 0 0.0
.dram0.data 34024 34024 0 0.0
.flash.rodata 224608 224608 0 0.0
.flash.text 976687 976695 8 0.0
.iram0.text 123399 123399 0 0.0
k32w light k32w061+release (read/write) 661708 661708 0 0.0
.bss 75212 75212 0 0.0
.data 1852 1852 0 0.0
.text 578844 578844 0 0.0
lock k32w061+release (read/write) 662892 662892 0 0.0
.bss 75532 75532 0 0.0
.data 1892 1892 0 0.0
.text 579668 579668 0 0.0
linux all-clusters-app debug (read only) 2303353 2303321 -32 -0.0
(read/write) 148320 148320 0 0.0
.bss 63040 63040 0 0.0
.data 1200 1200 0 0.0
.data.rel.ro 78392 78392 0 0.0
.dynamic 592 592 0 0.0
.got 4128 4128 0 0.0
.init 27 27 0 0.0
.init_array 920 920 0 0.0
.rodata 204357 204357 0 0.0
.text 1936994 1936962 -32 -0.0
bridge-app debug+rpc (read only) 1630733 1630717 -16 -0.0
(read/write) 88296 88296 0 0.0
.bss 43776 43776 0 0.0
.data 1922 1922 0 0.0
.data.rel.ro 37440 37440 0 0.0
.dynamic 592 592 0 0.0
.got 3968 3968 0 0.0
.init 27 27 0 0.0
.init_array 544 544 0 0.0
.rodata 137940 137940 0 0.0
.text 1377701 1377685 -16 -0.0
chip-tool debug (read only) 7135381 7135349 -32 -0.0
(read/write) 188296 188296 0 0.0
.bss 36264 36264 0 0.0
.data 1088 1088 0 0.0
.data.rel.ro 144960 144960 0 0.0
.dynamic 608 608 0 0.0
.got 4760 4760 0 0.0
.init 27 27 0 0.0
.init_array 600 600 0 0.0
.rodata 407413 407413 0 0.0
.text 6360597 6360565 -32 -0.0
chip-tool-ipv6only arm64 (read only) 7088532 7088532 0 0.0
(read/write) 290881 290881 0 0.0
.bss 54577 54577 0 0.0
.data 1128 1128 0 0.0
.data.rel.ro 186160 186160 0 0.0
.dynamic 560 560 0 0.0
.got 45232 45232 0 0.0
.init 24 24 0 0.0
.init_array 184 184 0 0.0
.rodata 390460 390460 0 0.0
.text 6097780 6097780 0 0.0
door-lock-app debug (read only) 1852145 1852129 -16 -0.0
(read/write) 118080 118080 0 0.0
.bss 49856 49856 0 0.0
.data 962 962 0 0.0
.data.rel.ro 61856 61856 0 0.0
.dynamic 592 592 0 0.0
.got 4104 4104 0 0.0
.init 27 27 0 0.0
.init_array 664 664 0 0.0
.rodata 171260 171260 0 0.0
.text 1532818 1532802 -16 -0.0
lighting-app debug+rpc (read only) 1953513 1953481 -32 -0.0
(read/write) 123200 123200 0 0.0
.bss 50808 50808 0 0.0
.data 1392 1392 0 0.0
.data.rel.ro 65504 65504 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 704 704 0 0.0
.rodata 161297 161297 0 0.0
.text 1637794 1637762 -32 -0.0
ota-provider-app debug (read only) 1631425 1631409 -16 -0.0
(read/write) 87592 87592 0 0.0
.bss 48736 48736 0 0.0
.data 1064 1064 0 0.0
.data.rel.ro 32296 32296 0 0.0
.dynamic 608 608 0 0.0
.got 4240 4240 0 0.0
.init 27 27 0 0.0
.init_array 608 608 0 0.0
.rodata 147878 147878 0 0.0
.text 1366226 1366210 -16 -0.0
ota-requestor-app debug (read only) 1640193 1640161 -32 -0.0
(read/write) 90672 90672 0 0.0
.bss 50048 50048 0 0.0
.data 1096 1096 0 0.0
.data.rel.ro 34248 34248 0 0.0
.dynamic 592 592 0 0.0
.got 4032 4032 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 144183 144183 0 0.0
.text 1379746 1379714 -32 -0.0
shell debug (read only) 2277977 2277945 -32 -0.0
(read/write) 150096 150096 0 0.0
.bss 70752 70752 0 0.0
.data 768 768 0 0.0
.data.rel.ro 72896 72896 0 0.0
.dynamic 592 592 0 0.0
.got 4160 4160 0 0.0
.init 27 27 0 0.0
.init_array 904 904 0 0.0
.rodata 205138 205138 0 0.0
.text 1916770 1916738 -32 -0.0
thermostat-no-ble arm64 (read only) 2086380 2086364 -16 -0.0
(read/write) 148865 148865 0 0.0
.bss 66177 66177 0 0.0
.data 952 952 0 0.0
.data.rel.ro 74624 74624 0 0.0
.dynamic 560 560 0 0.0
.got 4136 4136 0 0.0
.init 24 24 0 0.0
.init_array 336 336 0 0.0
.rodata 131004 131004 0 0.0
.text 1737648 1737632 -16 -0.0
tv-app debug (read only) 2407873 2407841 -32 -0.0
(read/write) 318864 318864 0 0.0
.bss 237312 237312 0 0.0
.data 3104 3104 0 0.0
.data.rel.ro 72456 72456 0 0.0
.dynamic 592 592 0 0.0
.got 4520 4520 0 0.0
.init 27 27 0 0.0
.init_array 872 872 0 0.0
.rodata 192140 192140 0 0.0
.text 2044370 2044338 -32 -0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2385832 2385832 0 0.0
.bss 189236 189236 0 0.0
.data 5288 5288 0 0.0
.text 1348432 1348432 0 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2338640 2338640 0 0.0
.bss 178152 178152 0 0.0
.data 5568 5568 0 0.0
.text 1301240 1301240 0 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2302296 2302296 0 0.0
.bss 178032 178032 0 0.0
.data 5544 5544 0 0.0
.text 1264896 1264896 0 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1139712 1139712 0 0.0
.bss 11756 11756 0 0.0
.data 4368 4368 0 0.0
.text 103096 103096 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2292804 2292804 0 0.0
.bss 175316 175316 0 0.0
.data 5368 5368 0 0.0
.text 1255376 1255376 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 980927 980927 0 0.0
bss 118524 118524 0 0.0
rodata 116548 116548 0 0.0
text 668068 668072 4 0.0
nrf52840dk_nrf52840+rpc (read/write) 966487 966487 0 0.0
bss 115568 115568 0 0.0
rodata 108020 108020 0 0.0
text 664508 664512 4 0.0
nrf52840dongle_nrf52840 (read/write) 996995 996995 0 0.0
bss 119696 119696 0 0.0
rodata 115380 115380 0 0.0
text 673460 673464 4 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 890126 890126 0 0.0
bss 115312 115312 0 0.0
rodata 109824 109824 0 0.0
text 584256 584260 4 0.0
lock-app nrf52840dk_nrf52840 (read/write) 913143 913159 16 0.0
bss 116884 116884 0 0.0
rodata 104804 104804 0 0.0
text 614068 614072 4 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 823210 823210 0 0.0
bss 113700 113700 0 0.0
rodata 98012 98012 0 0.0
text 531040 531044 4 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 541779 541779 0 0.0
bss 52588 52588 0 0.0
rodata 50048 50048 0 0.0
text 376940 376940 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 915927 915943 16 0.0
bss 116640 116640 0 0.0
rodata 105324 105324 0 0.0
text 616524 616528 4 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 911135 911135 0 0.0
bss 116648 116648 0 0.0
rodata 104428 104428 0 0.0
text 612576 612580 4 0.0
shell nrf52840dk_nrf52840 (read/write) 798379 798379 0 0.0
bss 109772 109772 0 0.0
rodata 78352 78352 0 0.0
text 533752 533752 0 0.0
p6 all-clusters-app default (read/write) 2446944 2446944 0 0.0
.bss 117388 117388 0 0.0
.data 2576 2576 0 0.0
.text 1405208 1405208 0 0.0
light-app default (read/write) 2340536 2340552 16 0.0
.bss 103348 103348 0 0.0
.data 2400 2400 0 0.0
.text 1298800 1298816 16 0.0
lock-app default (read/write) 2305560 2305560 0 0.0
.bss 103068 103068 0 0.0
.data 2360 2360 0 0.0
.text 1263824 1263824 0 0.0
qpg lighting-app qpg6105+debug (read only) 571104 571104 0 0.0
(read/write) 146940 146940 0 0.0
.bss 87496 87496 0 0.0
.data 1056 1056 0 0.0
.text 565784 565784 0 0.0
lock-app qpg6105+debug (read only) 516840 516840 0 0.0
(read/write) 146940 146940 0 0.0
.bss 86936 86936 0 0.0
.data 992 992 0 0.0
.text 511520 511520 0 0.0
persistent-storage-app qpg6105+debug (read only) 107140 107140 0 0.0
(read/write) 146940 146940 0 0.0
.bss 38504 38504 0 0.0
.data 288 288 0 0.0
.text 101820 101820 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 845494 845494 0 0.0
bss 85552 85552 0 0.0
noinit 37160 37160 0 0.0
text 592264 592268 4 0.0

@lnikulin lnikulin changed the title Fix Engine::ScheduleRun() in case callback function done before ScheduleWork() returns. (#14596) Incorrect processing in Engine::ScheduleRun() (#14596) Feb 2, 2022
@lnikulin lnikulin changed the title Incorrect processing in Engine::ScheduleRun() (#14596) Incorrect processing in Engine::ScheduleRun() Feb 2, 2022
mrjerryjohns
mrjerryjohns suggested changes Feb 2, 2022
View reviewed changes
Copy link
Contributor

@mrjerryjohns mrjerryjohns left a comment

Choose a reason for hiding this comment

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

This doesn't actually fix the core issue. If there are multiple threads calling ScheduleRun, this all still breaks.

The correct solution (and resultant analysis needed) is posted in the original issue.

@mrjerryjohns mrjerryjohns self-requested a review February 2, 2022 19:02
bzbarsky-apple
bzbarsky-apple requested changes Feb 3, 2022
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.

Yes, this does not seem like the right fix. Let's follow up in #14596 to figure out what's going on here.

mrjerryjohns
mrjerryjohns suggested changes Feb 10, 2022
View reviewed changes
Copy link
Contributor

@mrjerryjohns mrjerryjohns left a comment

Choose a reason for hiding this comment

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

Given the updates in the related issue, this should be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrjerryjohns mrjerryjohns mrjerryjohns requested changes

@bzbarsky-apple bzbarsky-apple bzbarsky-apple requested changes

@andy31415 andy31415 andy31415 approved these changes

@yufengwangca yufengwangca yufengwangca approved these changes

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

Successfully merging this pull request may close these issues.

Incorrect processing in Engine::ScheduleRun()
6 participants
@lnikulin @CLAassistant @andy31415 @mrjerryjohns @yufengwangca @bzbarsky-apple