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

SubscriptionCallback::ReportData needs to capture its callbacks in block vars. #21453

Closed
bzbarsky-apple opened this issue Jul 29, 2022 · 2 comments · Fixed by #21456
Closed

Comments

@bzbarsky-apple
Copy link
Contributor

Problem

SubscriptionCallback::ReportData (in MTRBaseDevice.mm) does this:

    if (mAttributeReportCallback && attributeReports.count) {
        dispatch_async(mQueue, ^{
            mAttributeReportCallback(attributeReports);

but by the time the async block runs, mAttributeReportCallback can have become null, right? And in general, this is doing racy access to the callback....

We have crashes on that call to mAttributeReportCallback that I bet are due to this.

Proposed Solution

The callback should be getting captured into a block var, like the attributeReports are.

@bzbarsky-apple bzbarsky-apple self-assigned this Jul 29, 2022
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Jul 30, 2022
We were null-checking members, then calling them from an async block,
by which point they might be null.

Fixes project-chip#21453
woody-apple pushed a commit that referenced this issue Aug 1, 2022
We were null-checking members, then calling them from an async block,
by which point they might be null.

Fixes #21453
github-actions bot pushed a commit that referenced this issue Aug 1, 2022
We were null-checking members, then calling them from an async block,
by which point they might be null.

Fixes #21453
woody-apple added a commit that referenced this issue Aug 2, 2022
We were null-checking members, then calling them from an async block,
by which point they might be null.

Fixes #21453

Co-authored-by: Boris Zbarsky <[email protected]>
@aajain-com
Copy link
Contributor

@bzbarsky-apple @woody-apple this seems very similar to #21142 that I filed given that SubscriptionCallback::OnReportEnd ends up calling ReportData() as per the code. WDYT?

@bzbarsky-apple
Copy link
Contributor Author

@aajain-com Yes, I expect #21142 is the same issue, with mEventReportCallback having become null.

isiu-apple pushed a commit to isiu-apple/connectedhomeip that referenced this issue Sep 16, 2022
We were null-checking members, then calling them from an async block,
by which point they might be null.

Fixes project-chip#21453
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants