If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Remove Off-Main-Thread XPCWrappedJS refcounting from nsConsoleService

RESOLVED WONTFIX

Status

()

Core
XPCOM
RESOLVED WONTFIX
5 years ago
5 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
Created attachment 723990 [details] [diff] [review]
Remove Off-Main-Thread XPCWrappedJS refcounting from nsConsoleService. v1

I had to change mListeners from an nsInterfaceHashtable with nsISupportsHashKey
to an nsRefPtrHashtable with an nsIConsoleListener* hash key. This has two
implications. First, we no longer have to canonicalize key pointers, which is
great because we're only allowing one interface type for keys. Second, we no
longer hold strong refs to our hash key, but this should also be fine because
we hold strong references to our hash value (why isn't this just a hashset
anyway?).
Attachment #723990 - Flags: review?(benjamin)

Comment 2

5 years ago
Hrm, this is technically breaking XPCOM rules, because you could have multiple tearoffs for the same root object. In practice I don't think that happens much any more, and probably never with console listeners as long as XPConnect guarantees that you can't have multiple tearoffs for the same interface. XPConnect caches tearoffs, right?

Updated

5 years ago
Flags: needinfo?(bobbyholley+bmo)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> Hrm, this is technically breaking XPCOM rules, because you could have
> multiple tearoffs for the same root object. In practice I don't think that
> happens much any more, and probably never with console listeners as long as
> XPConnect guarantees that you can't have multiple tearoffs for the same
> interface. XPConnect caches tearoffs, right?

I'm pretty sure it does, yeah. But all in all I'd like to be as safe as possible, especially because one of these dozen patches seemed to be causing winXP-only crashes on try that I couldn't reproduce locally.

So it seems like I could still just canonicalize the pointers. And maybe just convert the whole thing to a hashset while I'm at it. Does that sound good?
Flags: needinfo?(bobbyholley+bmo)
Comment on attachment 723990 [details] [diff] [review]
Remove Off-Main-Thread XPCWrappedJS refcounting from nsConsoleService. v1

As it turns out, this bug was obsoleted by bug 831428. Happy day!
Attachment #723990 - Flags: review?(benjamin)
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.