Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
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.
The macOS tray runs a manual AppKit event loop without draining an autorelease pool. Repeated directory updates accumulate temporary menus, images and related objects. A long-running 0.12.13 tray reached about 1 GiB of physical footprint, with 131,878
NSImageobjects and 12,042 autorelease-pool pages.This change drains a finite pool after startup and a new pool after each complete event-loop iteration. The application, tray and current menu keep their retained ownership across iterations. This follows AppKit's autorelease-pool requirements.
The new macOS regression check runs the actual tray against a private synthetic IPC directory: eight machines and 56–64 sessions, with changing connectivity, RTT, status and activity text. It measures physical footprint with
vmmap, writes JSON evidence, and checks that polling continues. Its result is explicitly memory-only; the script documents a separate submenu interaction check.Before and after
Both builds used Rust 1.99.1, the existing Cargo.lock and the debug profile on macOS 27.0.1 arm64. The baseline was built from upstream
5a6d0835868e1e648bed9144360d29455ebe8e6f. Each run had a 15-second warmup, 120 seconds of measurement and 60 measured directory polls. Menus stayed closed during measurement.A separate patched-process stack sample observed menu creation/replacement, image drawing,
objc_autoreleasePoolPop, andNSImagedeallocation. Its heap contained fourNSImageobjects and four autorelease-pool pages at the sampled point. This supports that the main thread continued processing updates, rather than merely leaving the background poller alive.Validation
cargo fmt --checkcargo clippy --all-targets --locked -- -D warningscargo test --locked: 254 unit tests and 2 install-script tests passedpython3 tests/e2e/tray_memory.py --binary <unpatched-build> --output <baseline.json>: failed for memory growthpython3 tests/e2e/tray_memory.py --output <fixed.json>: passedgit diff --checkThe unchanged upstream CI workflow passed on all three platforms with Rust 1.99.1: CI run.
-D warnings)windows-latest)The fork CI commit
7b88c2619940c7834ab0ac0f4b619d4dce7a18abhas the same Git tree as the proposed fix commit05dd37fd5c5d666f7bbc92fe1bda501fe431b4e3. Two empty commits triggered CI after enabling Actions on the new fork; they are not part of this PR branch. No workflow files were changed in the proposed fix.Manual submenu interaction remains unverified: the available app-control interface could not address the menu-bar-only process, including through a temporary app bundle. The measured two-minute regression is not an overnight soak test.