fix: prevent use-after-free in PowerMonitor via dangling OS callbacks (#50090)

PowerMonitor registered OS-level callbacks (HWND UserData and
WTS/suspend notifications on Windows, shutdown handler and lock-screen
observer on macOS) but never cleaned them up in its destructor. The JS
layer also only held the native object in a closure-local variable,
allowing GC to reclaim it while those registrations still referenced
freed memory.

Retain the native PowerMonitor at module level in power-monitor.ts so
it cannot be garbage-collected. Add DestroyPlatformSpecificMonitors()
to properly tear down OS registrations on destruction: on Windows,
unregister WTS and suspend notifications, clear GWLP_USERDATA, and
destroy the HWND; on macOS, remove the emitter from the global
MacLockMonitor and reset the Browser shutdown handler.

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
This commit is contained in:
trop[bot]
2026-03-05 15:52:38 -05:00
committed by GitHub
parent 38b309043a
commit 18ee76cc47
5 changed files with 41 additions and 2 deletions

View File

@@ -8,13 +8,19 @@ const {
isOnBatteryPower
} = process._linkedBinding('electron_browser_power_monitor');
// Hold the native PowerMonitor at module level so it is never garbage-collected
// while this module is alive. The C++ side registers OS-level callbacks (HWND
// user-data on Windows, shutdown handler on macOS, notification observers) that
// prevent safe collection of the C++ wrapper while those registrations exist.
let pm: any;
class PowerMonitor extends EventEmitter implements Electron.PowerMonitor {
constructor () {
super();
// Don't start the event source until both a) the app is ready and b)
// there's a listener registered for a powerMonitor event.
this.once('newListener', () => {
const pm = createPowerMonitor();
pm = createPowerMonitor();
pm.emit = this.emit.bind(this);
if (process.platform === 'linux') {

View File

@@ -80,6 +80,14 @@ PowerMonitor::PowerMonitor(v8::Isolate* isolate) {
}
PowerMonitor::~PowerMonitor() {
#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
DestroyPlatformSpecificMonitors();
#endif
#if BUILDFLAG(IS_MAC)
Browser::Get()->SetShutdownHandler(base::RepeatingCallback<bool()>());
#endif
auto* power_monitor = base::PowerMonitor::GetInstance();
power_monitor->RemovePowerStateObserver(this);
power_monitor->RemovePowerSuspendObserver(this);

View File

@@ -49,6 +49,7 @@ class PowerMonitor final : public gin_helper::DeprecatedWrappable<PowerMonitor>,
#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
void InitPlatformSpecificMonitors();
void DestroyPlatformSpecificMonitors();
#endif
// base::PowerStateObserver implementations:

View File

@@ -15,6 +15,7 @@
}
- (void)addEmitter:(electron::api::PowerMonitor*)monitor_;
- (void)removeEmitter:(electron::api::PowerMonitor*)monitor_;
@end
@@ -62,6 +63,10 @@
self->emitters.push_back(monitor_);
}
- (void)removeEmitter:(electron::api::PowerMonitor*)monitor_ {
std::erase(self->emitters, monitor_);
}
- (void)onScreenLocked:(NSNotification*)notification {
for (auto* emitter : self->emitters) {
emitter->Emit("lock-screen");
@@ -98,4 +103,9 @@ void PowerMonitor::InitPlatformSpecificMonitors() {
[g_lock_monitor addEmitter:this];
}
void PowerMonitor::DestroyPlatformSpecificMonitors() {
if (g_lock_monitor)
[g_lock_monitor removeEmitter:this];
}
} // namespace electron::api

View File

@@ -49,6 +49,20 @@ void PowerMonitor::InitPlatformSpecificMonitors() {
DEVICE_NOTIFY_WINDOW_HANDLE);
}
void PowerMonitor::DestroyPlatformSpecificMonitors() {
if (window_) {
WTSUnRegisterSessionNotification(window_);
UnregisterSuspendResumeNotification(static_cast<HANDLE>(window_));
gfx::SetWindowUserData(window_, nullptr);
DestroyWindow(window_);
window_ = nullptr;
}
if (atom_) {
UnregisterClass(MAKEINTATOM(atom_), instance_);
atom_ = 0;
}
}
LRESULT CALLBACK PowerMonitor::WndProcStatic(HWND hwnd,
UINT message,
WPARAM wparam,
@@ -76,7 +90,7 @@ LRESULT CALLBACK PowerMonitor::WndProc(HWND hwnd,
}
if (should_treat_as_current_session) {
if (wparam == WTS_SESSION_LOCK) {
// Unretained is OK because this object is eternally pinned.
// SelfKeepAlive prevents GC of this object, so Unretained is safe.
content::GetUIThreadTaskRunner({})->PostTask(
FROM_HERE,
base::BindOnce([](PowerMonitor* pm) { pm->Emit("lock-screen"); },