Browse Source

fix: use OS process handle to clear object registry (#14324)

RenderProcessHost switch can happen between ipc calls when
speculative process are invvolved, which will lead to deletion
of entries on current context. Use OS process handles to
uniquely associate a destruction handler for a render process.
Robo 6 years ago
parent
commit
edd5c4b9bb

+ 2 - 1
atom/browser/api/atom_api_web_contents.cc

@@ -768,7 +768,8 @@ void WebContents::RenderViewCreated(content::RenderViewHost* render_view_host) {
 }
 
 void WebContents::RenderViewDeleted(content::RenderViewHost* render_view_host) {
-  Emit("render-view-deleted", render_view_host->GetProcess()->GetID());
+  Emit("render-view-deleted", render_view_host->GetProcess()->GetID(),
+       base::GetProcId(render_view_host->GetProcess()->GetHandle()));
 }
 
 void WebContents::RenderProcessGone(base::TerminationStatus status) {

+ 3 - 2
atom/renderer/renderer_client_base.cc

@@ -16,7 +16,7 @@
 #include "atom/renderer/content_settings_observer.h"
 #include "atom/renderer/preferences_manager.h"
 #include "base/command_line.h"
-#include "base/process/process_handle.h"
+#include "base/process/process.h"
 #include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/renderer/media/chrome_key_systems.h"
@@ -97,7 +97,8 @@ void RendererClientBase::DidCreateScriptContext(
     content::RenderFrame* render_frame) {
   // global.setHidden("contextId", `${processId}-${++next_context_id_}`)
   std::string context_id = base::StringPrintf(
-      "%" CrPRIdPid "-%d", base::GetCurrentProcId(), ++next_context_id_);
+      "%" CrPRIdPid "-%d", base::GetProcId(base::Process::Current().Handle()),
+      ++next_context_id_);
   v8::Isolate* isolate = context->GetIsolate();
   v8::Local<v8::String> key = mate::StringToSymbol(isolate, "contextId");
   v8::Local<v8::Private> private_key = v8::Private::ForApi(isolate, key);

+ 4 - 3
lib/browser/objects-registry.js

@@ -103,9 +103,10 @@ class ObjectsRegistry {
 
   // Private: Clear the storage when renderer process is destroyed.
   registerDeleteListener (webContents, contextId) {
-    const processId = webContents.getProcessId()
-    const listener = (event, deletedProcessId) => {
-      if (deletedProcessId === processId) {
+    // contextId => ${OSProcessId}-${contextCount}
+    const OSProcessId = contextId.split('-')[0]
+    const listener = (event, deletedProcessId, deletedOSProcessId) => {
+      if (deletedOSProcessId && deletedOSProcessId.toString() === OSProcessId) {
         webContents.removeListener('render-view-deleted', listener)
         this.clear(webContents, contextId)
       }

+ 1 - 1
spec/fixtures/api/render-view-deleted.html

@@ -17,7 +17,7 @@
       }
 
       // This should trigger a dereference and a remote getURL call should fail
-      contents.emit('render-view-deleted', {}, contents.getProcessId())
+      contents.emit('render-view-deleted', {}, '', contents.getOSProcessId())
       try {
         contents.getURL()
         ipcRenderer.send('error-message', 'No error thrown')