Repository navigation
Conversation
With a nil queue, NSNotificationCenter runs observer blocks on whichever thread posts the notification, and the block called the JS callback there. A background thread would block on the isolate's v8::Locker held by the main thread, which could hang app.quit(). Keep the callbacks in a main-thread-only map and post notifications from other threads to the main thread. Also make the detached-frame cppgc fixture collect with async GCs so conservative stack scanning (notably ASan fake stacks) cannot keep a stale callback holder alive, and disable retries in cpp-heap.spec.ts. Fixes #55392 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
| void SystemPreferences::DoUnsubscribeNotification(int request_id, | ||
| NotificationCenterKind kind) { | ||
| auto iter = GetIdMap().find(request_id); | ||
| if (iter != GetIdMap().end()) { | ||
| id observer = iter->second; | ||
| [GetNotificationCenter(kind) removeObserver:observer]; | ||
| GetIdMap().erase(iter); | ||
| GetKindMap().erase(request_id); | ||
| auto& subscriptions = GetSubscriptions(); | ||
| auto iter = subscriptions.find(request_id); | ||
| if (iter != subscriptions.end()) { | ||
| [GetNotificationCenter(kind) removeObserver:iter->second.observer]; | ||
| subscriptions.erase(iter); |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: pre-existing: Unsubscribing with the wrong kind's method now silently mutes the observer instead of removing it, and it is never removed afterwards. DoUnsubscribeNotification still calls removeObserver on the center for the kind argument, but erases the map entry whatever its kind. Fix: remove the observer from iter->second.kind's center (or ignore ids whose stored kind differs from kind), so the stored kind decides which center removeObserver runs on.
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
A caller passes an id from subscribeNotification to unsubscribeLocalNotification (or any other kind mismatch). DoUnsubscribeNotification (lines 334-340) calls [GetNotificationCenter(kind) removeObserver:iter->second.observer] on the wrong center, which does nothing, then erases the entry. The observer stays registered on its real center. Every later notification posts to RunNotificationCallback, finds no entry and is dropped. ClearNotificationSubscriptions no longer sees the entry, so the observer leaks for the rest of the process. On the base branch the same wrong-center removal happened, but the block kept invoking the captured callback. Now that each Subscription stores its kind, the fix is a one-line change.
Verification: pre-existing. At line 338, DoUnsubscribeNotification calls [GetNotificationCenter(kind) removeObserver:iter->second.observer]; with the caller's kind instead of iter->second.kind, then runs subscriptions.erase(iter); no matter what the stored kind is. The observer stays registered on its real center. The base did the same thing, so this is not a regression.
|
Release Notes Persisted
|
Backport of #55629
See that PR for details.
Notes: Fixed a possible hang on quit on macOS when using
systemPreferences.subscribeLocalNotification,subscribeNotification, orsubscribeWorkspaceNotification.