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

Allow 'expired system timers' to be cancelled before execution. #22372

Closed

Conversation

andy31415
Copy link
Contributor

Problem

SystemTimers cannot be reliably cancelled for socket impl.

Specifically to avoid starvation, system timers keeps a list of 'active timers' and that is not cancelable.

Change overview

  • Add unit test to verify that we can cancel in-progress timers
  • Fix timer cancellation (have a member list instead of stack variable for 'in process' list
  • Fix unit test for LoopbackTransport (self-cancelling timers on message send scheduling seemed to interfere with tests).

Fixes #22160
Fixes #19387

Testing

Unit tests. CI will also validate.

@andy31415
Copy link
Contributor Author

1.0 Acceptance: high value fix (fixes test flakyness and potential use after free)

@andy31415 andy31415 changed the title System timer cancel fix Allow 'expired system timers' to be cancelled before execution. Sep 2, 2022
class LoopbackTransport : public Transport::Base
{
private:
// Use unique pointers for work callbacks, so that one callback does not cancel another.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a workaround for the real issue, which is that ScheduleWork on the select impl is broken. We should be fixing that, not working around it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes... did not try to yak shave into that one...

@github-actions
Copy link

github-actions bot commented Sep 2, 2022

PR #22372: Size comparison from 3522317 to f1fdaf4

Increases (21 builds for bl602, cc13x2_26x2, esp32, linux, nrfconnect, psoc6, telink)
platform target config section 3522317 f1fdaf4 change % change
bl602 lighting-app bl602 .text 1052584 1052588 4 0.0
cc13x2_26x2 lock-ftd LP_CC2652R7 (read only) 676411 676419 8 0.0
.text 598864 598872 8 0.0
esp32 all-clusters-app c3devkit (read only) 1033912 1033914 2 0.0
.flash.text 1033912 1033914 2 0.0
linux all-clusters-app debug (read only) 3045113 3045177 64 0.0
(read/write) 156032 156064 32 0.0
.bss 61792 61824 32 0.1
.text 2590242 2590306 64 0.0
all-clusters-minimal-app debug (read only) 2880897 2880977 80 0.0
(read/write) 147632 147664 32 0.0
.bss 61024 61056 32 0.1
.text 2428642 2428722 80 0.0
bridge-app debug+rpc (read only) 2378681 2378761 80 0.0
.text 2011474 2011554 80 0.0
chip-tool debug (read only) 10944537 10944617 80 0.0
(read/write) 657320 657352 32 0.0
.bss 25240 25272 32 0.1
.text 8854676 8854756 80 0.0
chip-tool-ipv6only arm64 (read only) 10319348 10319412 64 0.0
(read/write) 705169 705185 16 0.0
.bss 33297 33313 16 0.0
.text 8167636 8167700 64 0.0
lighting-app debug+rpc (read only) 2604105 2604169 64 0.0
.text 2211810 2211874 64 0.0
lock-app debug (read only) 2587105 2587185 80 0.0
.text 2182034 2182114 80 0.0
ota-provider-app debug (read only) 2364345 2364425 80 0.0
.text 1990482 1990562 80 0.0
ota-requestor-app debug (read only) 2529609 2529673 64 0.0
.text 2139874 2139938 64 0.0
shell debug (read only) 2613401 2613481 80 0.0
.text 2219154 2219234 80 0.0
thermostat-no-ble arm64 (read only) 2363012 2363076 64 0.0
.text 1983568 1983632 64 0.0
tv-app debug (read only) 3191689 3191753 64 0.0
.text 2741554 2741618 64 0.0
tv-casting-app debug (read only) 5510017 5510097 80 0.0
.text 4893042 4893122 80 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1181087 1181119 32 0.0
text 815004 815036 32 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1160283 1160299 16 0.0
text 803400 803424 24 0.0
psoc6 lock cy8ckit_062s2_43012 .debug_info 22294086 22294087 1 0.0
telink light-switch-app tlsr9518adk80d (read/write) 809004 809044 40 0.0
text 571480 571522 42 0.0
lighting-app tlsr9518adk80d (read/write) 830912 830960 48 0.0
text 589604 589646 42 0.0
Decreases (5 builds for cc13x2_26x2, esp32, psoc6)
platform target config section 3522317 f1fdaf4 change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read/write) 165308 165300 -8 -0.0
esp32 all-clusters-app m5stack (read/write) 490924 490916 -8 -0.0
.flash.rodata 247464 247456 -8 -0.0
psoc6 all-clusters cy8ckit_062s2_43012 .debug_info 26715435 26715434 -1 -0.0
all-clusters-minimal cy8ckit_062s2_43012 .debug_info 26452059 26452057 -2 -0.0
light cy8ckit_062s2_43012 .debug_info 21914340 21914339 -1 -0.0
Full report (45 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
platform target config section 3522317 f1fdaf4 change % change
bl602 lighting-app bl602 (read/write) 1386190 1386190 0 0.0
.bss 120298 120298 0 0.0
.data 4488 4488 0 0.0
.text 1052584 1052588 4 0.0
bl602+rpc (read/write) 1431846 1431846 0 0.0
.bss 127730 127730 0 0.0
.data 4600 4600 0 0.0
.text 1084344 1084344 0 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 675359 675359 0 0.0
(read/write) 176048 176048 0 0.0
.bss 74300 74300 0 0.0
.data 3380 3380 0 0.0
.rodata 89215 89215 0 0.0
.text 585832 585832 0 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 640071 640071 0 0.0
(read/write) 157868 157868 0 0.0
.bss 73572 73572 0 0.0
.data 3380 3380 0 0.0
.rodata 78367 78367 0 0.0
.text 561384 561384 0 0.0
lock-ftd LP_CC2652R7 (read only) 676411 676419 8 0.0
(read/write) 165308 165300 -8 -0.0
.bss 71500 71500 0 0.0
.data 3304 3304 0 0.0
.rodata 77067 77067 0 0.0
.text 598864 598872 8 0.0
lock-mtd LP_CC2652R7 (read only) 659379 659379 0 0.0
(read/write) 178028 178028 0 0.0
.bss 67188 67188 0 0.0
.data 3304 3304 0 0.0
.rodata 102323 102323 0 0.0
.text 556576 556576 0 0.0
pump-app LP_CC2652R7 (read only) 685247 685247 0 0.0
(read/write) 157176 157176 0 0.0
.bss 71436 71436 0 0.0
.data 3296 3296 0 0.0
.rodata 90079 90079 0 0.0
.text 594684 594684 0 0.0
pump-controller-app LP_CC2652R7 (read only) 669739 669739 0 0.0
(read/write) 172796 172796 0 0.0
.bss 71548 71548 0 0.0
.data 3292 3292 0 0.0
.rodata 85627 85627 0 0.0
.text 583632 583632 0 0.0
shell LP_CC2652R7 (read only) 666010 666010 0 0.0
(read/write) 180916 180916 0 0.0
.bss 76620 76620 0 0.0
.data 3376 3376 0 0.0
.rodata 85770 85770 0 0.0
.text 579924 579924 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 586882 586882 0 0.0
.app_xip_area 463540 463540 0 0.0
.bss 65776 65776 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) 592634 592634 0 0.0
.app_xip_area 464508 464508 0 0.0
.bss 70560 70560 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) 599818 599818 0 0.0
.app_xip_area 477196 477196 0 0.0
.bss 65088 65088 0 0.0
.data 716 716 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read/write) 1108528 1108528 0 0.0
.bss 136332 136332 0 0.0
.data 2072 2072 0 0.0
.text 970104 970104 0 0.0
BRD4161A+rpc (read/write) 972188 972188 0 0.0
.bss 150844 150844 0 0.0
.data 2252 2252 0 0.0
.text 819072 819072 0 0.0
BRD4161A+rs911x (read/write) 1002244 1002244 0 0.0
.bss 169168 169168 0 0.0
.data 2064 2064 0 0.0
.text 830992 830992 0 0.0
lock-app BRD4161A+wf200 (read/write) 1150304 1150304 0 0.0
.bss 152248 152248 0 0.0
.data 2072 2072 0 0.0
.text 995964 995964 0 0.0
window-app BRD4161A (read/write) 1099768 1099768 0 0.0
.bss 137772 137772 0 0.0
.data 2096 2096 0 0.0
.text 959880 959880 0 0.0
esp32 all-clusters-app c3devkit (read only) 1033912 1033914 2 0.0
(read/write) 1493646 1493646 0 0.0
.dram0.bss 71120 71120 0 0.0
.dram0.data 13696 13696 0 0.0
.flash.rodata 218160 218160 0 0.0
.flash.text 1033912 1033914 2 0.0
.iram0.text 65204 65204 0 0.0
m5stack (read only) 1086515 1086515 0 0.0
(read/write) 490924 490916 -8 -0.0
.dram0.bss 76640 76640 0 0.0
.dram0.data 34152 34152 0 0.0
.flash.rodata 247464 247456 -8 -0.0
.flash.text 1081131 1081131 0 0.0
.iram0.text 123939 123939 0 0.0
k32w light k32w0+release (read/write) 648204 648204 0 0.0
.bss 70712 70712 0 0.0
.data 2068 2068 0 0.0
.text 572696 572696 0 0.0
lock k32w0+release (read/write) 705216 705216 0 0.0
.bss 71160 71160 0 0.0
.data 2076 2076 0 0.0
.text 629252 629252 0 0.0
linux all-clusters-app debug (read only) 3045113 3045177 64 0.0
(read/write) 156032 156064 32 0.0
.bss 61792 61824 32 0.1
.data 2096 2096 0 0.0
.data.rel.ro 85768 85768 0 0.0
.dynamic 608 608 0 0.0
.got 4568 4568 0 0.0
.init 27 27 0 0.0
.init_array 1176 1176 0 0.0
.rodata 275435 275435 0 0.0
.text 2590242 2590306 64 0.0
all-clusters-minimal-app debug (read only) 2880897 2880977 80 0.0
(read/write) 147632 147664 32 0.0
.bss 61024 61056 32 0.1
.data 2064 2064 0 0.0
.data.rel.ro 78264 78264 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 1160 1160 0 0.0
.rodata 275595 275595 0 0.0
.text 2428642 2428722 80 0.0
bridge-app debug+rpc (read only) 2378681 2378761 80 0.0
(read/write) 127752 127752 0 0.0
.bss 50656 50656 0 0.0
.data 3600 3600 0 0.0
.data.rel.ro 67640 67640 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 204296 204296 0 0.0
.text 2011474 2011554 80 0.0
chip-tool debug (read only) 10944537 10944617 80 0.0
(read/write) 657320 657352 32 0.0
.bss 25240 25272 32 0.1
.data 3266 3266 0 0.0
.data.rel.ro 622288 622288 0 0.0
.dynamic 608 608 0 0.0
.got 5096 5096 0 0.0
.init 27 27 0 0.0
.init_array 776 776 0 0.0
.rodata 569045 569045 0 0.0
.text 8854676 8854756 80 0.0
chip-tool-ipv6only arm64 (read only) 10319348 10319412 64 0.0
(read/write) 705169 705185 16 0.0
.bss 33297 33313 16 0.0
.data 3280 3280 0 0.0
.data.rel.ro 649784 649784 0 0.0
.dynamic 560 560 0 0.0
.got 13840 13840 0 0.0
.init 24 24 0 0.0
.init_array 200 200 0 0.0
.rodata 499300 499300 0 0.0
.text 8167636 8167700 64 0.0
lighting-app debug+rpc (read only) 2604105 2604169 64 0.0
(read/write) 130536 130536 0 0.0
.bss 49792 49792 0 0.0
.data 2096 2096 0 0.0
.data.rel.ro 72680 72680 0 0.0
.dynamic 608 608 0 0.0
.got 4392 4392 0 0.0
.init 27 27 0 0.0
.init_array 928 928 0 0.0
.rodata 221104 221104 0 0.0
.text 2211810 2211874 64 0.0
lock-app debug (read only) 2587105 2587185 80 0.0
(read/write) 125712 125712 0 0.0
.bss 48288 48288 0 0.0
.data 1712 1712 0 0.0
.data.rel.ro 69688 69688 0 0.0
.dynamic 608 608 0 0.0
.got 4464 4464 0 0.0
.init 27 27 0 0.0
.init_array 904 904 0 0.0
.rodata 238128 238128 0 0.0
.text 2182034 2182114 80 0.0
ota-provider-app debug (read only) 2364345 2364425 80 0.0
(read/write) 119144 119144 0 0.0
.bss 47808 47808 0 0.0
.data 1936 1936 0 0.0
.data.rel.ro 63512 63512 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 768 768 0 0.0
.rodata 210104 210104 0 0.0
.text 1990482 1990562 80 0.0
ota-requestor-app debug (read only) 2529609 2529673 64 0.0
(read/write) 127552 127552 0 0.0
.bss 50368 50368 0 0.0
.data 2304 2304 0 0.0
.data.rel.ro 68920 68920 0 0.0
.dynamic 608 608 0 0.0
.got 4480 4480 0 0.0
.init 27 27 0 0.0
.init_array 856 856 0 0.0
.rodata 216832 216832 0 0.0
.text 2139874 2139938 64 0.0
shell debug (read only) 2613401 2613481 80 0.0
(read/write) 142184 142184 0 0.0
.bss 57704 57704 0 0.0
.data 1264 1264 0 0.0
.data.rel.ro 77376 77376 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 1048 1048 0 0.0
.rodata 235538 235538 0 0.0
.text 2219154 2219234 80 0.0
thermostat-no-ble arm64 (read only) 2363012 2363076 64 0.0
(read/write) 141857 141857 0 0.0
.bss 55233 55233 0 0.0
.data 1680 1680 0 0.0
.data.rel.ro 76112 76112 0 0.0
.dynamic 560 560 0 0.0
.got 5056 5056 0 0.0
.init 24 24 0 0.0
.init_array 416 416 0 0.0
.rodata 141404 141404 0 0.0
.text 1983568 1983632 64 0.0
tv-app debug (read only) 3191689 3191753 64 0.0
(read/write) 258040 258040 0 0.0
.bss 167352 167352 0 0.0
.data 4752 4752 0 0.0
.data.rel.ro 79368 79368 0 0.0
.dynamic 608 608 0 0.0
.got 4856 4856 0 0.0
.init 27 27 0 0.0
.init_array 1080 1080 0 0.0
.rodata 260104 260104 0 0.0
.text 2741554 2741618 64 0.0
tv-casting-app debug (read only) 5510017 5510097 80 0.0
(read/write) 160536 160536 0 0.0
.bss 51352 51352 0 0.0
.data 2432 2432 0 0.0
.data.rel.ro 100304 100304 0 0.0
.dynamic 608 608 0 0.0
.got 4776 4776 0 0.0
.init 27 27 0 0.0
.init_array 1048 1048 0 0.0
.rodata 345073 345073 0 0.0
.text 4893042 4893122 80 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2455184 2455184 0 0.0
.bss 215044 215044 0 0.0
.data 5872 5872 0 0.0
.text 1417828 1417828 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1181087 1181119 32 0.0
bss 143641 143641 0 0.0
rodata 143504 143504 0 0.0
text 815004 815036 32 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1160283 1160299 16 0.0
bss 142868 142868 0 0.0
rodata 135092 135092 0 0.0
text 803400 803424 24 0.0
psoc6 all-clusters cy8ckit_062s2_43012 (read only) 841960 841960 0 0.0
(read/write) 1742748 1742748 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 188720 188720 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2664 2664 0 0.0
.debug_abbrev 1221450 1221450 0 0.0
.debug_aranges 111728 111728 0 0.0
.debug_frame 372968 372968 0 0.0
.debug_info 26715435 26715434 -1 -0.0
.debug_line 3657402 3657402 0 0.0
.debug_loc 3573108 3573108 0 0.0
.debug_ranges 338424 338424 0 0.0
.debug_str 3427409 3427409 0 0.0
.heap 841960 841960 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 570588 570588 0 0.0
.symtab 421488 421488 0 0.0
.text 1542976 1542976 0 0.0
.zero.table 8 8 0 0.0
text 0 0 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 (read only) 842696 842696 0 0.0
(read/write) 1685948 1685948 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 187984 187984 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2664 2664 0 0.0
.debug_abbrev 1213289 1213289 0 0.0
.debug_aranges 111200 111200 0 0.0
.debug_frame 376048 376048 0 0.0
.debug_info 26452059 26452057 -2 -0.0
.debug_line 3677918 3677918 0 0.0
.debug_loc 3560745 3560745 0 0.0
.debug_ranges 337040 337040 0 0.0
.debug_str 3416414 3416414 0 0.0
.heap 842696 842696 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 535062 535062 0 0.0
.symtab 408080 408080 0 0.0
.text 1486912 1486912 0 0.0
.zero.table 0 0 0 0.0
8 8 0 0.0
light cy8ckit_062s2_43012 (read only) 850928 850928 0 0.0
(read/write) 1603196 1603196 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 179960 179960 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2456 2456 0 0.0
.debug_abbrev 1048108 1048108 0 0.0
.debug_aranges 103376 103376 0 0.0
.debug_frame 346316 346316 0 0.0
.debug_info 21914340 21914339 -1 -0.0
.debug_line 3248383 3248383 0 0.0
.debug_loc 3259088 3259088 0 0.0
.debug_ranges 302512 302512 0 0.0
.debug_str 3221680 3221680 0 0.0
.heap 850928 850928 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 468361 468361 0 0.0
.symtab 375168 375168 0 0.0
.text 1412392 1412392 0 0.0
.zero.table 0 0 0 0.0
8 8 0 0.0
lock cy8ckit_062s2_43012 (read only) 845896 845896 0 0.0
(read/write) 1640892 1640892 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 184976 184976 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2472 2472 0 0.0
.debug_abbrev 1055543 1055543 0 0.0
.debug_aranges 104048 104048 0 0.0
.debug_frame 349144 349144 0 0.0
.debug_info 22294086 22294087 1 0.0
.debug_line 3257204 3257204 0 0.0
.debug_loc 3298941 3298941 0 0.0
.debug_ranges 305856 305856 0 0.0
.debug_str 3249101 3249101 0 0.0
.heap 845896 845896 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 474564 474564 0 0.0
.symtab 378352 378352 0 0.0
.text 1445056 1445056 0 0.0
.zero.table 0 0 0 0.0
8 8 0 0.0
qpg lighting-app qpg6105+debug (read/write) 1129172 1129172 0 0.0
.bss 106112 106112 0 0.0
.data 1028 1028 0 0.0
.text 576268 576268 0 0.0
lock-app qpg6105+debug (read/write) 1100184 1100184 0 0.0
.bss 102344 102344 0 0.0
.data 1032 1032 0 0.0
.text 547284 547284 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 809004 809044 40 0.0
bss 71344 71344 0 0.0
noinit 43488 43488 0 0.0
text 571480 571522 42 0.0
lighting-app tlsr9518adk80d (read/write) 830912 830960 48 0.0
bss 72200 72200 0 0.0
noinit 43488 43488 0 0.0
text 589604 589646 42 0.0

@andy31415
Copy link
Contributor Author

Closing as duplicate with #22375 and that one fixes more (fixes ScheduleWork too)

@andy31415 andy31415 closed this Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants