850255 - Remove Off-Main-Thread XPCWrappedJS refcounting from nsConsoleService

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect)

Comment 1

[Bobby Holley (:bholley)]

Attachment: 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?).

Comment 2

[Benjamin Smedberg]

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?

Comment 3

[Bobby Holley (:bholley)]

(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?

Comment 4

[Bobby Holley (:bholley)]

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!