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

Fix a race in CHIPDeviceController while shutting down #8803

Merged
merged 1 commit into from
Aug 5, 2021

Conversation

sagar-apple
Copy link
Contributor

Problem

The CHIPDeviceController can accept new jobs on its workqueue even though a "shutdown" is queued up. Because of this each Job has to make sure it checks the isRunning state of the controller before performing any tasks.

This is an unfortunate side-effect of not knowing whether or not a Shutdown has occurred since the current Job was queued up.

Change overview

Make sure to check isRunning while calling getConnectedDevice

Testing

How was this tested? (at least one bullet point required)
Since this was a race it is not straightforward to write a deterministic test.

  • If manually tested, what platforms controller and device platforms were manually tested, and how?
    Ran the Controller lifecycle tests a few 100x. It doesn't flake anymore.

@sagar-apple sagar-apple force-pushed the fix_shutdown_race_2 branch from b949556 to 1bcc01f Compare August 5, 2021 16:14
@woody-apple woody-apple merged commit e2fdaaf into project-chip:master Aug 5, 2021
@github-actions
Copy link

github-actions bot commented Aug 5, 2021

Size increase report for "esp32-example-build" from 017daa6

File Section File VM
chip-shell.elf .flash.text 20 20
chip-temperature-measurement-app.elf .flash.text -60 -60
chip-lock-app.elf .flash.text 64 64
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.flash.text,20,20
[Unmapped],0,-20

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize
[Unmapped],0,60
.flash.text,-60,-60

Comparing ./master_artifact/chip-bridge-app.elf and ./pull_artifact/chip-bridge-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
.flash.text,64,64
[Unmapped],0,-64

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize


@sagar-apple sagar-apple deleted the fix_shutdown_race_2 branch August 5, 2021 20:34
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
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.

5 participants