Browse Source

chore: add additional crash key to gin::Wrappable (#30161)

Keeley Hammond 3 years ago
parent
commit
19820fc2a7

+ 1 - 0
patches/chromium/.patches

@@ -102,3 +102,4 @@ refactor_restore_base_adaptcallbackforrepeating.patch
 hack_to_allow_gclient_sync_with_host_os_mac_on_linux_in_ci.patch
 don_t_run_pcscan_notifythreadcreated_if_pcscan_is_disabled.patch
 set_svgimage_page_after_document_install.patch
+add_gin_wrappable_crash_key.patch

+ 44 - 0
patches/chromium/add_gin_wrappable_crash_key.patch

@@ -0,0 +1,44 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: VerteDinde <[email protected]>
+Date: Thu, 15 Jul 2021 12:16:50 -0700
+Subject: chore: add gin::wrappable wrapperinfo crash key
+
+This patch adds an additional crash key for gin::Wrappable, to help
+debug a crash that is occurring during garbage collection in SecondWeakCallback.
+
+The crash seems to be due to a class that is holding a reference to
+gin::Wrappable even after being deleted. This added crash key compares
+the soon-to-be-deleted WrapperInfo with known WrapperInfo components to
+help determine where the crash originated from.
+
+This patch should not be upstreamed, and can be removed in Electron 15 and
+beyond once we identify the cause of the crash.
+
+diff --git a/gin/wrappable.cc b/gin/wrappable.cc
+index fe07eb94a8e679859bba6d76ff0d6ee86bd0c67e..d0066fca501eae5be4177440b44dbecc8e34c897 100644
+--- a/gin/wrappable.cc
++++ b/gin/wrappable.cc
+@@ -8,6 +8,10 @@
+ #include "gin/object_template_builder.h"
+ #include "gin/per_isolate_data.h"
+ 
++#if !defined(MAS_BUILD)
++#include "electron/shell/common/crash_keys.h"
++#endif
++
+ namespace gin {
+ 
+ WrappableBase::WrappableBase() = default;
+@@ -36,6 +40,12 @@ void WrappableBase::FirstWeakCallback(
+ void WrappableBase::SecondWeakCallback(
+     const v8::WeakCallbackInfo<WrappableBase>& data) {
+   WrappableBase* wrappable = data.GetParameter();
++
++#if !defined(MAS_BUILD)
++  WrapperInfo* info = static_cast<WrapperInfo*>(data.GetInternalField(0));
++  electron::crash_keys::SetCrashKeyForGinWrappable(info);
++#endif
++
+   delete wrappable;
+ }
+ 

+ 57 - 0
shell/common/crash_keys.cc

@@ -19,6 +19,21 @@
 #include "shell/common/options_switches.h"
 #include "third_party/crashpad/crashpad/client/annotation.h"
 
+#include "gin/wrappable.h"
+#include "shell/browser/api/electron_api_browser_view.h"
+#include "shell/browser/api/electron_api_cookies.h"
+#include "shell/browser/api/electron_api_desktop_capturer.h"
+#include "shell/browser/api/electron_api_menu.h"
+#include "shell/browser/api/electron_api_net_log.h"
+#include "shell/browser/api/electron_api_notification.h"
+#include "shell/browser/api/electron_api_power_monitor.h"
+#include "shell/browser/api/electron_api_protocol.h"
+#include "shell/browser/api/electron_api_service_worker_context.h"
+#include "shell/browser/api/electron_api_web_contents.h"
+#include "shell/browser/api/electron_api_web_frame_main.h"
+#include "shell/browser/api/electron_api_web_request.h"
+#include "shell/common/api/electron_api_native_image.h"
+
 namespace electron {
 
 namespace crash_keys {
@@ -155,6 +170,48 @@ void SetPlatformCrashKey() {
 #endif
 }
 
+void SetCrashKeyForGinWrappable(gin::WrapperInfo* info) {
+  std::string crash_location;
+
+  // Adds a breadcrumb for crashes within gin::WrappableBase::SecondWeakCallback
+  // (see patch: add_gin_wrappable_crash_key.patch)
+  // Compares the pointers for the kWrapperInfo within SecondWeakCallback
+  // with the wrapper info from classes that use gin::Wrappable and
+  // could potentially retain a reference after deletion.
+  if (info == &electron::api::WebContents::kWrapperInfo)
+    crash_location = "WebContents";
+  else if (info == &electron::api::BrowserView::kWrapperInfo)
+    crash_location = "BrowserView";
+  else if (info == &electron::api::Notification::kWrapperInfo)
+    crash_location = "Notification";
+  else if (info == &electron::api::Cookies::kWrapperInfo)
+    crash_location = "Cookies";
+  else if (info == &electron::api::DesktopCapturer::kWrapperInfo)
+    crash_location = "DesktopCapturer";
+  else if (info == &electron::api::NetLog::kWrapperInfo)
+    crash_location = "NetLog";
+  else if (info == &electron::api::NativeImage::kWrapperInfo)
+    crash_location = "NativeImage";
+  else if (info == &electron::api::Menu::kWrapperInfo)
+    crash_location = "Menu";
+  else if (info == &electron::api::PowerMonitor::kWrapperInfo)
+    crash_location = "PowerMonitor";
+  else if (info == &electron::api::Protocol::kWrapperInfo)
+    crash_location = "Protocol";
+  else if (info == &electron::api::ServiceWorkerContext::kWrapperInfo)
+    crash_location = "ServiceWorkerContext";
+  else if (info == &electron::api::WebFrameMain::kWrapperInfo)
+    crash_location = "WebFrameMain";
+  else if (info == &electron::api::WebRequest::kWrapperInfo)
+    crash_location = "WebRequest";
+  else
+    crash_location =
+        "Deleted kWrapperInfo does not match listed component. Please review "
+        "listed crash keys.";
+
+  SetCrashKey("gin-wrappable-fatal.location", crash_location);
+}
+
 }  // namespace crash_keys
 
 }  // namespace electron

+ 3 - 0
shell/common/crash_keys.h

@@ -8,6 +8,8 @@
 #include <map>
 #include <string>
 
+#include "gin/wrappable.h"
+
 namespace base {
 class CommandLine;
 }
@@ -22,6 +24,7 @@ void GetCrashKeys(std::map<std::string, std::string>* keys);
 
 void SetCrashKeysFromCommandLine(const base::CommandLine& command_line);
 void SetPlatformCrashKey();
+void SetCrashKeyForGinWrappable(gin::WrapperInfo* info);
 
 }  // namespace crash_keys