#51597: fix: Crash for Notification close
Merged
On the macOS platform, calling the close function again in the close event of a Notification will cause a crash. The specific reason is as follows: When the close function of a Notification is called, the dismiss logic dispatches a "close" event. If the close function is called again in the close event, it will reset |notification_|, causing the weakptr to become invalid. The first close function will continue to use this invalid weakptr directly, thus triggering a crash.
Description of Change
(lldb) bt
warning: Electron Framework was compiled with optimization - stepping may behave oddly; variables may not be available.
* thread #1, name = 'CrBrowserMain', queue = 'com.apple.main-thread', stop reason = EXC_BREAKPOINT (code=1, subcode=0x10d33e824)
* frame #0: 0x000000010d33e824 Electron Framework`base::ImmediateCrash() at immediate_crash.h:186:3 [opt] [inlined]
frame #1: 0x000000010d33e824 Electron Framework`logging::CheckFailure() at check.h:258:3 [opt] [inlined]
frame #2: 0x000000010d33e824 Electron Framework`base::WeakPtr<electron::Notification>::operator->(this=<unavailable>) const at weak_ptr.h:249:5 [opt] [inlined]
frame #3: 0x000000010d33e824 Electron Framework`electron::api::Notification::Close(this=0x0000012400186f60) at [electron_api_notification.cc:172](http://electron_api_notification.cc:172/):5 [opt]
frame #4: 0x000000010d340864 Electron Framework`base::RepeatingCallback<void (electron::api::Notification*)>::Run(this=0x000000016fdfb5f8, args=0x000000016fdfb5d0) const & at callback.h:344:12 [opt] [inlined]
frame #5: 0x000000010d340834 Electron Framework`gin::internal::Invoker<std::__Cr::integer_sequence<unsigned long, 0ul>, electron::api::Notification*>::DispatchToCallback(this=<unavailable>, callback=RepeatingCallback<void (electron::api::Notification *)> @ 0x000000016fdfb5f8) at function_template.h:232:14 [opt] [inlined]
frame #6: 0x000000010d340834 Electron Framework`gin::internal::Dispatcher<void (electron::api::Notification*)>::DispatchToCallbackImpl(args=0x000000016fdfb648) at function_template.h:264:15 [opt]
frame #7: 0x000000010d34066c Electron Framework`gin::internal::Dispatcher<void (electron::api::Notification*)>::DispatchToCallback(info=<unavailable>) at function_template.h:270:5 [opt]
frame #8: 0x0000000157e0f6f8
frame #9: 0x0000000157e0d710
frame #10: 0x0000000150012acc
frame #11: 0x0000000157e0b288
frame #12: 0x0000000157e0aed4
The crash stack is as shown above.The specific reason is:
void Notification::Close() {
if (notification_) {
if (notification_->is_dismissed()) {
notification_->Remove();
} else {
notification_->Dismiss(); ==> It will dispatch "close" event. If call notification.close in "close" event againt,
==> it run |notification_.reset()| in another close function.
}
notification_->set_delegate(nullptr); ==> notification_ is invaild now. And crash.
notification_.reset();
}
}
Checklist
- I have built and tested this change
- I have filled out the PR description
- I have reviewed and verified the changes
-
npm testpasses - 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, and are capitalized, punctuated, and past tense.
Release Notes
Notes: fix crash for Notification close
Backports
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