jkleinsc

#55629: fix: run local notification callbacks on the main thread on macOS

Merged
Created: Nov 5, 2026, 1:28:15 PM
Merged: Nov 6, 2026, 4:40:18 AM
5 comments
Target: main

Description of Change

Fixes #55392. Two cpp-heap.spec.ts tests only passed on retry. This PR fixes both, then turns retries off for the suite.

macOS: systemPreferences module > does not crash on exit with a live wrapper

DoSubscribeNotification registers its observer with queue:nil, so NSNotificationCenter runs the block on whichever thread posts the notification. A subscribeLocalNotification(null, ...) subscription receives every local notification, including ones posted from background threads, and the block called the JS callback right there. V8FunctionInvoker::Go takes a gin_helper::Locker, and the main thread holds the isolate's v8::Locker while JS is running. So a background thread blocks on that lock, and if the main thread waits on that thread during app.quit(), the app hangs. That matches the ~30s first-attempt timeouts in CI. Real apps can hit this too.

The JS callbacks now live in a map that only the main thread touches. Notifications posted on the main thread run the callback directly; notifications posted on other threads are sent to the main thread with PostTask, and are dropped if the subscription has been removed by the time they arrive. The block no longer holds any V8 handles. The docs now say the callback always runs on the main thread.

linux-x64-asan: native callback holders (growth and release)

spec/fixtures/api/cppgc-detached-frame.js checked its counts after synchronous GCs. Those scan the native stack conservatively, so under ASan a stale pointer on a fake stack can keep a callback holder and its realm alive, leaving a count off by one. The fixture now uses requestGarbageCollectionForTesting({ execution: 'async' }), which collects from a fresh task without scanning the stack.

Testing: on a local macOS testing build, cpp-heap.spec.ts passed 5 runs in a row with retry: 0, 90/90 tests each time. The exit test took about 530ms per run. I couldn't reproduce the ASan failure locally, so only the linux-x64-asan CI job will confirm that half.

Backports: 44-x-y and 45-x-y don't have cppgc-detached-frame.js, so those backports should only take the macOS fix and the docs note.

Checklist

  • PR description included
  • npm test passes
  • tests are changed or added
  • relevant API documentation, tutorials, and examples are updated and follow the documentation style guide
  • PR release notes describe the change in a way relevant to app developers

🤖 Generated with Claude Code

Notes: Fixed a possible hang on quit on macOS when using systemPreferences.subscribeLocalNotification, subscribeNotification, or subscribeWorkspaceNotification.

Backports

44-x-y
Pending
Waiting for a manual backport
45-x-y
Pending
Waiting for a manual backport
46-x-y
Merged
PR Number
#55646
Merged At
Nov 6, 2026, 10:09:48 AM
Released In
Not yet
Release Date
Not yet

Semver Impact

Major
Breaking changes
Minor
New features
Patch
Bug fixes
None
Docs, tests, etc.

Semantic Versioning helps users understand the impact of updates:

  • Major (X.y.z): Breaking changes that may require code modifications
  • Minor (x.Y.z): New features that maintain backward compatibility
  • Patch (x.y.Z): Bug fixes that don't change the API
  • None: Changes that don't affect using facing parts of Electron