Closed Bug 1735026 Opened 3 years ago Closed 2 years ago

Add mechanism for testing "cycle collection" of globals

Categories

(Core :: JavaScript: GC, task, P3)

task

Tracking

()

RESOLVED WONTFIX

People

(Reporter: sfink, Assigned: sfink)

References

Details

Attachments

(1 obsolete file)

I wanted to write a test case for bug 1717553 and found that I couldn't get it to work with the current facilities. The change is whether a gray map and black delegate mark the key black or gray. There are existing tests for the mark color, but I wanted to reproduce an actual memory leak fix by having something be collected that would not have been previously.

The challenge is that this requires the cycle collector to collect the gray key, which means having it as part of a cycle involving C++ objects. That, in turn, requires an outgoing edge to propoagate gray. But the delegate is black, so it has to be an edge that exists for the CCW key and not its delegate.

The only thing I know of that can fit the bill is the global -- a CCW will mark its global, but its delegate will not. (Well, it'll mark its own global.) So that means having a test case that allows the key to be collected, which means dropping the C++ -> key edge. This can almost be done with the existing grayRoot() mechanism, by removing the children of the grayRoot() to "cycle collect" them -- but this requires having access to the global, which means it has to be black.

So I need a mechanism to blow away grayRoot() entries without having access to either those entries or the grayRoot itself. (Which is the same; if you have the gray root, you can access the entries via properties.) And actually, the grayRoot() itself can reach the global, so if we're trying to drop the global we'll need to eliminate the grayRoot as well.

I propose a mechanism very close to what cycle collection does: the grayRoot() will remain to simulate outgoing edges from C++. It would probably be better to split it up into muultiple gray roots now, but never mind that. Then I'll add a set of "gray tails", which are JS objects simulating edges pointing into C++. The tails will be indirectly associated with the roots that are reachable via (simulated) paths through C++ objects. Then there is a command to do a "cycle collection", meaning any gray tail is assumed to be part of a CC cycle nad unlinked, which means its corresponding gray roots will no longer be traversed (they will be removed from the set of roots.)

It didn't come out as clean as I hoped, for the case of collecting the global, because of the path from the gray root singleton to the global. But it does at least reproduce the bugfix.

Having written this, I now realize that a vastly simpler approach would be to add a command that just stops tracing the gray root entirely. But unless I'm missing something that would make this useless for anything further, I rather like having something that mimics cycle collection this closely.

The severity field is not set for this bug.
:sdetar, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(sdetar)
Type: defect → task
Severity: -- → N/A
Flags: needinfo?(sdetar)

The following patch is waiting for review from an inactive reviewer:

ID Title Author Reviewer Status
D128016 Bug 1735026 - Add mechanism for testing "cycle collection" sfink jonco: Resigned from review

:sfink, could you please find another reviewer or abandon the patch if it is no longer relevant?

For more information, please visit auto_nag documentation.

Flags: needinfo?(sphink)
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Flags: needinfo?(sphink)
Resolution: --- → WONTFIX
Attachment #9245121 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: