connectedhomeip icon indicating copy to clipboard operation
connectedhomeip copied to clipboard

EXC_BAD_ACCESS during `CleanupCommissioning`

Open dtodor opened this issue 2 years ago • 3 comments

The DeviceCommissioner::ReleaseCommissioneeDevice call inside DeviceCommissioner::CleanupCommissioning will set the mDeviceBeingCommissioned to a nullptr.

https://github.com/project-chip/connectedhomeip/blob/ee04eb687a88824583f1afeadd9164c8656648e6/src/controller/CHIPDeviceController.cpp#L585

Then the subsequent call to DeviceCommissioner::CommissioningStageComplete will crash with an EXC_BAD_ACCESS since the nodeId is being retrieved:

https://github.com/project-chip/connectedhomeip/blob/ee04eb687a88824583f1afeadd9164c8656648e6/src/controller/CHIPDeviceController.cpp#L1590

Here is the relevant stack trace:

* thread #3, queue = 'com.csa.matter.framework.controller.workqueue', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001001eb9b4 c1`chip::Controller::DeviceCommissioner::CommissioningStageComplete(this=0x000000010202d200, err=(mError = 0, mFile = "../../third_party/connectedhomeip/src/controller/CHIPDeviceController.cpp", mLine = 1502), report=CommissioningReport @ 0x000000016ff0e788) at CHIPDeviceController.cpp:1590:58
    frame #1: 0x00000001001eeb90 c1`chip::Controller::DeviceCommissioner::CleanupCommissioning(this=0x000000010202d200, proxy=0x0000000105808200, nodeId=1015, completionStatus=0x000000016fdfe958) at CHIPDeviceController.cpp:1502:9
    frame #2: 0x00000001001f3024 c1`chip::Controller::DeviceCommissioner::PerformCommissioningStep(this=0x000000010202d200, proxy=0x0000000105808200, step=kCleanup, params=0x000000016fdfe7c0, delegate=0x000000016fdfe798, endpoint=0, timeout=Optional<std::__1::chrono::duration<unsigned int, std::__1::ratio<1, 1000> > > @ 0x000000016ff10640) at CHIPDeviceController.cpp:2305:9
    frame #3: 0x00000001001e616c c1`chip::Controller::AutoCommissioner::PerformStep(this=0x000000016fdfe798, nextStage=kCleanup) at AutoCommissioner.cpp:572:20
    frame #4: 0x00000001001e5354 c1`chip::Controller::AutoCommissioner::CommissioningStepFinished(this=0x000000016fdfe798, err=(mError = 0, mFile = "../../controller/CustomAutoCommissioner.cpp", mLine = 37), report=CommissioningReport @ 0x000000016ff10988) at AutoCommissioner.cpp:550:12

dtodor avatar Aug 05 '22 15:08 dtodor

@dtodor Is this reproducible? If so, with what steps, on what SHA?

The reason this is supposed to work, in theory, is that in CleanupCommissioning if err is CHIP_NO_ERROR that means we commissioned successfully. That means that mDeviceBeingCommissioned is not a CommissioneeDeviceProxy (it's an operational device at that point), and hence can't be cleared by ReleaseCommissioneeDevice.

But clearly something is going wrong with this logic in your case...

bzbarsky-apple avatar Aug 06 '22 03:08 bzbarsky-apple

Thanks for the explanation, I think the problem is not related to the SDK then. We are using a custom AutoCommissioner, which probably has to be aligned with the latest changes.

dtodor avatar Aug 08 '22 08:08 dtodor

Doesn't it make more sense to have the code the other way around, e.g.

        // Send the callbacks, we're done.
        CommissioningStageComplete(CHIP_NO_ERROR);
        SendCommissioningCompleteCallbacks(nodeId, commissioningCompletionStatus);

        CommissioneeDeviceProxy * commissionee = FindCommissioneeDevice(nodeId);
        if (commissionee != nullptr)
        {
            ReleaseCommissioneeDevice(commissionee);
        }

It feels weird to clean up something that is being used later on, i.e. inside CommissioningStageComplete .

dtodor avatar Aug 26 '22 14:08 dtodor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale[bot] avatar Apr 25 '23 19:04 stale[bot]

This stale issue has been automatically closed. Thank you for your contributions.

stale[bot] avatar May 04 '23 02:05 stale[bot]