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

[Platform] On Darwin, stop using PlatformMgrImpl().GetWorkQueue() directly as much as possible #22847

Closed
bzbarsky-apple opened this issue Sep 23, 2022 · 1 comment · Fixed by #23859
Closed

[Platform] On Darwin, stop using PlatformMgrImpl().GetWorkQueue() directly as much as possible #22847

bzbarsky-apple opened this issue Sep 23, 2022 · 1 comment · Fixed by #23859
Assignees
@bzbarsky-apple
Labels
darwin v1.1

Comments

@bzbarsky-apple
Copy link
Contributor

bzbarsky-apple commented Sep 23, 2022 •
edited
Loading

Reproduction steps

We should be going through a controller and checking isRunning; otherwise we have no idea whether the queue is even enabled.

Platform

darwin

Platform Version(s)

No response

Type

Manually tested with SDK

(Optional) If manually tested please explain why this is only manually tested

No response

Anything else?

No response
@bzbarsky-apple bzbarsky-apple self-assigned this Sep 23, 2022
@bzbarsky-apple bzbarsky-apple mentioned this issue Oct 3, 2022
Closed
@bzbarsky-apple
Copy link
Contributor Author

bzbarsky-apple commented Oct 3, 2022 •
edited
Loading

Places where we currently have possibly-unsafe uses:

  • OTA provider bits. This might get fixed in [Platform] Apply API review comments to MTROTAProviderDelegate #22541.
  • PurgeReadClientContainers in MTRBaseDevice.mm. Might be able to track the MTRBaseDevice and hence the controller in the MTRReadClientContainer to use that to make things safe.
  • CauseReadClientFailure in MTRBaseDevice.mm (might be OK because unit-test-only).
  • The readAttribute*WithClusterStateCache APIs on MTRBaseClusters (indirectly, via the callback bridge constructor it uses). We might be able to use the controller of the MTRBaseDevice to make these safe.
  • readAttributePathWithEndpointID on MTRClusterStateCacheContainer. We might be able to make this safe by having it store the controller or MTRBaseDevice it's associated with.

@jtung-apple @kpark-apple

bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Dec 1, 2022
@bzbarsky-apple
Make sure we don't use the Matter dispatch queue when it's not running.
434eb4e 
We've had issues where things get queued to the Matter dispatch queue while it's
not running, and then when we start it up again those things run too early in
startup and break in interesting ways.

The fix is to make sure we only queue things to the Matter dispatch queue when
they're associated with a currently-running controller.

Fixes project-chip#22847
@bzbarsky-apple bzbarsky-apple mentioned this issue Dec 1, 2022
Merged
yufengwangca pushed a commit that referenced this issue Dec 1, 2022
@bzbarsky-apple
Make sure we don't use the Matter dispatch queue when it's not runnin…
efbb120 
…g. (#23859)

We've had issues where things get queued to the Matter dispatch queue while it's
not running, and then when we start it up again those things run too early in
startup and break in interesting ways.

The fix is to make sure we only queue things to the Matter dispatch queue when
they're associated with a currently-running controller.

Fixes #22847
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bzbarsky-apple bzbarsky-apple

Labels
darwin v1.1
Projects
None yet
Milestone
No milestone
1 participant
@bzbarsky-apple