Miscellaneous improvements to the nsMainThreadPtrHolder API

RESOLVED FIXED in mozilla22

Status

()

Core
XPCOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Bobby Holley (On Leave Until June 11th), Assigned: Bobby Holley (On Leave Until June 11th))

Tracking

unspecified
mozilla22
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

While working on switching necko over to this API, I ran into a few places where I wanted to massage the API a little bit. The most interesting one is the ability to opt out of the strict thread checking, for cases when consumers store different types of listeners in a single pointer, and sometimes access the pointer off-main-thread, but only in the non-js-implemented cases. We'll still catch all naughty access with the eventual hard assertions in XPCWrappedJS::{AddRef,Release}
Created attachment 723709 [details] [diff] [review]
Part 1 - Do some more work on nsMainThreadPtrHandle. v2
Attachment #723709 - Flags: review?(benjamin)
Created attachment 723710 [details] [diff] [review]
Part 2 - Add opt-out for strict checking in nsMainThreadPtrHolder. v3
Attachment #723710 - Flags: review?(benjamin)
Blocks: 850245

Comment 3

5 years ago
Comment on attachment 723709 [details] [diff] [review]
Part 1 - Do some more work on nsMainThreadPtrHandle. v2

+  // These are safe to call on other threads.

Maybe add "with external locking" so that it doesn't appear like this class is synchronizing the access automatically?
Attachment #723709 - Flags: review?(benjamin) → review+

Comment 4

5 years ago
Comment on attachment 723710 [details] [diff] [review]
Part 2 - Add opt-out for strict checking in nsMainThreadPtrHolder. v3

I feel like there's a programming error in the client code (in the client of DNSListenerProxy perhaps?). It feels like the objects here ought to be at least single-threaded, if perhaps not main-threaded. I almost feel like instead of strict, we ought to be storing the pointer to "the thread" and asserting that we're using the object on that thread.

But I'm marking r+ for now, and you said you'd file a followup ;-)
Attachment #723710 - Flags: review?(benjamin) → review+
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #4)
> But I'm marking r+ for now, and you said you'd file a followup ;-)

Filed bug 851564.
https://hg.mozilla.org/mozilla-central/rev/9d4a41ffdcb7
https://hg.mozilla.org/mozilla-central/rev/9ea11ddff33b
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.