[CP-stable]On iOS 27+ devices, manually process lldb stops - #192810
flutteractionsbot wants to merge 1 commit into
Conversation
On iOS 27+ devices, `lldb` sometimes doesn't stop on breakpoints when auto-continue is enabled. To workaround this, we disable auto-continue and manually process stops. If a stop is caused by a breakpoint, we tell the process to continue. Otherwise, we print the backtrace and detach. Fixes flutter#190307. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
|
@vashworth please fill out the PR description above, afterwards the release team will review this request. |
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
github-actions
Bot
added
platform-ios
![gemini-code-assist[bot]](/web-api/iframe-proxy?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a workaround for iOS devices failing to stop at the JIT breakpoint in debug mode by adding conditional manual stop-handling logic to the LLDB attachment process based on the device's SDK version. The review feedback highlights a critical typo in the version check (using iOS version 27 instead of 17) that would prevent the workaround from executing on current devices. Additionally, the feedback identifies a potential race condition in the manual continue logic that could discard application logs, and suggests resetting the stop-tracking state immediately upon sending the continue command, along with updating the associated tests and comments.
| if (mode == BuildMode.debug && | ||
| _deviceVersion != null && | ||
| _deviceVersion >= Version(27, 0, 0)) { | ||
| manualContinue = true; | ||
| } |
There was a problem hiding this comment.
The version check uses Version(27, 0, 0), which appears to be a typo for Version(17, 0, 0). iOS 17 introduced CoreDevice and devicectl, which is the version where this JIT breakpoint issue and the need for manual continue occurs. Using 27.0.0 will make this workaround dead code on all current iOS devices.
if (mode == BuildMode.debug &&
_deviceVersion != null &&
_deviceVersion >= Version(17, 0, 0)) {
manualContinue = true;
}| if (line.contains(_targetStoppedPattern)) { | ||
| if (stopLogs.any((log) => log.contains('stop reason = breakpoint'))) { | ||
| await _lldbProcess?.stdinWriteln(_processContinueCommand); | ||
| } else { | ||
| stopLogs.forEach(printLine); | ||
| await _lldbProcess?.stdinWriteln(_threadBacktraceAllCommand); | ||
| await _lldbProcess?.stdinWriteln(_detachCommand); | ||
| processIsStoppedAfterAttaching = false; | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
When manually continuing after hitting a breakpoint, processIsStoppedAfterAttaching is not reset to false and stopLogs is not cleared immediately. Instead, the code waits for the _lldbProcessResuming log. This creates a race condition: any application logs printed by the target immediately after resuming but before the _lldbProcessResuming log is processed will be added to stopLogs and permanently swallowed/discarded. Resetting the state and clearing stopLogs immediately when sending the continue command prevents this log loss.
if (line.contains(_targetStoppedPattern)) {
if (stopLogs.any((log) => log.contains('stop reason = breakpoint'))) {
await _lldbProcess?.stdinWriteln(_processContinueCommand);
processIsStoppedAfterAttaching = false;
stopLogs.clear();
} else {
stopLogs.forEach(printLine);
await _lldbProcess?.stdinWriteln(_threadBacktraceAllCommand);
await _lldbProcess?.stdinWriteln(_detachCommand);
processIsStoppedAfterAttaching = false;
}
return;
}| // iOS 27+ devices sometimes fail to stop at the JIT breakpoint (only used in debug mode) | ||
| // when `auto-continue` is enabled. This causes the app to crash. To workaround this, we | ||
| // manually listen for stops and continue the process if the stop is due to the breakpoint. |
There was a problem hiding this comment.
Update the comment to refer to iOS 17+ instead of iOS 27+.
// iOS 17+ devices sometimes fail to stop at the JIT breakpoint (only used in debug mode)
// when 'auto-continue' is enabled. This causes the app to crash. To workaround this, we
// manually listen for stops and continue the process if the stop is due to the breakpoint.| logger: logger, | ||
| processUtils: processUtils, | ||
| xcodeProjectInterpreter: FakeXcodeProjectInterpreter(), | ||
| deviceVersion: Version(27, 0, 0), |
There was a problem hiding this comment.
Update the test device version to 17, 0, 0 to match the corrected iOS version check.
| deviceVersion: Version(27, 0, 0), | |
| deviceVersion: Version(17, 0, 0), |
| logger: logger, | ||
| processUtils: processUtils, | ||
| xcodeProjectInterpreter: FakeXcodeProjectInterpreter(), | ||
| deviceVersion: Version(27, 0, 0), |
There was a problem hiding this comment.
Update the test device version to 17, 0, 0 to match the corrected iOS version check.
| deviceVersion: Version(27, 0, 0), | |
| deviceVersion: Version(17, 0, 0), |
| }); | ||
| }); | ||
|
|
||
| testWithoutContext('Stops are handled manually for iOS 27+', () async { |
There was a problem hiding this comment.
Update the test description to refer to iOS 17+ instead of iOS 27+.
| testWithoutContext('Stops are handled manually for iOS 27+', () async { | |
| testWithoutContext('Stops are handled manually for iOS 17+', () async { |
2 participants
This pull request is created by automatic cherry pick workflow
Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request.
Issue Link:
What is the link to the issue this cherry-pick is addressing?
#190307
Impact Description:
What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)?
Does it impact development (ex. flutter doctor crashes when Android Studio is installed),
or the shipping of production apps (the app crashes on launch).
This information is for domain experts and release engineers to understand the consequences of saying yes or no to the cherry pick.
Debugging on iOS 27+ occasionally crashes.
Changelog Description:
Explain this cherry pick:
See best practices for examples.
< Replace with changelog description here >
[flutter/190307] When debugging on physical iOS 27 devices, app occasionally crashes.
Workaround:
Is there a workaround for this issue?
No
Risk:
What is the risk level of this cherry-pick?
Test Coverage:
Are you confident that your fix is well-tested by automated tests?
Validation Steps:
What are the steps to validate that this fix works?
Use example code a script to reproduce from #190307