Closed Bug 1174344 Opened 4 years ago Closed 4 years ago

Add a comment to the "bad size recorded" assertion to explain how to work around it

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox41 --- affected
firefox42 --- fixed

People

(Reporter: mccr8, Assigned: froydnj)

Details

Attachments

(1 file)

This assertion is not very helpful. We should add a comment to the effect that you shouldn't implement refcounting in a templated class, but rather a non-templated base class. Or something like that. It would also be nice if we could somehow statically check that but I'm not sure how.
Comment on attachment 8627361 [details] [diff] [review]
make error message for mismatched leak log entries more helpful

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

Thanks for fixing this!

::: xpcom/base/nsTraceRefcnt.cpp
@@ +425,5 @@
>        }
>      } else {
> +#ifdef DEBUG
> +      static const char kInconsistentSizesMessage[] =
> +        "Mismatched sizes were recorded in the memory leak logging table. "

Mismatched -> Inconsistent? You are being inconsistent here. ;)

@@ +429,5 @@
> +        "Mismatched sizes were recorded in the memory leak logging table. "
> +        "The usual cause of this is having a templated class that uses "
> +        "MOZ_COUNT_{C,D}TOR in the constructor or destructor, respectively. "
> +        "As a workaround, the MOZ_COUNT_{C,D}TOR calls can be moved to a "
> +        "(non-templated) base class.";

the () around non-templated seems unnecessary to me.

@@ +434,1 @@
>        NS_ASSERTION(aInstanceSize == 0 ||

I wonder if we should make this MOZ_ASSERT. Probably not for this bug, though, as I have a bad feeling about what might go wrong.
Attachment #8627361 - Flags: review?(continuation) → review+
Assignee: nobody → nfroyd
https://hg.mozilla.org/mozilla-central/rev/46d01eca4537
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.