Electron crash in escape key event handler on MacOS #96492
Comments
|
(Experimental duplicate detection) |
|
Note that this stack trace is different from the one attached in #92192 (comment), also referenced in #92420 |
|
@pelmers thanks for the report, based on it the root cause is same as the other issues you have linked to. Just to confirm, is this crash seen with an internal build of VSCode at Facebook, if so do you build with OSS electron releases https://www.electronjs.org/releases/stable ? |
deepak1556
added
freeze-slow-crash-leak
Yes, it's an internal build of VS Code, however it does use OSS Electron releases (the differences from OSS VS Code are mostly cosmetic, e.g. the welcome page) |
|
Thanks for confirming, the fix for this landed in an internal build of electron and I didn't upstream it yet. From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: deepak1556 <hop2deep@gmail.com>
Date: Tue, 17 Mar 2020 14:36:38 -0700
Subject: fix: ensure guest-embedder map is updated when webview is removed
There are use cases of webview where the container holding the webview is not
actually destroyed first, instead just webview gets removed from DOM, in such
situations the browser process map is not updated accordingly and holds reference
to stale guest contents, and any window operations like scroll, resize or keyboard
events that has to chain through browser embedder will lead to UAF crash.
Ref: https://github.com/microsoft/vscode/issues/92420
diff --git a/lib/browser/guest-view-manager.js b/lib/browser/guest-view-manager.js
index 1bbf487159b46edb4c89791dab0ea270837357c3..10e0ebf85f67591b983d9778391979f4de01d3c3 100644
--- a/lib/browser/guest-view-manager.js
+++ b/lib/browser/guest-view-manager.js
@@ -254,6 +254,9 @@ const attachGuest = function (event, embedderFrameId, elementInstanceId, guestIn
// Remove an guest-embedder relationship.
const detachGuest = function (embedder, guestInstanceId) {
const guestInstance = guestInstances[guestInstanceId]
+
+ if (!guestInstance) return
+
if (embedder !== guestInstance.embedder) {
return
}
@@ -336,6 +339,10 @@ handleMessage('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', function (event, embed
}
})
+handleMessage('ELECTRON_GUEST_VIEW_MANAGER_DETACH_GUEST', function (event, guestInstanceId) {
+ return detachGuest(event.sender, guestInstanceId)
+})
+
// this message is sent by the actual <webview>
ipcMainInternal.on('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', function (event, focus, guestInstanceId) {
const guest = getGuest(guestInstanceId)
diff --git a/lib/renderer/web-view/guest-view-internal.ts b/lib/renderer/web-view/guest-view-internal.ts
index 551e6f8539e0e516ee1dde488de8a6677f9e50e8..da43ee42fd633df1ad0763508143995a30886017 100644
--- a/lib/renderer/web-view/guest-view-internal.ts
+++ b/lib/renderer/web-view/guest-view-internal.ts
@@ -110,9 +110,14 @@ export function attachGuest (
invoke('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', embedderFrameId, elementInstanceId, guestInstanceId, params)
}
+export function detachGuest (guestInstanceId: number) {
+ return invokeSync('ELECTRON_GUEST_VIEW_MANAGER_DETACH_GUEST', guestInstanceId)
+}
+
export const guestViewInternalModule = {
deregisterEvents,
createGuest,
createGuestSync,
- attachGuest
+ attachGuest,
+ detachGuest
}
diff --git a/lib/renderer/web-view/web-view-element.ts b/lib/renderer/web-view/web-view-element.ts
index cdb81308e22d29961dfc21a58df47b4dacb78639..22574c505088bd74ed8b8e6949987289303da896 100644
--- a/lib/renderer/web-view/web-view-element.ts
+++ b/lib/renderer/web-view/web-view-element.ts
@@ -66,6 +66,9 @@ const defineWebViewElement = (v8Util: NodeJS.V8UtilBinding, webViewImpl: typeof
return
}
guestViewInternal.deregisterEvents(internal.viewInstanceId)
+ if (internal.guestInstanceId) {
+ guestViewInternal.detachGuest(internal.guestInstanceId)
+ }
internal.elementAttached = false
this.internalInstanceId = 0
internal.reset()
|
|
I will upstream it today, that should help fix the crash on your end. |
deepak1556
added
upstream
|
Excellent, thanks for the quick response and fix! |
deepak1556
added
the
upstream-issue-linked
A number of users of VS Code at Facebook have reported VS Code crashes when the escape key is hit. A similar issue is #92880, but that was closed without resolution.
Steps to Reproduce:
Unfortunately this is not very reliable for me, but I do have multiple stack traces for evidence that all point to the same exception:
Crash report (from VS Code 1.44):
Symbolicated trace:
The text was updated successfully, but these errors were encountered: