Possible leak because the cycle collector doesn't understand debugger weak edges




JavaScript Engine
7 years ago
4 years ago


(Reporter: billm, Unassigned)



Firefox Tracking Flags

(Not tracked)


After Andrew found bug 693527, it got me thinking that debugger objects might also be involved in leaks. The marking logic for them is quite complex. To me, it looks like it might be possible.

As an example, maybe a DOM object could point to a Debugger object. Then the debugger object could have a breakpoint, and the handler for the breakpoint could point back to the DOM object. The GC will not collect this cycle because it pretty much treats the pointers from DOM as roots. The cycle collector will not collect this cycle because it doesn't know about the any of the weak edges caused by the debugger and the breakpoint.

Andrew and I worked out a way that we might be able to inform the cycle collector of these edges, but it would be a fair amount of work. Jason and Jim, does this seem like something that could happen? And if it could, how worried should we be?

Comment 1

7 years ago
Can the debugger use weak maps to make sure the cycles can be broken?
Debugger objects seem to have weird lifetimes with side conditions like "keep this object alive if we're currently executing its associated breakpoint", so I'm not sure weak maps can be directly used.  The debugger is already using weak maps in some places, so I assume these non-weakmaps are there for a reason. ;)
OS: Linux → All
Hardware: x86_64 → All
Version: unspecified → Trunk
Here's another scenario that's fairly realistic and clearly leaks.

Chrome script creates a Debugger object, obtains a Debug.Object referring to a content DOM object with an event handler, and sets a breakpoint in the event handler script. The breakpoint handler has a reference to the Debug.Object.

Now we have:

  DOM object -> event handler -> script -> breakpoint -> breakpoint handler
  -> Debug.Object -> DOM object

The edge "script -> breakpoint" is drawn by Debugger::markAllIteratively. :-P

Just chatted with billm and we have about a third of a hand-wavy plan to replace that edge with a WeakMap. It could work maybe.
To elaborate on what Jason said, we're thinking of simplifying Debugger::hasAnyLiveHooks so that it just returns enabled.

Comment 5

7 years ago
It seems to me like hooks of an enabled debugger should be handled as strong references from the debuggees (for onNewScript, onThrow, etc.) or frames (for onPop) or Objects (for watchpoints).

The issue here is that these edges are conditional on the debugger's 'enabled' flag. It might not be bad for clearing 'enabled' to actually go out and delete a bunch of incoming edges, and setting 'enabled' to go and stick 'em back in. It seems like that might simplify a lot of stuff (other than the 'enabled' getter and setter).

But if that works, then I think everything could become either an ordinary edge or a WeakMap instance.


4 years ago
Assignee: general → nobody
You need to log in before you can comment on or make changes to this bug.