From 85515c0096cd111616adf17bd94ae60ccf1168b4 Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Fri, 8 Nov 2024 17:02:31 -0700 Subject: [PATCH] LibWeb: Queue the task for MessagePort receive in targetPort's realm We were delaying sending an IPC message until the HTML PortMessage task was run, but we were not queuing the task in on the receiver side. The result of this was that message port posting was needlessly creating an HTML task just to send an IPC message. On the flip side, we were directly calling dispatch_event from the socket notifier of the target port's message queue. This is a huge problem, because it means that we were effectively running javascript-aware code from an 'in parallel' context. By switching around which side of the IPC interface is responsible for queuing a task, we can avoid problems where a document is destroyed from a port message-attached callback and crashes. --- .../Libraries/LibWeb/HTML/MessagePort.cpp | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/Userland/Libraries/LibWeb/HTML/MessagePort.cpp b/Userland/Libraries/LibWeb/HTML/MessagePort.cpp index b53e1257a40..8bcf90b1b41 100644 --- a/Userland/Libraries/LibWeb/HTML/MessagePort.cpp +++ b/Userland/Libraries/LibWeb/HTML/MessagePort.cpp @@ -280,15 +280,12 @@ ErrorOr MessagePort::send_message_on_transport(SerializedTransferRecord co void MessagePort::post_port_message(SerializedTransferRecord serialize_with_transfer_result) { - // FIXME: Use the correct task source? - queue_global_task(Task::Source::PostedMessage, relevant_global_object(*this), JS::create_heap_function(heap(), [this, serialize_with_transfer_result = move(serialize_with_transfer_result)]() mutable { - if (!m_transport.has_value() || !m_transport->is_open()) - return; - if (auto result = send_message_on_transport(serialize_with_transfer_result); result.is_error()) { - dbgln("Failed to post message: {}", result.error()); - disentangle(); - } - })); + if (!m_transport.has_value() || !m_transport->is_open()) + return; + if (auto result = send_message_on_transport(serialize_with_transfer_result); result.is_error()) { + dbgln("Failed to post message: {}", result.error()); + disentangle(); + } } ErrorOr MessagePort::parse_message() @@ -324,7 +321,11 @@ ErrorOr MessagePort::parse_message() m_buffered_data.remove(0, HEADER_SIZE + m_socket_incoming_message_size); - post_message_task_steps(serialized_transfer_record); + // Note: this is step 7 of message_port_post_message_steps: + // 7. Add a task that runs the following steps to the port message queue of targetPort: + queue_global_task(Task::Source::PostedMessage, relevant_global_object(*this), JS::create_heap_function(heap(), [this, serialized_transfer_record = move(serialized_transfer_record)]() mutable { + this->post_message_task_steps(serialized_transfer_record); + })); break; }