-
Notifications
You must be signed in to change notification settings - Fork 15k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: ensure MessagePorts get GCed when not referenced (#40189)
- Loading branch information
1 parent
5d6023a
commit bbd2236
Showing
3 changed files
with
75 additions
and
5 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
73 changes: 73 additions & 0 deletions
73
patches/chromium/ensure_messageports_get_gced_when_not_referenced.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Yoav Weiss <yoavweiss@chromium.org> | ||
Date: Mon, 9 Oct 2023 14:21:44 +0000 | ||
Subject: Ensure MessagePorts get GCed when not referenced | ||
|
||
[1] regressed MessagePort memory and caused them to no longer be | ||
collected after not being referenced. | ||
This CL fixes that by turning the mutual pointers between attached | ||
MessagePorts to be WeakMembers. | ||
|
||
[1] https://chromium-review.googlesource.com/4919476 | ||
|
||
Bug: 1487835 | ||
Change-Id: Icd7eba09a217ad5d588958504d5c4794d7d8a295 | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4919476 | ||
Commit-Queue: Yoav Weiss <yoavweiss@chromium.org> | ||
Reviewed-by: Michael Lippautz <mlippautz@chromium.org> | ||
Cr-Commit-Position: refs/heads/main@{#1207067} | ||
|
||
diff --git a/third_party/blink/renderer/core/messaging/message_port.cc b/third_party/blink/renderer/core/messaging/message_port.cc | ||
index 0fd5bbf170efa02d3e710de3cb6733158faec858..15f5f0f6aa05d3b17adae87286c92df9cc26a712 100644 | ||
--- a/third_party/blink/renderer/core/messaging/message_port.cc | ||
+++ b/third_party/blink/renderer/core/messaging/message_port.cc | ||
@@ -162,8 +162,10 @@ MessagePortChannel MessagePort::Disentangle() { | ||
DCHECK(!IsNeutered()); | ||
port_descriptor_.GiveDisentangledHandle(connector_->PassMessagePipe()); | ||
connector_ = nullptr; | ||
- if (initially_entangled_port_) { | ||
- initially_entangled_port_->OnEntangledPortDisconnected(); | ||
+ // Using a variable here places the WeakMember pointer on the stack, ensuring | ||
+ // it doesn't get GCed while it's being used. | ||
+ if (auto* entangled_port = initially_entangled_port_.Get()) { | ||
+ entangled_port->OnEntangledPortDisconnected(); | ||
} | ||
OnEntangledPortDisconnected(); | ||
return MessagePortChannel(std::move(port_descriptor_)); | ||
@@ -340,7 +342,10 @@ bool MessagePort::Accept(mojo::Message* mojo_message) { | ||
// lifetime of this function. | ||
std::unique_ptr<scheduler::TaskAttributionTracker::TaskScope> | ||
task_attribution_scope; | ||
- if (initially_entangled_port_ && message.sender_origin && | ||
+ // Using a variable here places the WeakMember pointer on the stack, ensuring | ||
+ // it doesn't get GCed while it's being used. | ||
+ auto* entangled_port = initially_entangled_port_.Get(); | ||
+ if (entangled_port && message.sender_origin && | ||
message.sender_origin->IsSameOriginWith(context->GetSecurityOrigin()) && | ||
context->IsSameAgentCluster(message.sender_agent_cluster_id) && | ||
context->IsWindow()) { | ||
@@ -364,9 +369,9 @@ bool MessagePort::Accept(mojo::Message* mojo_message) { | ||
ThreadScheduler::Current()->GetTaskAttributionTracker()) { | ||
// Since `initially_entangled_port_` is not nullptr, neither should be | ||
// its `post_message_task_container_`. | ||
- CHECK(initially_entangled_port_->post_message_task_container_); | ||
+ CHECK(entangled_port->post_message_task_container_); | ||
scheduler::TaskAttributionInfo* parent_task = | ||
- initially_entangled_port_->post_message_task_container_ | ||
+ entangled_port->post_message_task_container_ | ||
->GetAndDecrementPostMessageTask(message.parent_task_id); | ||
task_attribution_scope = tracker->CreateTaskScope( | ||
script_state, parent_task, | ||
diff --git a/third_party/blink/renderer/core/messaging/message_port.h b/third_party/blink/renderer/core/messaging/message_port.h | ||
index 98ed58a9a765f5101d9b421507bf25db4359d7e5..a178d15c11b1e5fb1ff74d182021fe39e9d9b107 100644 | ||
--- a/third_party/blink/renderer/core/messaging/message_port.h | ||
+++ b/third_party/blink/renderer/core/messaging/message_port.h | ||
@@ -195,7 +195,7 @@ class CORE_EXPORT MessagePort : public EventTarget, | ||
|
||
// The entangled port. Only set on initial entanglement, and gets unset as | ||
// soon as the ports are disentangled. | ||
- Member<MessagePort> initially_entangled_port_; | ||
+ WeakMember<MessagePort> initially_entangled_port_; | ||
|
||
Member<PostMessageTaskContainer> post_message_task_container_; | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters