Closed Bug 1359415 Opened 4 years ago Closed 4 years ago

move threadsafety checks inside nsAutoOwningThread

Categories

(Core :: XPCOM, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

Details

Attachments

(1 file)

This change moves most of the logic for the threadsafety check into
nsAutoOwningThread, rather than having part of the logic live in
nsAutoOwningThread and part of the logic live in nsDebug.h.  Changing
this also forces us to clean up a couple of places that replicated the
logic that lived in nsDebug.h as well.
I think this is an improvement, WDYT?
Attachment #8861419 - Flags: review?(erahm)
Comment on attachment 8861419 [details] [diff] [review]
move threadsafety checks inside nsAutoOwningThread

Review of attachment 8861419 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, this is a nice cleanup.

::: xpcom/base/nsISupportsImpl.cpp
@@ +30,5 @@
> +#ifdef MOZ_THREAD_SAFETY_OWNERSHIP_CHECKS_SUPPORTED
> +void
> +nsAutoOwningThread::AssertCurrentThreadOwnsMe(const char* msg) const
> +{
> +  if (mThread != PR_GetCurrentThread()) {

Should we MOZ_UNLIKELY this?

::: xpcom/base/nsISupportsImpl.h
@@ +57,5 @@
>  public:
>    nsAutoOwningThread() { mThread = PR_GetCurrentThread(); }
> +
> +  template<int N>
> +  void AssertOwnership(const char (&aMsg)[N]) const

Maybe a comment here that we did it this way to guarantee a string literal is used.
Attachment #8861419 - Flags: review?(erahm) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8457e1358dda
move threadsafety checks inside nsAutoOwningThread; r=erahm
https://hg.mozilla.org/mozilla-central/rev/8457e1358dda
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.