864313 - Event listeners/handlers should stop tracing their target if its compartment has been nuked

Status: RESOLVED INACTIVE

(Core :: XPConnect, defect)

Description

[Bobby Holley (:bholley)]

See the discussion in [1].

Basically, when we do Cu.nukeSandbox, we remove all the cross-compartment wrappers point into the sandbox, which generally allows the sandbox to go away. However, because we don't re-wrap event handlers/listeners into the same compartment as the target node (or at least won't bug 862629), we can still end up tracing the sandbox via the trace hook on the C++ EventListener object. This pegs the lifetime of the sandbox to the lifetime of the associated DOM.

I still think we shouldn't be rewrapping event listeners here. However, I have an alternative proposal to solve this issue:

1 - Add some state to the compartment private tracking whether the compartment has been nuked - that is to say, whether we want it to go away as soon as possible. This is necessary because nuking can happen between arbitrary source and destination compartments, and doesn't necessarily mean that the target needs to go away. We'd set this bit in Cu.nukeSandbox (and, later on, possibly in other places if we decide to expand hueyfix).

2 - Modify the trace hook of the C++ EventListener/EventHandler code to check for this bit on its underlying object, and if so, replace it with a DeadObjectProxy instead of tracing it.

Testing this should actually be pretty straightforward. We just have to set up the conditions for this to happen, and make sure that calling the event listener after the sandbox is nuked results in a DeadObjectProxy exception.

[1] https://groups.google.com/forum/?fromgroups=#!topic/mozilla.dev.platform/PBwUUrXDQa8

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Sounds bad if we need to have such special case for event listeners/handlers.
What about other callbacks?

Comment 2

[Bobby Holley (:bholley)]

(In reply to Olli Pettay [:smaug] from comment #1)
> Sounds bad if we need to have such special case for event listeners/handlers.
> What about other callbacks?

This logic would live in the generic Callback code, so I think it should be fine.

Comment 3

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.