move threadsafety checks inside nsAutoOwningThread

RESOLVED FIXED in Firefox 55

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

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: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.