Fix handling of OperationalDeviceProxy and CHIPCluster in Darwin framework #21504
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
22cb73b to
2a5ea0f
Compare
|
PR #21504: Size comparison from c902d88 to 2a5ea0f Increases (3 builds for bl602, telink)
Decreases (1 build for esp32)
Full report (32 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
2a5ea0f to
2b77009
Compare
|
PR #21504: Size comparison from 488c9b7 to 2b77009 Increases (4 builds for bl602, cc13x2_26x2, telink)
Decreases (3 builds for cc13x2_26x2, esp32)
Full report (32 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
2b77009 to
2b444fd
Compare
…ework. Darwin framework objects were holding on to OperationalDeviceProxy and CHIPCluster instances, but OperationalDeviceProxy can be destroyed randomly by anyone who cares to do it, and CHIPCluster is really designed as a stack-only object. This PR makes the following changes: * MTRBaseDevice now either stores a DeviceProxy (for PASE only) or a node id (for CASE). * MTRBaseCluster instances now just store a DeviceProxy. * MTRCallbackBridge will handle getting an exchange manager and session before invoking the action block, unless the action block is runnable without any remote interaction (the "read from cache" case). * Most of the MTRDevice and MTRClusters APIs that used to sync-run things on the Matter queue now do it async when doing CASE. They were changed to copy their incoming arguments to avoid the caller mutating them after the call. I believe this should fix project-chip#21430
2b444fd to
586a0bc
Compare
|
PR #21504: Size comparison from 97134c5 to 586a0bc Increases (3 builds for cc13x2_26x2, telink)
Decreases (2 builds for cc13x2_26x2, esp32)
Full report (43 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
github-actions bot
pushed a commit
that referenced
this pull request
Aug 2, 2022
…ework. (#21504) Darwin framework objects were holding on to OperationalDeviceProxy and CHIPCluster instances, but OperationalDeviceProxy can be destroyed randomly by anyone who cares to do it, and CHIPCluster is really designed as a stack-only object. This PR makes the following changes: * MTRBaseDevice now either stores a DeviceProxy (for PASE only) or a node id (for CASE). * MTRBaseCluster instances now just store a DeviceProxy. * MTRCallbackBridge will handle getting an exchange manager and session before invoking the action block, unless the action block is runnable without any remote interaction (the "read from cache" case). * Most of the MTRDevice and MTRClusters APIs that used to sync-run things on the Matter queue now do it async when doing CASE. They were changed to copy their incoming arguments to avoid the caller mutating them after the call. I believe this should fix #21430
woody-apple
added a commit
that referenced
this pull request
Aug 2, 2022
…ework. (#21504) (#21528) Darwin framework objects were holding on to OperationalDeviceProxy and CHIPCluster instances, but OperationalDeviceProxy can be destroyed randomly by anyone who cares to do it, and CHIPCluster is really designed as a stack-only object. This PR makes the following changes: * MTRBaseDevice now either stores a DeviceProxy (for PASE only) or a node id (for CASE). * MTRBaseCluster instances now just store a DeviceProxy. * MTRCallbackBridge will handle getting an exchange manager and session before invoking the action block, unless the action block is runnable without any remote interaction (the "read from cache" case). * Most of the MTRDevice and MTRClusters APIs that used to sync-run things on the Matter queue now do it async when doing CASE. They were changed to copy their incoming arguments to avoid the caller mutating them after the call. I believe this should fix #21430 Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
isiu-apple
pushed a commit
to isiu-apple/connectedhomeip
that referenced
this pull request
Sep 16, 2022
…ework. (project-chip#21504) Darwin framework objects were holding on to OperationalDeviceProxy and CHIPCluster instances, but OperationalDeviceProxy can be destroyed randomly by anyone who cares to do it, and CHIPCluster is really designed as a stack-only object. This PR makes the following changes: * MTRBaseDevice now either stores a DeviceProxy (for PASE only) or a node id (for CASE). * MTRBaseCluster instances now just store a DeviceProxy. * MTRCallbackBridge will handle getting an exchange manager and session before invoking the action block, unless the action block is runnable without any remote interaction (the "read from cache" case). * Most of the MTRDevice and MTRClusters APIs that used to sync-run things on the Matter queue now do it async when doing CASE. They were changed to copy their incoming arguments to avoid the caller mutating them after the call. I believe this should fix project-chip#21430
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Darwin framework objects were holding on to OperationalDeviceProxy and CHIPCluster instances, but OperationalDeviceProxy can be destroyed randomly by anyone who cares to do it, and CHIPCluster is really designed as a stack-only object.
This PR makes the following changes:
MTRBaseDevice now either stores a DeviceProxy (for PASE only) or a node id
(for CASE).
MTRBaseCluster instances now just store a DeviceProxy.
MTRCallbackBridge will handle getting an exchange manager and session before
invoking the action block, unless the action block is runnable without any
remote interaction (the "read from cache" case).
Most of the MTRDevice and MTRClusters APIs that used to sync-run things on the
Matter queue now do it async when doing CASE. They were changed to copy their
incoming arguments to avoid the caller mutating them after the call.
I believe this should fix #21430
Problem
Crashes, lifetime issues.
Change overview
See above.
Testing
Should pass all existing CI; no behavior changes in normal use, just making things saner for other work and improving error handling.