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

Stop calling the attribute change callback on the wrong thread in bridge-app. #21230

Merged

Conversation

bzbarsky-apple
Copy link
Contributor

A few changes here:

  1. Stop trying to pass values to the attribute change callback, since those get
    ignored anyway.
  2. Run the attribute change callback off a task on the Matter thread, instead of
    calling it directly from a different thread.
  3. Change initial bridge setup to set state before setting change callbacks, so
    we won't try to notify changes at startup before the Matter stack is actually
    initialized.

Fixes #21212

Problem

See above.

Change overview

See above.

Testing

Ran bridge-app locally. Tried triggering things by typing b and c followed by return in the bridge app, and with these changes no longer fail the threading asserts.

…dge-app.

A few changes here:

1. Stop trying to pass values to the attribute change callback, since those get
   ignored anyway.
2. Run the attribute change callback off a task on the Matter thread, instead of
   calling it directly from a different thread.
3. Change initial bridge setup to set state before setting change callbacks, so
   we won't try to notify changes at startup before the Matter stack is actually
   initialized.

Fixes project-chip#21212
@github-actions
Copy link

github-actions bot commented Jul 26, 2022

PR #21230: Size comparison from d94cbdc to 892107a

Increases (8 builds for bl602, cc13x2_26x2, cyw30739, k32w, linux, telink)
platform target config section d94cbdc 892107a change % change
bl602 lighting-app bl602 .text 1051376 1051380 4 0.0
bl602+rpc .text 1083032 1083036 4 0.0
cc13x2_26x2 all-clusters-minimal-app LP_CC2652R7 (read only) 633911 633919 8 0.0
.text 555980 555988 8 0.0
shell LP_CC2652R7 (read only) 660802 660810 8 0.0
.text 575332 575340 8 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 583234 583242 8 0.0
.app_xip_area 460372 460380 8 0.0
k32w lock k32w0+release (read/write) 698956 698972 16 0.0
.text 624024 624040 16 0.0
linux bridge-app debug+rpc (read only) 2342921 2343033 112 0.0
.text 1980514 1980626 112 0.0
telink light-switch-app tlsr9518adk80d text 567106 567108 2 0.0
Decreases (3 builds for cc13x2_26x2, esp32, telink)
platform target config section d94cbdc 892107a change % change
cc13x2_26x2 shell LP_CC2652R7 (read/write) 186076 186068 -8 -0.0
esp32 all-clusters-app c3devkit (read only) 1021998 1021996 -2 -0.0
.flash.text 1021998 1021996 -2 -0.0
telink lighting-app tlsr9518adk80d text 583678 583676 -2 -0.0
Full report (43 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section d94cbdc 892107a change % change
bl602 lighting-app bl602 (read/write) 1381290 1381290 0 0.0
.bss 117538 117538 0 0.0
.data 4480 4480 0 0.0
.text 1051376 1051380 4 0.0
bl602+rpc (read/write) 1426698 1426698 0 0.0
.bss 124978 124978 0 0.0
.data 4600 4600 0 0.0
.text 1083032 1083036 4 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 668319 668319 0 0.0
(read/write) 183040 183040 0 0.0
.bss 74252 74252 0 0.0
.data 3356 3356 0 0.0
.rodata 88383 88383 0 0.0
.text 579620 579620 0 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 633911 633919 8 0.0
(read/write) 157820 157820 0 0.0
.bss 73548 73548 0 0.0
.data 3356 3356 0 0.0
.rodata 77607 77607 0 0.0
.text 555980 555988 8 0.0
lock-ftd LP_CC2652R7 (read only) 671475 671475 0 0.0
(read/write) 170076 170076 0 0.0
.bss 71332 71332 0 0.0
.data 3280 3280 0 0.0
.rodata 76443 76443 0 0.0
.text 594552 594552 0 0.0
lock-mtd LP_CC2652R7 (read only) 653711 653711 0 0.0
(read/write) 183528 183528 0 0.0
.bss 67020 67020 0 0.0
.data 3280 3280 0 0.0
.rodata 101143 101143 0 0.0
.text 552088 552088 0 0.0
pump-app LP_CC2652R7 (read only) 680907 680907 0 0.0
(read/write) 161476 161476 0 0.0
.bss 71396 71396 0 0.0
.data 3280 3280 0 0.0
.rodata 89147 89147 0 0.0
.text 591276 591276 0 0.0
pump-controller-app LP_CC2652R7 (read only) 666659 666659 0 0.0
(read/write) 175860 175860 0 0.0
.bss 71532 71532 0 0.0
.data 3276 3276 0 0.0
.rodata 84987 84987 0 0.0
.text 581192 581192 0 0.0
shell LP_CC2652R7 (read only) 660802 660810 8 0.0
(read/write) 186076 186068 -8 -0.0
.bss 76572 76572 0 0.0
.data 3360 3360 0 0.0
.rodata 85154 85154 0 0.0
.text 575332 575340 8 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 583234 583242 8 0.0
.app_xip_area 460372 460380 8 0.0
.bss 65656 65656 0 0.0
.data 716 716 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 589162 589162 0 0.0
.app_xip_area 461572 461572 0 0.0
.bss 70384 70384 0 0.0
.data 720 720 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 588998 588998 0 0.0
.app_xip_area 466952 466952 0 0.0
.bss 64896 64896 0 0.0
.data 660 660 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read/write) 1088056 1088056 0 0.0
.bss 133276 133276 0 0.0
.data 2048 2048 0 0.0
.text 952712 952712 0 0.0
BRD4161A+rpc (read/write) 1142356 1142356 0 0.0
.bss 149956 149956 0 0.0
.data 2260 2260 0 0.0
.text 990120 990120 0 0.0
BRD4161A+rs911x (read/write) 952592 952592 0 0.0
.bss 140992 140992 0 0.0
.data 2048 2048 0 0.0
.text 809532 809532 0 0.0
lock-app BRD4161A+wf200 (read/write) 1128188 1128188 0 0.0
.bss 144360 144360 0 0.0
.data 2056 2056 0 0.0
.text 981752 981752 0 0.0
window-app BRD4161A (read/write) 1081540 1081540 0 0.0
.bss 134748 134748 0 0.0
.data 2076 2076 0 0.0
.text 944692 944692 0 0.0
esp32 all-clusters-app c3devkit (read only) 1021998 1021996 -2 -0.0
(read/write) 1486562 1486562 0 0.0
.dram0.bss 70288 70288 0 0.0
.dram0.data 14600 14600 0 0.0
.flash.rodata 216240 216240 0 0.0
.flash.text 1021998 1021996 -2 -0.0
.iram0.text 62902 62902 0 0.0
m5stack (read only) 1075575 1075575 0 0.0
(read/write) 488576 488576 0 0.0
.dram0.bss 75800 75800 0 0.0
.dram0.data 34144 34144 0 0.0
.flash.rodata 246636 246636 0 0.0
.flash.text 1070191 1070191 0 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w0+release (read/write) 641684 641684 0 0.0
.bss 69728 69728 0 0.0
.data 2028 2028 0 0.0
.text 567200 567200 0 0.0
lock k32w0+release (read/write) 698956 698972 16 0.0
.bss 70168 70168 0 0.0
.data 2036 2036 0 0.0
.text 624024 624040 16 0.0
linux all-clusters-app debug (read only) 2985145 2985145 0 0.0
(read/write) 155416 155416 0 0.0
.bss 61856 61856 0 0.0
.data 2064 2064 0 0.0
.data.rel.ro 85224 85224 0 0.0
.dynamic 608 608 0 0.0
.got 4568 4568 0 0.0
.init 27 27 0 0.0
.init_array 1072 1072 0 0.0
.rodata 267627 267627 0 0.0
.text 2539330 2539330 0 0.0
all-clusters-minimal-app debug (read only) 2828025 2828025 0 0.0
(read/write) 147120 147120 0 0.0
.bss 61056 61056 0 0.0
.data 2064 2064 0 0.0
.data.rel.ro 77816 77816 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 1064 1064 0 0.0
.rodata 267627 267627 0 0.0
.text 2384818 2384818 0 0.0
bridge-app debug+rpc (read only) 2342921 2343033 112 0.0
(read/write) 127024 127024 0 0.0
.bss 50144 50144 0 0.0
.data 3824 3824 0 0.0
.data.rel.ro 67272 67272 0 0.0
.dynamic 608 608 0 0.0
.got 4392 4392 0 0.0
.init 27 27 0 0.0
.init_array 744 744 0 0.0
.rodata 199976 199976 0 0.0
.text 1980514 1980626 112 0.0
chip-tool debug (read only) 10383585 10383585 0 0.0
(read/write) 631056 631056 0 0.0
.bss 24824 24824 0 0.0
.data 3266 3266 0 0.0
.data.rel.ro 596568 596568 0 0.0
.dynamic 608 608 0 0.0
.got 5088 5088 0 0.0
.init 27 27 0 0.0
.init_array 656 656 0 0.0
.rodata 531413 531413 0 0.0
.text 8400692 8400692 0 0.0
chip-tool-ipv6only arm64 (read only) 9808260 9808260 0 0.0
(read/write) 678561 678561 0 0.0
.bss 32897 32897 0 0.0
.data 3272 3272 0 0.0
.data.rel.ro 623904 623904 0 0.0
.dynamic 560 560 0 0.0
.got 13536 13536 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 466196 466196 0 0.0
.text 7759860 7759860 0 0.0
lighting-app debug+rpc (read only) 2565993 2565993 0 0.0
(read/write) 129992 129992 0 0.0
.bss 49696 49696 0 0.0
.data 2096 2096 0 0.0
.data.rel.ro 72328 72328 0 0.0
.dynamic 608 608 0 0.0
.got 4392 4392 0 0.0
.init 27 27 0 0.0
.init_array 832 832 0 0.0
.rodata 215664 215664 0 0.0
.text 2179810 2179810 0 0.0
lock-app debug (read only) 2531049 2531049 0 0.0
(read/write) 124976 124976 0 0.0
.bss 48096 48096 0 0.0
.data 1712 1712 0 0.0
.data.rel.ro 69304 69304 0 0.0
.dynamic 608 608 0 0.0
.got 4424 4424 0 0.0
.init 27 27 0 0.0
.init_array 808 808 0 0.0
.rodata 230672 230672 0 0.0
.text 2134658 2134658 0 0.0
ota-provider-app debug (read only) 2334825 2334825 0 0.0
(read/write) 118776 118776 0 0.0
.bss 47744 47744 0 0.0
.data 1936 1936 0 0.0
.data.rel.ro 63288 63288 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 688 688 0 0.0
.rodata 205656 205656 0 0.0
.text 1965842 1965842 0 0.0
ota-requestor-app debug (read only) 2459769 2459769 0 0.0
(read/write) 126168 126168 0 0.0
.bss 50112 50112 0 0.0
.data 2240 2240 0 0.0
.data.rel.ro 67960 67960 0 0.0
.dynamic 608 608 0 0.0
.got 4480 4480 0 0.0
.init 27 27 0 0.0
.init_array 752 752 0 0.0
.rodata 209600 209600 0 0.0
.text 2078226 2078226 0 0.0
shell debug (read only) 2568785 2568785 0 0.0
(read/write) 141568 141568 0 0.0
.bss 57704 57704 0 0.0
.data 1264 1264 0 0.0
.data.rel.ro 76888 76888 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 944 944 0 0.0
.rodata 230002 230002 0 0.0
.text 2181106 2181106 0 0.0
thermostat-no-ble arm64 (read only) 2341196 2341196 0 0.0
(read/write) 141345 141345 0 0.0
.bss 55297 55297 0 0.0
.data 1672 1672 0 0.0
.data.rel.ro 75624 75624 0 0.0
.dynamic 560 560 0 0.0
.got 4984 4984 0 0.0
.init 24 24 0 0.0
.init_array 400 400 0 0.0
.rodata 139428 139428 0 0.0
.text 1964864 1964864 0 0.0
tv-app debug (read only) 3115577 3115577 0 0.0
(read/write) 257160 257160 0 0.0
.bss 167160 167160 0 0.0
.data 4736 4736 0 0.0
.data.rel.ro 78824 78824 0 0.0
.dynamic 608 608 0 0.0
.got 4848 4848 0 0.0
.init 27 27 0 0.0
.init_array 968 968 0 0.0
.rodata 251336 251336 0 0.0
.text 2675698 2675698 0 0.0
tv-casting-app debug (read only) 5369609 5369609 0 0.0
(read/write) 158432 158432 0 0.0
.bss 51320 51320 0 0.0
.data 2432 2432 0 0.0
.data.rel.ro 98384 98384 0 0.0
.dynamic 608 608 0 0.0
.got 4736 4736 0 0.0
.init 27 27 0 0.0
.init_array 928 928 0 0.0
.rodata 335393 335393 0 0.0
.text 4767570 4767570 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2449240 2449240 0 0.0
.bss 214508 214508 0 0.0
.data 5872 5872 0 0.0
.text 1411884 1411884 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1177023 1177023 0 0.0
bss 143132 143132 0 0.0
rodata 142632 142632 0 0.0
text 812332 812332 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1157075 1157075 0 0.0
bss 142368 142368 0 0.0
rodata 134164 134164 0 0.0
text 801644 801644 0 0.0
p6 all-clusters-app default (read only) 881568 881568 0 0.0
(read/write) 1686924 1686924 0 0.0
.bss 149128 149128 0 0.0
.data 2648 2648 0 0.0
.text 1526760 1526760 0 0.0
all-clusters-minimal-app default (read only) 882288 882288 0 0.0
(read/write) 1631044 1631044 0 0.0
.bss 148408 148408 0 0.0
.data 2648 2648 0 0.0
.text 1471600 1471600 0 0.0
light-app default (read only) 890592 890592 0 0.0
(read/write) 1551396 1551396 0 0.0
.bss 140312 140312 0 0.0
.data 2440 2440 0 0.0
.text 1400256 1400256 0 0.0
lock-app default (read only) 886120 886120 0 0.0
(read/write) 1588988 1588988 0 0.0
.bss 144768 144768 0 0.0
.data 2456 2456 0 0.0
.text 1433376 1433376 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 799556 799556 0 0.0
bss 70808 70808 0 0.0
noinit 40416 40416 0 0.0
text 567106 567108 2 0.0
lighting-app tlsr9518adk80d (read/write) 819656 819656 0 0.0
bss 71652 71652 0 0.0
noinit 40416 40416 0 0.0
text 583678 583676 -2 -0.0

@woody-apple
Copy link
Contributor

Fast tracking platform changes.

@woody-apple woody-apple merged commit 4573cf6 into project-chip:master Jul 27, 2022
@bzbarsky-apple bzbarsky-apple deleted the fix-bridge-threading branch July 27, 2022 20:16
github-actions bot pushed a commit that referenced this pull request Jul 27, 2022
…dge-app. (#21230)

A few changes here:

1. Stop trying to pass values to the attribute change callback, since those get
   ignored anyway.
2. Run the attribute change callback off a task on the Matter thread, instead of
   calling it directly from a different thread.
3. Change initial bridge setup to set state before setting change callbacks, so
   we won't try to notify changes at startup before the Matter stack is actually
   initialized.

Fixes #21212
woody-apple added a commit that referenced this pull request Jul 28, 2022
…dge-app. (#21230) (#21309)

A few changes here:

1. Stop trying to pass values to the attribute change callback, since those get
   ignored anyway.
2. Run the attribute change callback off a task on the Matter thread, instead of
   calling it directly from a different thread.
3. Change initial bridge setup to set state before setting change callbacks, so
   we won't try to notify changes at startup before the Matter stack is actually
   initialized.

Fixes #21212

Co-authored-by: Boris Zbarsky <[email protected]>
isiu-apple pushed a commit to isiu-apple/connectedhomeip that referenced this pull request Sep 16, 2022
…dge-app. (project-chip#21230)

A few changes here:

1. Stop trying to pass values to the attribute change callback, since those get
   ignored anyway.
2. Run the attribute change callback off a task on the Matter thread, instead of
   calling it directly from a different thread.
3. Change initial bridge setup to set state before setting change callbacks, so
   we won't try to notify changes at startup before the Matter stack is actually
   initialized.

Fixes project-chip#21212
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.

Bridge-functionality - The keypress is aborting the DUT
2 participants