Commit 3fe6c21
committed
[MTRCommissioning] Add watchdog for client failure to advance past PASE
If a client (CHIP framework consumer) implements
commissioning:paseSessionEstablishmentComplete: but then never calls
commissionNodeWithID: or cancelCommissioningForNodeID:, the
MTRCommissioningOperation sits in isWaitingAfterPASEEstablished = YES
forever. MTRDeviceController_Concrete keeps the operation alive via
currentInternalCommissioning, so every subsequent commissioning attempt
hits the busy check in startWithController and fails with
CHIP_ERROR_BUSY. User-visible symptom: Matter accessories refuse to
commission until the daemon (or app) is restarted.
Add a 5-minute dispatch_source_t watchdog armed when
isWaitingAfterPASEEstablished is set to YES, cancelled on legitimate
advance, stop, or terminal error. On expiry, calls
stopCommissioning:forCommissioningID: and routes CHIP_ERROR_TIMEOUT
through the standard _dispatchCommissioningError path so client state
can settle. Clear isWaitingAfterPASEEstablished BEFORE the controller
stop call so -_cancelCommissioning does not synthesize a CANCELLED
OnCommissioningComplete -- the wedge IS that no notifications arrived,
so suppressing the synthetic CANCELLED avoids a redundant
MATTER_LOG_METRIC_END(kMetricDeviceCommissioning) and a spurious
CANCELLED on _delegateQueue ahead of the TIMEOUT we want to deliver.
Defensive secondary fix in
setupCommissioningSessionWithPayload:newNodeID:error:: if a stale
internally-created commissioning is still parked at the post-PASE
waiting state with a different setup payload, treat the new call as an
implicit cancel of the previous one rather than failing the new call
with BUSY. Perform the implicit cancel BEFORE
MATTER_LOG_METRIC_BEGIN(kMetricDeviceCommissioning) and the metrics
reset for the new attempt -- otherwise the synthetic CANCELLED end
emitted by the bridge from the stale operation would close out the
new attempt's metric, leaving it without a matching begin/end and
skewing telemetry. This also keeps the cancel-failure early return
from leaving kMetricDeviceCommissioning open across the returned NO.
Threading hardening from review:
* atomic test-only watchdog interval override so concurrent TSan setters
on any queue race-free with the production read in _armPostPASEWatchdog
* clamp non-finite (NaN / +/-INFINITY) test interval overrides to 0
(use production interval) so the multiply-by-NSEC_PER_SEC and cast
to int64_t in -_armPostPASEWatchdog stays defined behavior
* atomic stop via stopCommissioningAtomically:forCommissioningID:, closing
the TOCTOU window between currentCommissioning read and StopPairing
* synchronous _cancelPostPASEWatchdog in -stop so tests can observe the
teardown immediately after -stop returns
Honor the @Property (copy) declaration on -setupPayload by [copy]'ing
the input in the designated initializer; mutable inputs were aliased.
Regression coverage:
* test_PostPASEWatchdogIntervalResetToZeroRestoresProductionInterval --
pins that +setPostPASEWatchdogIntervalForTesting:0 actually restores
the production interval after a non-zero override.
* test_ForceArmFailureFlag_ConcurrentSettersAndArmer_NoTSan -- TSan
smoke test for the per-instance _forceNextArmFailureForTesting flag.
* test_WatchdogCancel_IsSynchronousOnSelfFire -- pins the round-2
sync-cancel contract for the watchdog's own fire path;
-_firePostPASEWatchdog must synchronously tear down _postPASEWatchdog
at the top of the handler.1 parent 8a162c6 commit 3fe6c21
7 files changed
Lines changed: 5048 additions & 40 deletions
File tree
- src/darwin/Framework
- CHIPTests
- CHIP
- Matter.xcodeproj
Large diffs are not rendered by default.
Lines changed: 18 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
41 | 49 | | |
42 | 50 | | |
43 | 51 | | |
44 | 52 | | |
45 | 53 | | |
46 | 54 | | |
47 | | - | |
48 | | - | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
49 | 65 | | |
50 | 66 | | |
51 | 67 | | |
| |||
Lines changed: 52 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
255 | 255 | | |
256 | 256 | | |
257 | 257 | | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
258 | 296 | | |
259 | 297 | | |
260 | 298 | | |
| |||
Lines changed: 96 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
146 | 146 | | |
147 | 147 | | |
148 | 148 | | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
149 | 162 | | |
150 | 163 | | |
151 | 164 | | |
| |||
205 | 218 | | |
206 | 219 | | |
207 | 220 | | |
| 221 | + | |
208 | 222 | | |
209 | 223 | | |
210 | 224 | | |
| |||
861 | 875 | | |
862 | 876 | | |
863 | 877 | | |
| 878 | + | |
| 879 | + | |
| 880 | + | |
| 881 | + | |
| 882 | + | |
| 883 | + | |
| 884 | + | |
| 885 | + | |
| 886 | + | |
| 887 | + | |
| 888 | + | |
| 889 | + | |
| 890 | + | |
| 891 | + | |
| 892 | + | |
| 893 | + | |
| 894 | + | |
| 895 | + | |
| 896 | + | |
| 897 | + | |
| 898 | + | |
| 899 | + | |
| 900 | + | |
| 901 | + | |
| 902 | + | |
| 903 | + | |
| 904 | + | |
| 905 | + | |
| 906 | + | |
| 907 | + | |
| 908 | + | |
| 909 | + | |
| 910 | + | |
| 911 | + | |
| 912 | + | |
| 913 | + | |
| 914 | + | |
| 915 | + | |
| 916 | + | |
| 917 | + | |
| 918 | + | |
| 919 | + | |
| 920 | + | |
| 921 | + | |
| 922 | + | |
864 | 923 | | |
865 | 924 | | |
866 | 925 | | |
867 | 926 | | |
868 | 927 | | |
869 | 928 | | |
870 | | - | |
871 | | - | |
872 | | - | |
873 | | - | |
874 | | - | |
875 | | - | |
| 929 | + | |
876 | 930 | | |
877 | 931 | | |
878 | 932 | | |
| |||
1357 | 1411 | | |
1358 | 1412 | | |
1359 | 1413 | | |
| 1414 | + | |
| 1415 | + | |
| 1416 | + | |
| 1417 | + | |
| 1418 | + | |
| 1419 | + | |
| 1420 | + | |
| 1421 | + | |
| 1422 | + | |
| 1423 | + | |
| 1424 | + | |
| 1425 | + | |
| 1426 | + | |
| 1427 | + | |
| 1428 | + | |
| 1429 | + | |
| 1430 | + | |
| 1431 | + | |
| 1432 | + | |
| 1433 | + | |
| 1434 | + | |
| 1435 | + | |
| 1436 | + | |
| 1437 | + | |
| 1438 | + | |
| 1439 | + | |
| 1440 | + | |
| 1441 | + | |
| 1442 | + | |
| 1443 | + | |
| 1444 | + | |
| 1445 | + | |
| 1446 | + | |
| 1447 | + | |
| 1448 | + | |
| 1449 | + | |
1360 | 1450 | | |
1361 | 1451 | | |
1362 | 1452 | | |
| |||
0 commit comments