fix(notifications): stop macOS sendNotification pinning a core#486
Conversation
There was a problem hiding this comment.
Code Review
This pull request vendors the mac-notification-sys crate to apply a local patch that resolves a busy-spin issue on background threads when waiting for notification clicks. The patch introduces a dummy NSMachPort to the run loop in objc/notify.m to allow the thread to sleep. However, the review feedback correctly points out that the newly created NSMachPort is not invalidated before the function exits, which can leak Mach ports and eventually exhaust system resources. It is recommended to call [wakePort invalidate] before exiting.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
I'm not a heavy script user, so I hadn't hit this issue myself. Thanks for catching it and fixing it — merging as is. |
I was worried I'd broken something, as my macbook was running very hot. Asyar was sat on ~200% CPU with the main thread idle...
Sampling showed two threads stuck in mac-notification-sys's sendNotification wait loop, one per un-clicked notification.
I had been testing out running scripts, each run that finishes or fails sends a "Script finished"/"Run failed" notification with an action button, which I hadn't clicked.
The crate polls runUntilDate: while it waits for the click, but send() runs on a bare std::thread with no input sources on its run loop, so runUntilDate: returns immediately and the wait spins a full core until the notification is clicked or the app restarts. It's set up before delivery too, so it spins even if the notification never actually shows.
This vendors 0.6.12 via [patch.crates-io] with one fix in objc/notify.m: park a dummy NSMachPort on the run loop so each 0.1s poll actually sleeps. Delegate callbacks land on the main thread and get picked up on the next poll, so click handling is unchanged. VENDORED.md has the details.
Longer-term, moving to UNUserNotificationCenter with one persistent delegate would also fix the crate's shared-delegate race between concurrent sends.
Test: