connectedhomeip
Prevent cancellation of task with timeout in asyncio runner.
#35045
Merged
Overview
Commits
1
Changes
View On GitHub

Prevent cancellation of task with timeout in asyncio runner. #35045

mergify merged 1 commit into project-chip:master from andy31415:fix_cancellation
andy31415
andy31415247 days ago (edited 247 days ago)

I am yet unclear why on new python/os/websocket we do not get a timeout error, however from what I could tell, wait_for would cancel the websocket.recv and that stops instead of throwing a timeout exception, resulting in a full success result.

What I believe is happening:

  • wait_for attempts a cancel
  • new websocket does handle cancel and returns immediately
  • test execution for loop returns immediately and executes the finally returning True
  • True is processed

Something is still off because I do not understand how we can execute the finally even though our loop is fully interrupted and no exception is reported. I have never seen such a thing (an entire block is skipped without an exeception and finally takes effect). Shield essentially protects the execution from being cancelled by wait_for.

Making this change seems to make the test TestPurposefulFailureExtraReportingOnToggle pass on my machine (well ... fail as expected instead of passing with a successful stop)
This fixes repl tests after trying to move to ubunt 24.04 (see #35009)

This is what we are dealing with:

image

andreilitvin Prevent cancellation of task with timeout in asyncio runner.
209b0bcf
semanticdiff-com
semanticdiff-com247 days ago (edited 247 days ago)

Review changes with SemanticDiff.

Analyzed 1 of 1 files.

Overall, the semantic diff is 50% smaller than the GitHub diff.

Filename Status
✔️ scripts/py_matter_yamltests/matter_yamltests/runner.py 49.55% smaller
github-actions github-actions added scripts
pullapprove pullapprove requested a review from andyg-apple andyg-apple 247 days ago
pullapprove pullapprove requested a review from anush-apple anush-apple 247 days ago
pullapprove pullapprove requested a review from arkq arkq 247 days ago
pullapprove pullapprove requested a review from axelnxp axelnxp 247 days ago
pullapprove pullapprove requested a review from bauerschwan bauerschwan 247 days ago
pullapprove pullapprove requested a review from bzbarsky-apple bzbarsky-apple 247 days ago
pullapprove pullapprove requested a review from carol-apple carol-apple 247 days ago
pullapprove pullapprove requested a review from cecille cecille 247 days ago
pullapprove pullapprove requested a review from chapongatien chapongatien 247 days ago
pullapprove pullapprove requested a review from chrisdecenzo chrisdecenzo 247 days ago
pullapprove pullapprove requested a review from chshu chshu 247 days ago
pullapprove pullapprove requested a review from chulspro chulspro 247 days ago
pullapprove pullapprove requested a review from cliffamzn cliffamzn 247 days ago
pullapprove pullapprove requested a review from Damian-Nordic Damian-Nordic 247 days ago
pullapprove pullapprove requested a review from dhrishi dhrishi 247 days ago
pullapprove pullapprove requested a review from doru91 doru91 247 days ago
pullapprove pullapprove requested a review from fessehaeve fessehaeve 247 days ago
pullapprove pullapprove requested a review from harimau-qirex harimau-qirex 247 days ago
pullapprove pullapprove requested a review from harsha-rajendran harsha-rajendran 247 days ago
pullapprove pullapprove requested a review from hawk248 hawk248 247 days ago
pullapprove pullapprove requested a review from hicklin hicklin 247 days ago
pullapprove pullapprove requested a review from jepenven-silabs jepenven-silabs 247 days ago
pullapprove pullapprove requested a review from jmartinez-silabs jmartinez-silabs 247 days ago
pullapprove pullapprove requested a review from jmeg-sfy jmeg-sfy 247 days ago
pullapprove pullapprove requested a review from joonhaengHeo joonhaengHeo 247 days ago
pullapprove pullapprove requested a review from jtung-apple jtung-apple 247 days ago
pullapprove pullapprove requested a review from kiel-apple kiel-apple 247 days ago
pullapprove pullapprove requested a review from kkasperczyk-no kkasperczyk-no 247 days ago
pullapprove pullapprove requested a review from kpschoedel kpschoedel 247 days ago
pullapprove pullapprove requested a review from ksperling-apple ksperling-apple 247 days ago
pullapprove pullapprove requested a review from lazarkov lazarkov 247 days ago
pullapprove pullapprove requested a review from lpbeliveau-silabs lpbeliveau-silabs 247 days ago
pullapprove pullapprove requested a review from LuDuda LuDuda 247 days ago
pullapprove pullapprove requested a review from mhazley mhazley 247 days ago
pullapprove pullapprove requested a review from mkardous-silabs mkardous-silabs 247 days ago
pullapprove pullapprove requested a review from mspang mspang 247 days ago
pullapprove pullapprove requested a review from nivi-apple nivi-apple 247 days ago
pullapprove pullapprove requested a review from p0fi p0fi 247 days ago
pullapprove pullapprove requested a review from pidarped pidarped 247 days ago
pullapprove pullapprove requested a review from plauric plauric 247 days ago
pullapprove pullapprove requested a review from rcasallas-silabs rcasallas-silabs 247 days ago
pullapprove pullapprove requested a review from ReneJosefsen ReneJosefsen 247 days ago
pullapprove pullapprove requested a review from robszewczyk robszewczyk 247 days ago
pullapprove pullapprove requested a review from saurabhst saurabhst 247 days ago
pullapprove pullapprove requested a review from selissia selissia 247 days ago
pullapprove pullapprove requested a review from sharadb-amazon sharadb-amazon 247 days ago
pullapprove pullapprove requested a review from tcarmelveilleux tcarmelveilleux 247 days ago
pullapprove pullapprove requested a review from tecimovic tecimovic 247 days ago
pullapprove pullapprove requested a review from tima-q tima-q 247 days ago
pullapprove pullapprove requested a review from tobiasgraf tobiasgraf 247 days ago
pullapprove pullapprove requested a review from turon turon 247 days ago
pullapprove pullapprove requested a review from vivien-apple vivien-apple 247 days ago
pullapprove pullapprove requested a review from wiba-nordic wiba-nordic 247 days ago
pullapprove pullapprove requested a review from woody-apple woody-apple 247 days ago
pullapprove pullapprove requested a review from younghak-hwang younghak-hwang 247 days ago
pullapprove pullapprove requested a review from yufengwangca yufengwangca 247 days ago
pullapprove pullapprove requested a review from yunhanw-google yunhanw-google 247 days ago
pullapprove pullapprove added review - pending
github-actions
github-actions247 days ago (edited 247 days ago)

PR #35045: Size comparison from 4e6847d to 209b0bc

Full report (77 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
platform target config section 4e6847d 209b0bc change % change
bl602 lighting-app bl602 FLASH 1278282 1278282 0 0.0
RAM 95880 95880 0 0.0
bl602+mfd FLASH 1292392 1292392 0 0.0
RAM 96024 96024 0 0.0
bl602+rpc FLASH 1317246 1317246 0 0.0
RAM 104304 104304 0 0.0
bl702 lighting-app bl702 FLASH 944186 944186 0 0.0
RAM 15209 15209 0 0.0
bl702+mfd FLASH 946586 946586 0 0.0
RAM 15353 15353 0 0.0
bl702+rpc FLASH 1039242 1039242 0 0.0
RAM 24221 24221 0 0.0
bl706-eth FLASH 646848 646848 0 0.0
RAM 25305 25305 0 0.0
bl706-wifi FLASH 894448 894448 0 0.0
RAM 14525 14525 0 0.0
bl702l lighting-app bl702l FLASH 960590 960590 0 0.0
RAM 16868 16868 0 0.0
bl702l+mfd FLASH 963468 963468 0 0.0
RAM 17012 17012 0 0.0
cyw30739 light CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 672465 672465 0 0.0
RAM 78300 78300 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 692325 692325 0 0.0
RAM 80932 80932 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 692325 692325 0 0.0
RAM 80932 80932 0 0.0
CYW930739M2EVB-02 unknown 2040 2040 0 0.0
FLASH 649261 649261 0 0.0
RAM 73368 73368 0 0.0
light-switch CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 610929 610929 0 0.0
RAM 71292 71292 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 630557 630557 0 0.0
RAM 73844 73844 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 630557 630557 0 0.0
RAM 73844 73844 0 0.0
lock CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 628969 628969 0 0.0
RAM 74308 74308 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 648677 648677 0 0.0
RAM 76860 76860 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 648677 648677 0 0.0
RAM 76860 76860 0 0.0
thermostat CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 601917 601917 0 0.0
RAM 68340 68340 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 621777 621777 0 0.0
RAM 70972 70972 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 621777 621777 0 0.0
RAM 70972 70972 0 0.0
efr32 lighting-app BRD4187C FLASH 930496 930496 0 0.0
RAM 135096 135096 0 0.0
lock-app BRD4338a FLASH 738276 738276 0 0.0
RAM 207816 207816 0 0.0
window-app BRD4187C FLASH 1015652 1015652 0 0.0
RAM 127036 127036 0 0.0
esp32 all-clusters-app c3devkit DRAM 94128 94128 0 0.0
FLASH 1533652 1533652 0 0.0
IRAM 82538 82538 0 0.0
m5stack DRAM 115056 115056 0 0.0
FLASH 1543398 1543398 0 0.0
IRAM 117039 117039 0 0.0
linux air-purifier-app debug unknown 4624 4624 0 0.0
FLASH 2755435 2755435 0 0.0
RAM 128408 128408 0 0.0
all-clusters-app debug unknown 5408 5408 0 0.0
FLASH 6037492 6037492 0 0.0
RAM 509704 509704 0 0.0
all-clusters-minimal-app debug unknown 5304 5304 0 0.0
FLASH 5376992 5376992 0 0.0
RAM 240072 240072 0 0.0
bridge-app debug unknown 5288 5288 0 0.0
FLASH 4729232 4729232 0 0.0
RAM 216952 216952 0 0.0
chip-tool debug unknown 5832 5832 0 0.0
FLASH 12488688 12488688 0 0.0
RAM 557202 557202 0 0.0
chip-tool-ipv6only arm64 unknown 20312 20312 0 0.0
FLASH 11164284 11164284 0 0.0
RAM 607272 607272 0 0.0
fabric-admin debug unknown 5672 5672 0 0.0
FLASH 11221555 11221555 0 0.0
RAM 555306 555306 0 0.0
fabric-bridge-app debug unknown 4520 4520 0 0.0
FLASH 4540174 4540174 0 0.0
RAM 203456 203456 0 0.0
lighting-app debug+rpc+ui unknown 5936 5936 0 0.0
FLASH 5663793 5663793 0 0.0
RAM 228008 228008 0 0.0
lock-app debug unknown 5224 5224 0 0.0
FLASH 4784578 4784578 0 0.0
RAM 203736 203736 0 0.0
ota-provider-app debug unknown 4600 4600 0 0.0
FLASH 4424052 4424052 0 0.0
RAM 197720 197720 0 0.0
ota-requestor-app debug unknown 4536 4536 0 0.0
FLASH 4562670 4562670 0 0.0
RAM 202256 202256 0 0.0
shell debug unknown 4176 4176 0 0.0
FLASH 3076765 3076765 0 0.0
RAM 159200 159200 0 0.0
thermostat-no-ble arm64 unknown 9352 9352 0 0.0
FLASH 4364324 4364324 0 0.0
RAM 242416 242416 0 0.0
tv-app debug unknown 5496 5496 0 0.0
FLASH 6007077 6007077 0 0.0
RAM 582968 582968 0 0.0
tv-casting-app debug unknown 5168 5168 0 0.0
FLASH 10692781 10692781 0 0.0
RAM 644280 644280 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 909200 909200 0 0.0
RAM 142207 142207 0 0.0
nrf7002dk_nrf5340_cpuapp FLASH 881060 881060 0 0.0
RAM 140346 140346 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 FLASH 841036 841036 0 0.0
RAM 141026 141026 0 0.0
nxp contact k32w0+release FLASH 577580 577580 0 0.0
RAM 70444 70444 0 0.0
k32w1+release FLASH 590208 590208 0 0.0
RAM 62488 62488 0 0.0
light k32w0+release FLASH 613008 613008 0 0.0
RAM 69916 69916 0 0.0
k32w1+release FLASH 674880 674880 0 0.0
RAM 71472 71472 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1639340 1639340 0 0.0
RAM 210904 210904 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1544108 1544108 0 0.0
RAM 207624 207624 0 0.0
light cy8ckit_062s2_43012 FLASH 1464028 1464028 0 0.0
RAM 200728 200728 0 0.0
lock cy8ckit_062s2_43012 FLASH 1463236 1463236 0 0.0
RAM 225064 225064 0 0.0
qpg lighting-app qpg6105+debug FLASH 656116 656116 0 0.0
RAM 105128 105128 0 0.0
lock-app qpg6105+debug FLASH 613992 613992 0 0.0
RAM 99612 99612 0 0.0
stm32 light STM32WB5MM-DK FLASH 478472 478472 0 0.0
RAM 144716 144716 0 0.0
telink air-quality-sensor-app tlsr9528a_retention FLASH 620332 620332 0 0.0
RAM 50932 50932 0 0.0
all-clusters-app tlsr9118bdk40d FLASH 680748 680748 0 0.0
RAM 149544 149544 0 0.0
all-clusters-minimal-app tlsr9528a FLASH 774102 774102 0 0.0
RAM 110704 110704 0 0.0
bridge-app tlsr9258a FLASH 679010 679010 0 0.0
RAM 91588 91588 0 0.0
contact-sensor-app tlsr9528a_retention FLASH 621928 621928 0 0.0
RAM 50972 50972 0 0.0
light-switch-app-ota-shell-factory-data tlsr9528a FLASH 707812 707812 0 0.0
RAM 74312 74312 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 620758 620758 0 0.0
RAM 145532 145532 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 806930 806930 0 0.0
RAM 99456 99456 0 0.0
lock-app-dfu tlsr9528a FLASH 654902 654902 0 0.0
RAM 67008 67008 0 0.0
ota-requestor-app tlsr9258a FLASH 697026 697026 0 0.0
RAM 91272 91272 0 0.0
pump-app-usb tlsr9518adk80d FLASH 631318 631318 0 0.0
RAM 55752 55752 0 0.0
pump-controller-app tlsr9518adk80d FLASH 608828 608828 0 0.0
RAM 52996 52996 0 0.0
shell tlsr9518adk80d FLASH 466810 466810 0 0.0
RAM 68660 68660 0 0.0
smoke_co_alarm-app tlsr9528a_retention FLASH 628716 628716 0 0.0
RAM 52692 52692 0 0.0
temperature-measurement-app-mars-ota tlsr9518adk80d FLASH 652698 652698 0 0.0
RAM 56632 56632 0 0.0
thermostat tlsr9518adk80d FLASH 633882 633882 0 0.0
RAM 53392 53392 0 0.0
window-covering tlsr9118bdk40d FLASH 522774 522774 0 0.0
RAM 98532 98532 0 0.0
tizen all-clusters-app arm unknown 4808 4808 0 0.0
FLASH 1701400 1701400 0 0.0
RAM 89300 89300 0 0.0
chip-tool-ubsan arm unknown 10236 10236 0 0.0
FLASH 17308598 17308598 0 0.0
RAM 7561280 7561280 0 0.0
tehampson
tehampson approved these changes on 2024-08-16
cecille
cecille commented on 2024-08-16
Conversation is marked as resolved
Show resolved
scripts/py_matter_yamltests/matter_yamltests/runner.py
163163 if config.auto_start_stop:
164164 await self.start()
165 status = await asyncio.wait_for(self._run(parser, config), parser.timeout)
165
 task = self._run(parser, config)
cecille247 days ago

Does a similar change need to happen in the python matter_testing_support file?

andy31415247 days ago

Yes, the files should be 1:1 ... will update (was not aware they are there already)

andy31415247 days ago

actually re-reading I see that is in wait_for but not about the copy of the file that @vatsalghelani-csa is working on (which should contain this change).

For matter_testing_support we may need this however we need to see where. I don't think we should just add shield everywhere without knowing what the sideffects are. Here this seems specific to a websocket runner.

saurabhst
saurabhst approved these changes on 2024-08-16
pullapprove pullapprove removed review - pending
pullapprove pullapprove added review - approved
mergify mergify merged 44456d77 into master 247 days ago

Login to write a write a comment.

Login via GitHub

Reviewers
Assignees
No one assigned
Labels
scripts review - approved
Milestone
No milestone
Terms Of Use Privacy Policy Refund Policy Impressum