mirror of
https://github.com/electron/electron.git
synced 2026-05-02 03:00:22 -04:00
fix: shutdown crash when unregistering power notification on windows (#50893)
* fix: crash on shutdown when unregistering power notification
Refs https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-unregistersuspendresumenotification
the handle should be the return value of registeration api
not the window handle.
Co-authored-by: deepak1556 <hop2deep@gmail.com>
* chore: remove redundant selfkeepalive
Followup to 7c0cb61b3c
Co-authored-by: deepak1556 <hop2deep@gmail.com>
* chore: fix lint
Co-authored-by: deepak1556 <hop2deep@gmail.com>
* chore: address review feedback
Co-authored-by: deepak1556 <hop2deep@gmail.com>
---------
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: deepak1556 <hop2deep@gmail.com>
This commit is contained in:
@@ -8,7 +8,6 @@
|
||||
#include "base/power_monitor/power_observer.h"
|
||||
#include "gin/wrappable.h"
|
||||
#include "shell/browser/event_emitter_mixin.h"
|
||||
#include "shell/common/gin_helper/self_keep_alive.h"
|
||||
|
||||
#if BUILDFLAG(IS_LINUX)
|
||||
#include "shell/browser/lib/power_observer_linux.h"
|
||||
@@ -89,13 +88,14 @@ class PowerMonitor final : public gin::Wrappable<PowerMonitor>,
|
||||
|
||||
// The window used for processing events.
|
||||
HWND window_;
|
||||
|
||||
// Handle returned by RegisterSuspendResumeNotification.
|
||||
HPOWERNOTIFY power_notify_handle_ = nullptr;
|
||||
#endif
|
||||
|
||||
#if BUILDFLAG(IS_LINUX)
|
||||
PowerObserverLinux power_observer_linux_{this};
|
||||
#endif
|
||||
|
||||
gin_helper::SelfKeepAlive<PowerMonitor> keep_alive_{this};
|
||||
};
|
||||
|
||||
} // namespace electron::api
|
||||
|
||||
@@ -45,14 +45,19 @@ void PowerMonitor::InitPlatformSpecificMonitors() {
|
||||
|
||||
// For Windows 8 and later, a new "connected standby" mode has been added and
|
||||
// we must explicitly register for its notifications.
|
||||
RegisterSuspendResumeNotification(static_cast<HANDLE>(window_),
|
||||
DEVICE_NOTIFY_WINDOW_HANDLE);
|
||||
power_notify_handle_ = RegisterSuspendResumeNotification(
|
||||
static_cast<HANDLE>(window_), DEVICE_NOTIFY_WINDOW_HANDLE);
|
||||
PLOG_IF(ERROR, !power_notify_handle_)
|
||||
<< "RegisterSuspendResumeNotification failed";
|
||||
}
|
||||
|
||||
void PowerMonitor::DestroyPlatformSpecificMonitors() {
|
||||
if (window_) {
|
||||
WTSUnRegisterSessionNotification(window_);
|
||||
UnregisterSuspendResumeNotification(static_cast<HANDLE>(window_));
|
||||
if (power_notify_handle_) {
|
||||
UnregisterSuspendResumeNotification(power_notify_handle_);
|
||||
power_notify_handle_ = nullptr;
|
||||
}
|
||||
gfx::SetWindowUserData(window_, nullptr);
|
||||
DestroyWindow(window_);
|
||||
window_ = nullptr;
|
||||
@@ -90,7 +95,8 @@ LRESULT CALLBACK PowerMonitor::WndProc(HWND hwnd,
|
||||
}
|
||||
if (should_treat_as_current_session) {
|
||||
if (wparam == WTS_SESSION_LOCK) {
|
||||
// SelfKeepAlive prevents GC of this object, so Unretained is safe.
|
||||
// JS module reference prevents GC of this object, so Unretained is
|
||||
// safe.
|
||||
content::GetUIThreadTaskRunner({})->PostTask(
|
||||
FROM_HERE,
|
||||
base::BindOnce([](PowerMonitor* pm) { pm->Emit("lock-screen"); },
|
||||
|
||||
@@ -176,4 +176,44 @@ describe('cpp heap', () => {
|
||||
expect(result).to.equal(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('powerMonitor module', () => {
|
||||
it('should retain native PowerMonitor via JS module reference', async () => {
|
||||
const rc = await startRemoteControlApp(['--expose-internals', '--js-flags=--expose-gc']);
|
||||
const result = await rc.remotely(async (heap: string) => {
|
||||
const { powerMonitor, app } = require('electron');
|
||||
const { recordState } = require(heap);
|
||||
const v8Util = (process as any)._linkedBinding('electron_common_v8_util');
|
||||
|
||||
// Register a listener to trigger native PowerMonitor creation.
|
||||
const listener = () => {};
|
||||
powerMonitor.on('suspend', listener);
|
||||
|
||||
// Add and remove several listeners to exercise the path,
|
||||
// then GC to ensure no duplicate native objects are created.
|
||||
for (let i = 0; i < 5; i++) {
|
||||
const tmp = () => {};
|
||||
powerMonitor.on('resume', tmp);
|
||||
powerMonitor.removeListener('resume', tmp);
|
||||
}
|
||||
|
||||
v8Util.requestGarbageCollectionForTesting();
|
||||
|
||||
const state = recordState();
|
||||
const nodes = state.snapshot.filter(
|
||||
(node: any) => node.name === 'Electron / PowerMonitor'
|
||||
);
|
||||
const found = nodes.length > 0;
|
||||
const noDuplicates = nodes.length === 1;
|
||||
|
||||
setTimeout(() => app.quit());
|
||||
return { found, noDuplicates };
|
||||
}, path.join(__dirname, '../../third_party/electron_node/test/common/heap'));
|
||||
|
||||
const [code] = await once(rc.process, 'exit');
|
||||
expect(code).to.equal(0);
|
||||
expect(result.found).to.equal(true, 'PowerMonitor should be in snapshot (held by JS module)');
|
||||
expect(result.noDuplicates).to.equal(true, 'should have exactly one PowerMonitor instance');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user