leak message ports in dom, until they are closed

This commit is contained in:
Gregory Terzian 2020-02-15 10:36:52 +08:00
parent 8f3622af35
commit 291ab7473d
2 changed files with 126 additions and 114 deletions

View file

@ -12,7 +12,7 @@ use crate::dom::bindings::error::{report_pending_exception, Error, ErrorInfo};
use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::refcounted::{Trusted, TrustedPromise}; use crate::dom::bindings::refcounted::{Trusted, TrustedPromise};
use crate::dom::bindings::reflector::DomObject; use crate::dom::bindings::reflector::DomObject;
use crate::dom::bindings::root::{DomRoot, MutNullableDom}; use crate::dom::bindings::root::{Dom, DomRoot, MutNullableDom};
use crate::dom::bindings::settings_stack::{entry_global, incumbent_global, AutoEntryScript}; use crate::dom::bindings::settings_stack::{entry_global, incumbent_global, AutoEntryScript};
use crate::dom::bindings::str::DOMString; use crate::dom::bindings::str::DOMString;
use crate::dom::bindings::structuredclone; use crate::dom::bindings::structuredclone;
@ -293,18 +293,26 @@ pub enum BlobState {
/// Data representing a message-port managed by this global. /// Data representing a message-port managed by this global.
#[derive(JSTraceable, MallocSizeOf)] #[derive(JSTraceable, MallocSizeOf)]
pub enum ManagedMessagePort { #[unrooted_must_root_lint::must_root]
pub struct ManagedMessagePort {
/// The DOM port.
dom_port: Dom<MessagePort>,
/// The logic and data backing the DOM port.
/// The option is needed to take out the port-impl
/// as part of its transferring steps,
/// without having to worry about rooting the dom-port.
port_impl: Option<MessagePortImpl>,
/// We keep ports pending when they are first transfer-received, /// We keep ports pending when they are first transfer-received,
/// and only add them, and ask the constellation to complete the transfer, /// and only add them, and ask the constellation to complete the transfer,
/// in a subsequent task if the port hasn't been re-transfered. /// in a subsequent task if the port hasn't been re-transfered.
Pending(MessagePortImpl, WeakRef<MessagePort>), pending: bool,
/// A port who was transferred into, or initially created in, this realm, /// Has the port been closed? If closed, it can be dropped and later GC'ed.
/// and that hasn't been re-transferred in the same task it was noted. closed: bool,
Added(MessagePortImpl, WeakRef<MessagePort>),
} }
/// State representing whether this global is currently managing messageports. /// State representing whether this global is currently managing messageports.
#[derive(JSTraceable, MallocSizeOf)] #[derive(JSTraceable, MallocSizeOf)]
#[unrooted_must_root_lint::must_root]
pub enum MessagePortState { pub enum MessagePortState {
/// The message-port router id for this global, and a map of managed ports. /// The message-port router id for this global, and a map of managed ports.
Managed( Managed(
@ -410,7 +418,7 @@ impl MessageListener {
let _ = self.task_source.queue_with_canceller( let _ = self.task_source.queue_with_canceller(
task!(process_remove_message_port: move || { task!(process_remove_message_port: move || {
let global = context.root(); let global = context.root();
global.remove_message_port(&port_id); global.note_entangled_port_removed(&port_id);
}), }),
&self.canceller, &self.canceller,
); );
@ -581,12 +589,16 @@ impl GlobalScope {
None => { None => {
panic!("complete_port_transfer called for an unknown port."); panic!("complete_port_transfer called for an unknown port.");
}, },
Some(ManagedMessagePort::Pending(_, _)) => { Some(managed_port) => {
panic!("CompleteTransfer msg received for a pending port."); if managed_port.pending {
}, panic!("CompleteTransfer msg received for a pending port.");
Some(ManagedMessagePort::Added(port_impl, _port)) => { }
port_impl.complete_transfer(tasks); if let Some(port_impl) = managed_port.port_impl.as_mut() {
port_impl.enabled() port_impl.complete_transfer(tasks);
port_impl.enabled()
} else {
panic!("managed-port has no port-impl.");
}
}, },
} }
} else { } else {
@ -626,19 +638,13 @@ impl GlobalScope {
None => { None => {
return warn!("entangled_ports called on a global not managing the port."); return warn!("entangled_ports called on a global not managing the port.");
}, },
Some(ManagedMessagePort::Pending(port_impl, dom_port)) => { Some(managed_port) => {
dom_port if let Some(port_impl) = managed_port.port_impl.as_mut() {
.root() managed_port.dom_port.entangle(entangled_id.clone());
.expect("Port to be entangled to not have been GC'ed") port_impl.entangle(entangled_id.clone());
.entangle(entangled_id.clone()); } else {
port_impl.entangle(entangled_id.clone()); panic!("managed-port has no port-impl.");
}, }
Some(ManagedMessagePort::Added(port_impl, dom_port)) => {
dom_port
.root()
.expect("Port to be entangled to not have been GC'ed")
.entangle(entangled_id.clone());
port_impl.entangle(entangled_id.clone());
}, },
} }
} }
@ -651,23 +657,16 @@ impl GlobalScope {
.send(ScriptMsg::EntanglePorts(port1, port2)); .send(ScriptMsg::EntanglePorts(port1, port2));
} }
/// Remove all referrences to a port. /// Note that the entangled port of `port_id` has been removed in another global.
pub fn remove_message_port(&self, port_id: &MessagePortId) { pub fn note_entangled_port_removed(&self, port_id: &MessagePortId) {
let is_empty = if let MessagePortState::Managed(_id, message_ports) = // Note: currently this is a no-op,
&mut *self.message_port_state.borrow_mut() // as we only use the `close` method to manage the local lifecyle of a port.
{ // This could be used as part of lifecyle management to determine a port can be GC'ed.
match message_ports.remove(&port_id) { // See https://github.com/servo/servo/issues/25772
None => panic!("remove_message_port called on a global not managing the port."), warn!(
Some(_) => message_ports.is_empty(), "Entangled port of {:?} has been removed in another global",
} port_id
} else { );
return warn!("remove_message_port called on a global not managing any ports.");
};
if is_empty {
// Remove our port router,
// it will be setup again if we start managing ports again.
self.remove_message_ports_router();
}
} }
/// Handle the transfer of a port in the current task. /// Handle the transfer of a port in the current task.
@ -675,18 +674,20 @@ impl GlobalScope {
if let MessagePortState::Managed(_id, message_ports) = if let MessagePortState::Managed(_id, message_ports) =
&mut *self.message_port_state.borrow_mut() &mut *self.message_port_state.borrow_mut()
{ {
let mut port = match message_ports.remove(&port_id) { let mut port_impl = message_ports
None => { .remove(&port_id)
panic!("mark_port_as_transferred called on a global not managing the port.") .map(|ref mut managed_port| {
}, managed_port
Some(ManagedMessagePort::Pending(port_impl, _)) => port_impl, .port_impl
Some(ManagedMessagePort::Added(port_impl, _)) => port_impl, .take()
}; .expect("Managed port doesn't have a port-impl.")
port.set_has_been_shipped(); })
.expect("mark_port_as_transferred called on a global not managing the port.");
port_impl.set_has_been_shipped();
let _ = self let _ = self
.script_to_constellation_chan() .script_to_constellation_chan()
.send(ScriptMsg::MessagePortShipped(port_id.clone())); .send(ScriptMsg::MessagePortShipped(port_id.clone()));
port port_impl
} else { } else {
panic!("mark_port_as_transferred called on a global not managing any ports."); panic!("mark_port_as_transferred called on a global not managing any ports.");
} }
@ -697,12 +698,17 @@ impl GlobalScope {
if let MessagePortState::Managed(_id, message_ports) = if let MessagePortState::Managed(_id, message_ports) =
&mut *self.message_port_state.borrow_mut() &mut *self.message_port_state.borrow_mut()
{ {
let port = match message_ports.get_mut(&port_id) { let message_buffer = match message_ports.get_mut(&port_id) {
None => panic!("start_message_port called on a unknown port."), None => panic!("start_message_port called on a unknown port."),
Some(ManagedMessagePort::Pending(port_impl, _)) => port_impl, Some(managed_port) => {
Some(ManagedMessagePort::Added(port_impl, _)) => port_impl, if let Some(port_impl) = managed_port.port_impl.as_mut() {
port_impl.start()
} else {
panic!("managed-port has no port-impl.");
}
},
}; };
if let Some(message_buffer) = port.start() { if let Some(message_buffer) = message_buffer {
for task in message_buffer { for task in message_buffer {
let port_id = port_id.clone(); let port_id = port_id.clone();
let this = Trusted::new(&*self); let this = Trusted::new(&*self);
@ -725,12 +731,17 @@ impl GlobalScope {
if let MessagePortState::Managed(_id, message_ports) = if let MessagePortState::Managed(_id, message_ports) =
&mut *self.message_port_state.borrow_mut() &mut *self.message_port_state.borrow_mut()
{ {
let port = match message_ports.get_mut(&port_id) { match message_ports.get_mut(&port_id) {
None => panic!("close_message_port called on an unknown port."), None => panic!("close_message_port called on an unknown port."),
Some(ManagedMessagePort::Pending(port_impl, _)) => port_impl, Some(managed_port) => {
Some(ManagedMessagePort::Added(port_impl, _)) => port_impl, if let Some(port_impl) = managed_port.port_impl.as_mut() {
port_impl.close();
managed_port.closed = true;
} else {
panic!("managed-port has no port-impl.");
}
},
}; };
port.close();
} else { } else {
return warn!("close_message_port called on a global not managing any ports."); return warn!("close_message_port called on a global not managing any ports.");
} }
@ -742,12 +753,17 @@ impl GlobalScope {
if let MessagePortState::Managed(_id, message_ports) = if let MessagePortState::Managed(_id, message_ports) =
&mut *self.message_port_state.borrow_mut() &mut *self.message_port_state.borrow_mut()
{ {
let port = match message_ports.get_mut(&port_id) { let entangled_port = match message_ports.get_mut(&port_id) {
None => panic!("post_messageport_msg called on an unknown port."), None => panic!("post_messageport_msg called on an unknown port."),
Some(ManagedMessagePort::Pending(port_impl, _)) => port_impl, Some(managed_port) => {
Some(ManagedMessagePort::Added(port_impl, _)) => port_impl, if let Some(port_impl) = managed_port.port_impl.as_mut() {
port_impl.entangled_port_id()
} else {
panic!("managed-port has no port-impl.");
}
},
}; };
if let Some(entangled_id) = port.entangled_port_id() { if let Some(entangled_id) = entangled_port {
// Step 7 // Step 7
let this = Trusted::new(&*self); let this = Trusted::new(&*self);
let _ = self.port_message_queue().queue( let _ = self.port_message_queue().queue(
@ -782,23 +798,19 @@ impl GlobalScope {
self.re_route_port_task(port_id, task); self.re_route_port_task(port_id, task);
return; return;
} }
let (port_impl, dom_port) = match message_ports.get_mut(&port_id) { match message_ports.get_mut(&port_id) {
None => panic!("route_task_to_port called for an unknown port."), None => panic!("route_task_to_port called for an unknown port."),
Some(ManagedMessagePort::Pending(port_impl, dom_port)) => (port_impl, dom_port), Some(managed_port) => {
Some(ManagedMessagePort::Added(port_impl, dom_port)) => (port_impl, dom_port), // If the port is not enabled yet, or if is awaiting the completion of it's transfer,
}; // the task will be buffered and dispatched upon enablement or completion of the transfer.
if let Some(port_impl) = managed_port.port_impl.as_mut() {
// If the port is not enabled yet, or if is awaiting the completion of it's transfer, port_impl.handle_incoming(task).and_then(|to_dispatch| {
// the task will be buffered and dispatched upon enablement or completion of the transfer. Some((DomRoot::from_ref(&*managed_port.dom_port), to_dispatch))
if let Some(task_to_dispatch) = port_impl.handle_incoming(task) { })
// Get a corresponding DOM message-port object. } else {
let dom_port = match dom_port.root() { panic!("managed-port has no port-impl.");
Some(dom_port) => dom_port, }
None => panic!("Messageport Gc'ed too early"), },
};
Some((dom_port, task_to_dispatch))
} else {
None
} }
} else { } else {
self.re_route_port_task(port_id, task); self.re_route_port_task(port_id, task);
@ -833,23 +845,22 @@ impl GlobalScope {
{ {
let to_be_added: Vec<MessagePortId> = message_ports let to_be_added: Vec<MessagePortId> = message_ports
.iter() .iter()
.filter_map(|(id, port_info)| match port_info { .filter_map(|(id, managed_port)| {
ManagedMessagePort::Pending(_, _) => Some(id.clone()), if managed_port.pending {
_ => None, Some(id.clone())
} else {
None
}
}) })
.collect(); .collect();
for id in to_be_added.iter() { for id in to_be_added.iter() {
let (id, port_info) = message_ports let managed_port = message_ports
.remove_entry(&id) .get_mut(&id)
.expect("Collected port-id to match an entry"); .expect("Collected port-id to match an entry");
match port_info { if !managed_port.pending {
ManagedMessagePort::Pending(port_impl, dom_port) => { panic!("Only pending ports should be found in to_be_added")
let new_port_info = ManagedMessagePort::Added(port_impl, dom_port);
let present = message_ports.insert(id, new_port_info);
assert!(present.is_none());
},
_ => panic!("Only pending ports should be found in to_be_added"),
} }
managed_port.pending = false;
} }
let _ = let _ =
self.script_to_constellation_chan() self.script_to_constellation_chan()
@ -869,18 +880,17 @@ impl GlobalScope {
{ {
let to_be_removed: Vec<MessagePortId> = message_ports let to_be_removed: Vec<MessagePortId> = message_ports
.iter() .iter()
.filter_map(|(id, port_info)| { .filter_map(|(id, ref managed_port)| {
if let ManagedMessagePort::Added(_port_impl, dom_port) = port_info { if managed_port.closed {
if dom_port.root().is_none() { // Let the constellation know to drop this port and the one it is entangled with,
// Let the constellation know to drop this port and the one it is entangled with, // and to forward this message to the script-process where the entangled is found.
// and to forward this message to the script-process where the entangled is found. let _ = self
let _ = self .script_to_constellation_chan()
.script_to_constellation_chan() .send(ScriptMsg::RemoveMessagePort(id.clone()));
.send(ScriptMsg::RemoveMessagePort(id.clone())); Some(id.clone())
return Some(id.clone()); } else {
} None
} }
None
}) })
.collect(); .collect();
for id in to_be_removed { for id in to_be_removed {
@ -940,7 +950,12 @@ impl GlobalScope {
// if they're not re-shipped in the current task. // if they're not re-shipped in the current task.
message_ports.insert( message_ports.insert(
dom_port.message_port_id().clone(), dom_port.message_port_id().clone(),
ManagedMessagePort::Pending(port_impl, WeakRef::new(dom_port)), ManagedMessagePort {
port_impl: Some(port_impl),
dom_port: Dom::from_ref(dom_port),
pending: true,
closed: false,
},
); );
// Queue a task to complete the transfer, // Queue a task to complete the transfer,
@ -958,7 +973,12 @@ impl GlobalScope {
let port_impl = MessagePortImpl::new(dom_port.message_port_id().clone()); let port_impl = MessagePortImpl::new(dom_port.message_port_id().clone());
message_ports.insert( message_ports.insert(
dom_port.message_port_id().clone(), dom_port.message_port_id().clone(),
ManagedMessagePort::Added(port_impl, WeakRef::new(dom_port)), ManagedMessagePort {
port_impl: Some(port_impl),
dom_port: Dom::from_ref(dom_port),
pending: false,
closed: false,
},
); );
let _ = self let _ = self
.script_to_constellation_chan() .script_to_constellation_chan()

View file

@ -1,6 +1,5 @@
[embedded-credentials.tentative.sub.html] [embedded-credentials.tentative.sub.html]
type: testharness type: testharness
expected: TIMEOUT
[Embedded credentials are treated as network errors.] [Embedded credentials are treated as network errors.]
expected: FAIL expected: FAIL
@ -11,11 +10,4 @@
expected: FAIL expected: FAIL
[Embedded credentials matching the top-level are treated as network errors for cross-origin URLs.] [Embedded credentials matching the top-level are treated as network errors for cross-origin URLs.]
expected: TIMEOUT expected: FAIL
[Embedded credentials matching the top-level are not treated as network errors for same-origin URLs.]
expected: TIMEOUT
[Embedded credentials matching the top-level are not treated as network errors for relative URLs.]
expected: TIMEOUT