834769 - Change the "destroyed" state value for RefCounted

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Description

[Mike Hommey [:glandium]]

The current value is -0xdead, and when looking in a debugger, it can be confusing, because as a negative value, it will should up as -57005, and as hexadecimal, as 0xffff2153. This got me confused for a few in bug 834459 because i have had conflated values of ref count in the past because of a bug in the custom linker. If it had showed up as 0xffffdead, that would have been more obvious.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Change the "destroyed" state value for RefCounted

Comment 2

[Chris Peterson [:cpeterson]]

Comment on attachment 706454 [details] [diff] [review]
Change the "destroyed" state value for RefCounted

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

::: mfbt/RefPtr.h
@@ +46,5 @@
>      friend class RefPtr<T>;
>  
>    public:
>      RefCounted() : refCnt(0) { }
> +    ~RefCounted() { MOZ_ASSERT(refCnt == 0xffffdead); }

btw, you should probably make RefCounted's ctor and dtor protected. It doesn't make sense for someone to directly allocate or delete a RefCounted. Plus, RefCounted's dtor is not virtual, so derived classes' dtors would not be called (though a debug build would hit this assert).

Comment 3

[Jeff Walden [:Waldo]]

Comment on attachment 706454 [details] [diff] [review]
Change the "destroyed" state value for RefCounted

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

Looks good.  Good call on protecting the ctor/dtor, indeed.

Comment 4

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bc2bbe9836c7 (with ctor/dtor made protected)

Comment 5

[Mike Hommey [:glandium]]

Backed out. -Werror is making the tree red because of 0xffffdead being unsigned.
https://hg.mozilla.org/integration/mozilla-inbound/rev/954fc89373b8

Comment 6

[Mike Hommey [:glandium]]

Attachment: Change the "destroyed" state value for RefCounted.

Comment 7

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/799de1cf584b

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/799de1cf584b