New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: crash using powerMonitor
before ready event
#40888
Conversation
powerMonitor
before ready event
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lazy initialize seems like a good approach to me 👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit: wouldn't it be better to continue using the wrapper class to avoid missing any additional bluez initialization or shutdown paths that might get added in future ?
if (!bluez::BluezDBusManager::IsInitialized())
bluez::DBusBluezManagerWrapperLinux::Initialize();
@deepak1556 done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Failing test unrelated:
|
Release Notes Persisted
|
I have automatically backported this PR to "26-x-y", please check out #40922 |
I have automatically backported this PR to "27-x-y", please check out #40923 |
I have automatically backported this PR to "28-x-y", please check out #40924 |
I have automatically backported this PR to "29-x-y", please check out #40925 |
Description of Change
Closes #40694.
Refs #37937.
This PR fixes a crash resultant in trying to listen to power-related events before the
ready
event is emitted on Linux. This was happening because thePowerObserverLinux
ctor calledbluez::BluezDBusThreadManager::Get()
, which assumed thatbluez::BluezDBusThreadManager::Initialize()
had already been called andCHECK
ed if it hadn't been. However, our existing logic did not callbluez::BluezDBusThreadManager::Initialize()
untilElectronBrowserMainParts::PostCreateMainMessageLoop()
, which meant that trying to use:before the
ready
event would immediately die. I chose to fix this by callingbluez::BluezDBusThreadManager::Initialize()
as soon as it was needed, and adding a check to ensure that it was not initialized more than once. Alternately, we can bring back theready
event requirement.Checklist
npm test
passesRelease Notes
Notes: Fixed a crash resultant from trying to listen to power-related events before the
ready
event was emitted on Linux.