Cycle collect XPCWrappedNativeTearOff

RESOLVED INCOMPLETE

Status

()

RESOLVED INCOMPLETE
3 years ago
a year ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(firefox42 affected)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
There's a bunch of bizarre machinery around keeping tearoff natives alive. I think this is causing bug 1179782. I think we can get rid of it and make this behave like a normal class.

I think the steps involved are something like this:
- Make the reference from XPCWrappedNativeTearOffChunk to XPCWNTearOff a weak reference.
- Make XPCWNTearoff into a normal cycle collected class.
- Code using AutoMarkingWrappedNativeTearOffPtr just does an addref on the Tearoff on the stack instead of Mark().
- XPCCallContext holds a strong reference instead of calling Mark().
- The JS object for the tearoff holds a strong reference to the tearoff, like we do with other wrappers.
- SweepTearOffs just looks for chunks with null pointers and removes those chunks (I think right now those chunks just sit around forever?)

There is some increased overhead here, because a tearoff now needs to have a refcount, and we can't store the tearoff inline in the chunk, but we hopefully don't use these enough for it to matter much.
(Assignee)

Updated

3 years ago
Summary: Eliminate the weird marking of XPCWrappedNativeTearOff → Cycle collect XPCWrappedNativeTearOff
(Assignee)

Comment 1

3 years ago
(This wasn't the cause of bug 1179782.)
(Assignee)

Comment 2

3 years ago
Created attachment 8635531 [details] [diff] [review]
cycle collect XPCWrappedNativeTearOff. WIP

This seems to sort of work, except there are a lot of mochitest browser chrome timeouts. (plus some XPCShell red but whatever) Maybe this is just making things much slower? Anyways, I'm not going to look at this any more right now, as it doesn't seem to have any value besides code cleanup.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b583d118b13
(Assignee)

Comment 3

a year ago
I don't think it is worth wrangling with this class without a more specific goal.
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.