Fix: Null mContext crash when commissioning after background/foreground#43127
Merged
mergify[bot] merged 1 commit intoproject-chip:masterfrom Feb 19, 2026
Conversation
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request addresses a critical crash that occurs during commissioning after an app background/foreground cycle. The root cause was correctly identified as a null pointer dereference on mContext due to clusters failing to re-register.
The changes introduced are robust and well-thought-out:
- The
ServerClusterInterfaceRegistry::RegisterandLazyRegisteredServerCluster::Createmethods are now idempotent, which correctly handles re-registration without causingCHIP_ERROR_DUPLICATE_KEY_IDerrors. - The
DefaultServerCluster::Startupmethod is also made idempotent, allowing clusters to be re-initialized without being fully destroyed, preserving important state likemDataVersion. - The shutdown logic in
CodeDrivenDataModelProvider::Shutdownhas been correctly modified to only clear cluster state (mContext) without unregistering the clusters, which is essential for the warm-start lifecycle. - The changes in the tv-casting app example demonstrate a more robust start/stop sequence for the application lifecycle.
- A new unit test,
WarmStartAfterShutdown, has been added to specifically validate the fix for the background/foreground scenario, and existing tests have been updated to reflect the new idempotent behavior. - Defensive null checks have been added in
GeneralCommissioningClusterto prevent similar crashes in the future.
Overall, this is an excellent fix that addresses the issue at its core and improves the robustness of the application lifecycle management. The changes are consistent, well-commented, and well-tested.
pgregorr-amazon
force-pushed
the
ios_background_foreground_2026_02_11
branch
2 times, most recently
from
f3ff9d5 to
8bdfa18
Compare
|
PR #43127: Size comparison from 07f6efb to 8bdfa18 Full report (34 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nxp, psoc6, qpg, realtek, stm32, telink)
|
pgregorr-amazon
force-pushed
the
ios_background_foreground_2026_02_11
branch
from
8bdfa18 to
4437009
Compare
|
PR #43127: Size comparison from 58568d5 to 4437009 Full report (21 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, nrfconnect, nxp, psoc6, realtek, stm32)
|
pgregorr-amazon
force-pushed
the
ios_background_foreground_2026_02_11
branch
from
4437009 to
5f6da83
Compare
|
PR #43127: Size comparison from 58568d5 to 5f6da83 Full report (22 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, nrfconnect, psoc6, qpg, realtek, stm32)
|
pgregorr-amazon
force-pushed
the
ios_background_foreground_2026_02_11
branch
from
5f6da83 to
3fc953f
Compare
|
PR #43127: Size comparison from 58568d5 to 3fc953f Increases above 0.2%:
Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
chrisdecenzo
approved these changes
Feb 15, 2026
pgregorr-amazon
force-pushed
the
ios_background_foreground_2026_02_11
branch
from
3fc953f to
1bbe14f
Compare
|
PR #43127: Size comparison from 7636760 to 1bbe14f Increases above 0.2%:
Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #43127: Size comparison from 7636760 to 22a0a5a Increases above 0.2%:
Full report (31 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #43127: Size comparison from 7636760 to 133879d Increases above 0.2%:
Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #43127: Size comparison from 7636760 to c912e28 Increases above 0.2%:
Full report (27 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32)
|
|
PR #43127: Size comparison from 7636760 to d9699fc Increases above 0.2%:
Full report (27 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32)
|
|
PR #43127: Size comparison from 7636760 to 5acf3f6 Increases above 0.2%:
Full report (34 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
After backgrounding/foregrounding, commissioning crashes with null mContext. Root cause: - Background: ClearContext() sets mContext = null - Foreground: Register() fails with CHIP_ERROR_DUPLICATE_KEY_ID - SetContext() blocked, mContext stays null - Commission attempt accesses null mContext → crash Solution: - Make Register() and Shutdown() idempotent for app lifecycle - Unregister clusters in Shutdown() to clean state - SetContext() now succeeds, restoring mContext - Add 2-second delay in test framework after SIGTERM for socket cleanup Testing: Validated on iOS examples/tv-casting-app app and all unit tests pass. Test Fix: TC_TLSCERT_2_12 was failing because idempotent shutdown makes cleanup so fast that kernel doesn't finish releasing TCP sockets before next test starts. Added brief delay to allow proper socket cleanup. Applies to all platforms with app lifecycle (iOS, Android, tvOS, etc.) Apply restyle formatting (whitespace & clang-format)
pgregorr-amazon
force-pushed
the
ios_background_foreground_2026_02_11
branch
from
5acf3f6 to
b8423a7
Compare
|
PR #43127: Size comparison from be18b91 to b8423a7 Increases above 0.2%:
Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
andy31415
approved these changes
Feb 19, 2026
Contributor
andy31415
left a comment
andy31415
left a comment
There was a problem hiding this comment.
Consensur review checkmark: authored and reviewed by media maintainers.
pgregorr-amazon
added a commit
to pgregorr-amazon/connectedhomeip
that referenced
this pull request
Feb 24, 2026
…nd (project-chip#43127) After backgrounding/foregrounding, commissioning crashes with null mContext. Root cause: - Background: ClearContext() sets mContext = null - Foreground: Register() fails with CHIP_ERROR_DUPLICATE_KEY_ID - SetContext() blocked, mContext stays null - Commission attempt accesses null mContext → crash Solution: - Make Register() and Shutdown() idempotent for app lifecycle - Unregister clusters in Shutdown() to clean state - SetContext() now succeeds, restoring mContext - Add 2-second delay in test framework after SIGTERM for socket cleanup Testing: Validated on iOS examples/tv-casting-app app and all unit tests pass. Test Fix: TC_TLSCERT_2_12 was failing because idempotent shutdown makes cleanup so fast that kernel doesn't finish releasing TCP sockets before next test starts. Added brief delay to allow proper socket cleanup. Applies to all platforms with app lifecycle (iOS, Android, tvOS, etc.) Apply restyle formatting (whitespace & clang-format) (cherry picked from commit ead8174)
5 tasks
pgregorr-amazon
added a commit
to pgregorr-amazon/connectedhomeip
that referenced
this pull request
Feb 24, 2026
…nd (project-chip#43127) After backgrounding/foregrounding, commissioning crashes with null mContext. Root cause: - Background: ClearContext() sets mContext = null - Foreground: Register() fails with CHIP_ERROR_DUPLICATE_KEY_ID - SetContext() blocked, mContext stays null - Commission attempt accesses null mContext → crash Solution: - Make Register() and Shutdown() idempotent for app lifecycle - Unregister clusters in Shutdown() to clean state - SetContext() now succeeds, restoring mContext - Add 2-second delay in test framework after SIGTERM for socket cleanup Testing: Validated on iOS examples/tv-casting-app app and all unit tests pass. Test Fix: TC_TLSCERT_2_12 was failing because idempotent shutdown makes cleanup so fast that kernel doesn't finish releasing TCP sockets before next test starts. Added brief delay to allow proper socket cleanup. Applies to all platforms with app lifecycle (iOS, Android, tvOS, etc.) Apply restyle formatting (whitespace & clang-format) (cherry picked from commit ead8174)
pgregorr-amazon
added a commit
to pgregorr-amazon/connectedhomeip
that referenced
this pull request
Feb 24, 2026
…nd (project-chip#43127) After backgrounding/foregrounding, commissioning crashes with null mContext. Root cause: - Background: ClearContext() sets mContext = null - Foreground: Register() fails with CHIP_ERROR_DUPLICATE_KEY_ID - SetContext() blocked, mContext stays null - Commission attempt accesses null mContext → crash Solution: - Make Register() and Shutdown() idempotent for app lifecycle - Unregister clusters in Shutdown() to clean state - SetContext() now succeeds, restoring mContext - Add 2-second delay in test framework after SIGTERM for socket cleanup Testing: Validated on iOS examples/tv-casting-app app and all unit tests pass. Test Fix: TC_TLSCERT_2_12 was failing because idempotent shutdown makes cleanup so fast that kernel doesn't finish releasing TCP sockets before next test starts. Added brief delay to allow proper socket cleanup. Applies to all platforms with app lifecycle (iOS, Android, tvOS, etc.) Apply restyle formatting (whitespace & clang-format) (cherry picked from commit ead8174)
pgregorr-amazon
added a commit
to pgregorr-amazon/connectedhomeip
that referenced
this pull request
Feb 24, 2026
…nd (project-chip#43127) After backgrounding/foregrounding, commissioning crashes with null mContext. Root cause: - Background: ClearContext() sets mContext = null - Foreground: Register() fails with CHIP_ERROR_DUPLICATE_KEY_ID - SetContext() blocked, mContext stays null - Commission attempt accesses null mContext → crash Solution: - Make Register() and Shutdown() idempotent for app lifecycle - Unregister clusters in Shutdown() to clean state - SetContext() now succeeds, restoring mContext - Add 2-second delay in test framework after SIGTERM for socket cleanup Testing: Validated on iOS examples/tv-casting-app app and all unit tests pass. Test Fix: TC_TLSCERT_2_12 was failing because idempotent shutdown makes cleanup so fast that kernel doesn't finish releasing TCP sockets before next test starts. Added brief delay to allow proper socket cleanup. Applies to all platforms with app lifecycle (iOS, Android, tvOS, etc.) Apply restyle formatting (whitespace & clang-format) (cherry picked from commit ead8174)
andy31415
pushed a commit
that referenced
this pull request
Feb 26, 2026
…nd (#43127) (#43299) After backgrounding/foregrounding, commissioning crashes with null mContext. Root cause: - Background: ClearContext() sets mContext = null - Foreground: Register() fails with CHIP_ERROR_DUPLICATE_KEY_ID - SetContext() blocked, mContext stays null - Commission attempt accesses null mContext → crash Solution: - Make Register() and Shutdown() idempotent for app lifecycle - Unregister clusters in Shutdown() to clean state - SetContext() now succeeds, restoring mContext - Add 2-second delay in test framework after SIGTERM for socket cleanup Testing: Validated on iOS examples/tv-casting-app app and all unit tests pass. Test Fix: TC_TLSCERT_2_12 was failing because idempotent shutdown makes cleanup so fast that kernel doesn't finish releasing TCP sockets before next test starts. Added brief delay to allow proper socket cleanup. Applies to all platforms with app lifecycle (iOS, Android, tvOS, etc.) Apply restyle formatting (whitespace & clang-format) (cherry picked from commit ead8174)
andy31415
reviewed
Mar 3, 2026
| void DefaultServerCluster::Shutdown(ClusterShutdownType) | ||
| { | ||
| mContext = nullptr; | ||
| // Make shutdown idempotent - safe to call multiple times |
Contributor
There was a problem hiding this comment.
It was safe before though ... you can set mContext to nullptr multiple times. This seems like trying to preserve mContext non-null if mShutdown is called.
This feels off.
andy31415
reviewed
Mar 3, 2026
| protected: | ||
| const ConcreteClusterPath mPath; | ||
| ServerClusterContext * mContext = nullptr; | ||
| // Tracks if shutdown has been called to make it idempotent |
Contributor
There was a problem hiding this comment.
This comment does not make things clear.
andy31415
reviewed
Mar 3, 2026
Contributor
andy31415
left a comment
andy31415
left a comment
There was a problem hiding this comment.
@pgregorr-amazon I thought I originally reviewed changes to casting apps, but now I see changes to clusters and DefaultServerCluster which I am not clear about. it seems to not enforce Startup/Shutdown symmetry which feels off.
andy31415
reviewed
Mar 3, 2026
| { | ||
| VerifyOrReturnError(mContext == nullptr, CHIP_ERROR_ALREADY_INITIALIZED); | ||
| // Reset shutdown state to allow restart after shutdown | ||
| mIsShutdown = false; |
Contributor
There was a problem hiding this comment.
Is this not equivalent to using/checking mContext being null or not?
We should probably focus on mDataVersion updates as the differentiating factor.
At the same time, why do we allow startup called while startup was already called? The original API contract was that startup/shutdown are paired and documentation below seems to say the same.
andy31415
reviewed
Mar 3, 2026
| Span<const ConcreteClusterPath> paths = entry.serverClusterInterface->GetPaths(); | ||
| VerifyOrReturnError(!paths.empty(), CHIP_ERROR_INVALID_ARGUMENT); | ||
|
|
||
| // Check early if this cluster is already registered (idempotent case) |
Contributor
There was a problem hiding this comment.
I had missed this change in the original review ... the intent for registration was to disallow double-registration.
andy31415
reviewed
Mar 3, 2026
| // Double-checking for duplicates makes the checks O(n^2) on the total number of registered | ||
| // items. We preserve this however we may want to make this optional at some point in time. | ||
| VerifyOrReturnError(Get(path) == nullptr, CHIP_ERROR_DUPLICATE_KEY_ID); | ||
| // A different cluster is already registered for this path |
Contributor
There was a problem hiding this comment.
This seems equivalent to the previous code that did VerifyOrReturnError ... could we have made the delta smaller?
andy31415
reviewed
Mar 3, 2026
| // Handle idempotent Create() - if already constructed, this is a no-op. | ||
| // The cluster remains registered and functional. This supports the Stop() → Start() lifecycle | ||
| // where clusters are shutdown but remain constructed. | ||
| if (IsConstructed()) |
Contributor
There was a problem hiding this comment.
We do not actually wanted this .... sorry for the review, my original assumption was that we only changed tv casting code.
andy31415
added a commit
to andy31415/connectedhomeip
that referenced
this pull request
Mar 3, 2026
…foreground (project-chip#43127)" This reverts commit ead8174.
Sarthak-Shaha
pushed a commit
to Sarthak-Shaha/connectedhomeip_silabs
that referenced
this pull request
Mar 5, 2026
…foreground (project-chip#43127)" (project-chip#43416) This reverts commit ead8174.
4 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary
Fix null mContext crash when commissioning after app background/foreground cycle.
After backgrounding and foregrounding (e.g., switching to Photos app, locking device for 30+ seconds), commissioning crashes with null pointer dereference on mContext. Root cause: CHIP_ERROR_DUPLICATE_KEY_ID during cluster re-registration blocks SetContext() from completing, leaving mContext null. When commission attempt accesses mContext, app crashes.
Solution:
Architecture: Aligns registry state with cluster persistence - clusters remain registered across background/foreground cycles, only mContext cycles (cleared on background, restored on foreground).
Impact:
Attached Documentation:
0_Repro_Steps.mdfor detailed reproduction stepsTest Infrastructure Fix:
TC_TLSCERT_2_12 was failing due to socket cleanup timing. The idempotent shutdown optimization makes app termination so efficient that processes exit before the kernel completes TCP socket cleanup. This leaves ports actively bound (not in TIME_WAIT), causing "Address already in use" errors when the next test tries to bind 17 seconds later.
Investigation findings:
Fix: Added 2-second delay after SIGTERM in test framework to allow kernel time to complete socket cleanup. This is test-infrastructure specific and doesn't affect production behavior.
Related issues
Fixes iOS Matter Casting app crash after backgrounding/foregrounding.
Testing
Unit Tests:
Manual Testing:
Log Evidence:
Files Modified:
Readability checklist
The checklist below will help the reviewer finish PR review in time and keep the
code readable:
descriptive
"When in Rome…"
rule (coding style)
See: Pull Request Guidelines
0_Repro_Steps.md
1_iOS logs withOUT fix - Background Lock Unlock Foreground Connect.log.filterline.log
2_iOS logs WITH fix - Background Lock Unlock Foreground Connect.log.filterline.log
3_iOS Prod logs WITH fix - Background Lock Unlock Foreground Connect.log.filterline.log