You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Allocate the client and server on the heap - #3987
Allocate CClient and CServer with std::unique_ptr in main() instead of keeping them on the stack.
main()'s frame is sized for the larger of the two mutually exclusive branches, so it reserves roughly 341 KB of stack (aarch64 and x86-64 alike), of which CServer is ~336 KB and CClient ~9 KB. iOS gives the main thread a 1 MB stack, and the deep UIKit startup stack can overflow that. Stack corruption of that kind can hit anything that lives nearby, including Qt's own state.
Lifetime and destruction order are unchanged: the objects are still created and destroyed in the same scope, and call sites keep taking raw pointers via .get().
CClientSettings, CServerSettings, CClientDlg and CServerDlg are left on the stack — they are small (hundreds of bytes) and not the reason for the frame size. CClientRpc/CServerRpc are also left alone: they are already parented to pRpcServer, so a unique_ptr would double-free.
CHANGELOG: SKIP
Context: Fixes an issue?
Refs #3808 (upstream). It does not claim to close it: with the two Qt/iOS startup issues fixed separately — ann0see#341 (a fork-only PR, not yet submitted upstream) and the UIScene setup — I could not reproduce the original crash with or without this change.
Does this change need documentation? What needs to be documented and how?
No.
Status of this Pull Request
Working implementation, verified locally.
What is missing until this pull request can be merged?
It has only been built and run on an iOS simulator and needs a normal-platform check (Windows/Linux/macOS) to make sure nothing regressed in the client and server paths.
Whether the smaller dialog/settings objects should be converted too is left as a follow-up decision.
main() keeps CClient and CServer on the stack. On x86-64 and aarch64 that
reserves roughly 341 KB of stack for main(), almost all of it CServer
(~336 KB); CClient adds ~9 KB. iOS gives the main thread a 1 MB stack, and the
deep UIKit startup stack can overflow that, corrupting Qt state.
Allocate both objects with std::unique_ptr instead. Lifetime and destruction
order are unchanged, and the call sites keep using raw pointers via .get().
Refs jamulussoftware#3808
No actionable comments were generated in the recent review.
Walkthrough
main now manages the client and server instances with std::unique_ptr. Settings, RPC adapters, dialogs, and startup operations use the managed pointers. Existing conditions for RPC setup, settings initialization, and startup connections remain in place.
Changes: Managed instance ownership
Layer / File(s)
Summary
Client ownership and startup wiring src/main.cpp
The client is now managed by std::unique_ptr. Settings, the RPC adapter, the dialog, and startup connections use its pointer.
Server ownership and startup wiring src/main.cpp
The server is now managed by std::unique_ptr. The RPC adapter, settings, dialog, and headless directory configuration use its pointer.
No actionable issue is established for this ownership change. Complete the usual platform checks before merging.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title clearly summarizes the main change: allocating the client and server on the heap.
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Description check
The description covers the change, changelog decision, issue context, documentation, implementation status, remaining platform testing, and checklist. It clearly states that the change does not claim …
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.
Full details: Description check
Explanation
The description covers the change, changelog decision, issue context, documentation, implementation status, remaining platform testing, and checklist. It clearly states that the change does not claim to fix issue #3808.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Repository UI
Review profile: QUIET
Plan: Advanced
Run ID: f06040bc-ce1b-4c18-bcbd-147ebee27353
Commits
Reviewing files that changed from the base of the PR and between 19c50e5 and aab0ae8.
Files selected for processing: 1
src/main.cpp
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
This is not what blocked iOS from starting: the launch abort was the UIScene lifecycle issue addressed in #3988, and with that change alone the release tag launches and renders. This one removes a ~341 KB stack frame against iOS's 1 MB main thread stack, which is where the startup crash in #3808 was suspected.
Together with #3988 and #3989, the set is what allows an iOS release on Qt 6 (3.12.6 or 3.12.7, maintainers' call).
Built and ran with this change on an iPhone 15 Pro (iOS 27.0.1) and in the iPhone 17, iPad Pro and Apple Vision Pro simulators. It is not what blocked the iOS launch - that was the UIScene lifecycle issue addressed in #3988 - so this is not required for a 3.12.6/3.12.7 iOS build.
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
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.
Short description of changes
Allocate
CClientandCServerwithstd::unique_ptrinmain()instead of keeping them on the stack.main()'s frame is sized for the larger of the two mutually exclusive branches, so it reserves roughly 341 KB of stack (aarch64 and x86-64 alike), of whichCServeris ~336 KB andCClient~9 KB. iOS gives the main thread a 1 MB stack, and the deep UIKit startup stack can overflow that. Stack corruption of that kind can hit anything that lives nearby, including Qt's own state.Lifetime and destruction order are unchanged: the objects are still created and destroyed in the same scope, and call sites keep taking raw pointers via
.get().CClientSettings,CServerSettings,CClientDlgandCServerDlgare left on the stack — they are small (hundreds of bytes) and not the reason for the frame size.CClientRpc/CServerRpcare also left alone: they are already parented topRpcServer, so aunique_ptrwould double-free.CHANGELOG: SKIP
Context: Fixes an issue?
Refs #3808 (upstream). It does not claim to close it: with the two Qt/iOS startup issues fixed separately — ann0see#341 (a fork-only PR, not yet submitted upstream) and the UIScene setup — I could not reproduce the original crash with or without this change.
Does this change need documentation? What needs to be documented and how?
No.
Status of this Pull Request
Working implementation, verified locally.
What is missing until this pull request can be merged?
Checklist
AUTOBUILD: Please build all targets