[darwin_framework_tool] Add a shortcut (CTL('^')) to restart the stac… #22268
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
pullapprove
bot
requested review from
anush-apple,
arkq,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
franck-apple,
gjc13,
harimau-qirex,
harsha-rajendran,
hawk248,
isiu-apple,
jelderton,
jepenven-silabs,
jmartinez-silabs,
jtung-apple,
kghost,
kpschoedel,
lazarkov,
LuDuda,
mrjerryjohns,
msandstedt,
mspang and
rgoliver
|
I suspect that #22245 should have fixed the second crash (the one with the stack trace) but I have not tried yet. |
|
| I suspect that #22245 should have fixed the second crash (the one with the stack trace) but I have not tried yet. Seems like there is still a crash, but happening a little bit later now: |
|
PR #22268: Size comparison from 9bde9e3 to 9a804b4 Increases (3 builds for cc13x2_26x2, telink)
Decreases (4 builds for bl602, cc13x2_26x2, nrfconnect, psoc6)
Full report (43 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, telink)
|
|
Accepted for 1.0: changes in |
examples/darwin-framework-tool/commands/common/CHIPCommandBridge.mm
Outdated
Show resolved
Hide resolved
examples/darwin-framework-tool/commands/common/CHIPCommandBridge.mm
Outdated
Show resolved
Hide resolved
Fwiw, just tested using the above excellent steps to reproduce, and I can reproduce the crash and can also confirm that #22282 does not fix it. |
bzbarsky-apple
added a commit
to bzbarsky-apple/connectedhomeip
that referenced
this pull request
Aug 31, 2022
Before project-chip#21256 AutoCommissioner used the operational proxy if it existed at all. This could happen even if it was disconnected, as long as it had been connected at some point in the past. This was accidentally changed to "use the operational proxy only if it's connected" in project-chip#21256. This can lead to a crash, as described in project-chip#22268 (comment), if shutdown happens after the operational proxy is connected but before we get a response to CommissioningComplete. In that case, we will evict our CASE session, which will error out the CommissioningComplete command we sent and try to clean up, but it will select the (now dangling!) mCommissioneeDeviceProxy instead of correctly selecting mOperationalDeviceProxy, because the mOperationalDeviceProxy no longer has a session at that point. The fix is to check for an "initialized" (in the sense that it has a valid peer node id) mOperationalDeviceProxy instead of checking for a connected one. This matches the semantics of the check we used to have before project-chip#21256. Fixes project-chip#22293
#22294 will fix that. |
…k while in interactive mode
vivien-apple
force-pushed
the
DarwinFrameworkTool_ShortcutRestart
branch
from
9a804b4 to
ac532d1
Compare
|
PR #22268: Size comparison from 0a36d9f to ac532d1 Increases (1 build for cc13x2_26x2)
Decreases (9 builds for bl602, cc13x2_26x2, nrfconnect, psoc6, qpg, telink)
Full report (34 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
bzbarsky-apple
approved these changes
Aug 31, 2022
bzbarsky-apple
added a commit
that referenced
this pull request
Aug 31, 2022
Before #21256 AutoCommissioner used the operational proxy if it existed at all. This could happen even if it was disconnected, as long as it had been connected at some point in the past. This was accidentally changed to "use the operational proxy only if it's connected" in #21256. This can lead to a crash, as described in #22268 (comment), if shutdown happens after the operational proxy is connected but before we get a response to CommissioningComplete. In that case, we will evict our CASE session, which will error out the CommissioningComplete command we sent and try to clean up, but it will select the (now dangling!) mCommissioneeDeviceProxy instead of correctly selecting mOperationalDeviceProxy, because the mOperationalDeviceProxy no longer has a session at that point. The fix is to check for an "initialized" (in the sense that it has a valid peer node id) mOperationalDeviceProxy instead of checking for a connected one. This matches the semantics of the check we used to have before #21256. Fixes #22293
andy31415
approved these changes
Sep 1, 2022
isiu-apple
pushed a commit
to isiu-apple/connectedhomeip
that referenced
this pull request
Sep 16, 2022
Before project-chip#21256 AutoCommissioner used the operational proxy if it existed at all. This could happen even if it was disconnected, as long as it had been connected at some point in the past. This was accidentally changed to "use the operational proxy only if it's connected" in project-chip#21256. This can lead to a crash, as described in project-chip#22268 (comment), if shutdown happens after the operational proxy is connected but before we get a response to CommissioningComplete. In that case, we will evict our CASE session, which will error out the CommissioningComplete command we sent and try to clean up, but it will select the (now dangling!) mCommissioneeDeviceProxy instead of correctly selecting mOperationalDeviceProxy, because the mOperationalDeviceProxy no longer has a session at that point. The fix is to check for an "initialized" (in the sense that it has a valid peer node id) mOperationalDeviceProxy instead of checking for a connected one. This matches the semantics of the check we used to have before project-chip#21256. Fixes project-chip#22293
isiu-apple
pushed a commit
to isiu-apple/connectedhomeip
that referenced
this pull request
Sep 16, 2022
…k while in interactive mode (project-chip#22268)
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.
…k while in interactive mode
Problem
I'm trying to reproduce some crashes that happens on darwin. Some crashes seems related to the stack trying to shutting down.
With the current patch and the following steps I can reproduce 2 crashes:
The "right time" is hard to get since operations are going very fast locally.
To make it easier to get some crashes, one can drop some
sleepat the right place on the server side.For example
sleep(5)at the beginning ofemberAfGeneralCommissioningClusterArmFailSafeCallbackshould allow to reproduce #21811.But adding
sleep(5)at the beginning ofemberAfGeneralCommissioningClusterCommissioningCompleteCallbackshould get you a different stack. Pretty similar to #21130 but not coming from a timer firing.For reference the stack is:
Change overview
darwin-framework-toolin order to restart the stack. Using a shortcut to make it fast.Testing
Aș explained in the description I have used this setup to reproduce some crashes.
I am pretty sure that those are not related to darwin, one may implement a similar mechanism for
chip-tool.I would like to land that in
masteras I believed it does not affect the core SDK but only provide another way of reproducing some SDK behaviour via a tool.