Actually that test program contradicts what I said --- my
IsDestructorPrivateOrDeleted produces exactly the same result as
!is_destructible, and is_destructible does return 0 for the class with
private destructor. So you could just use that!
Benoit
2014-05-28 16:51 GMT-04:00 Benoit Jacob <jacob.benoi...@gmail.com>:
> Awesome work!
>
> By the way, I just figured a way that you could static_assert so that at
> least on supporting C++11 compilers, we would automatically catch this.
>
> The basic C++11 tool here is std::is_destructible from <type_traits>, but
> it has a problem: it only returns false if the destructor is deleted, it
> doesn't return false if the destructor is private. However, the example
> below shows how we can still achieve what we want by using wrapping the
> class that we are interested in as a member of a helper templated struct:
>
>
>
> #include <type_traits>
> #include <iostream>
>
> class ClassWithDeletedDtor {
> ~ClassWithDeletedDtor() = delete;
> };
>
> class ClassWithPrivateDtor {
> ~ClassWithPrivateDtor() {}
> };
>
> class ClassWithPublicDtor {
> public:
> ~ClassWithPublicDtor() {}
> };
>
> template <typename T>
> class IsDestructorPrivateOrDeletedHelper {
> T x;
> };
>
> template <typename T>
> struct IsDestructorPrivateOrDeleted
> {
> static const bool value =
> !std::is_destructible<IsDestructorPrivateOrDeletedHelper<T>>::value;
> };
>
> int main() {
> #define PRINT(x) std::cerr << #x " = " << (x) << std::endl;
>
> PRINT(std::is_destructible<ClassWithDeletedDtor>::value);
> PRINT(std::is_destructible<ClassWithPrivateDtor>::value);
> PRINT(std::is_destructible<ClassWithPublicDtor>::value);
>
> std::cerr << std::endl;
>
> PRINT(IsDestructorPrivateOrDeleted<ClassWithDeletedDtor>::value);
> PRINT(IsDestructorPrivateOrDeleted<ClassWithPrivateDtor>::value);
> PRINT(IsDestructorPrivateOrDeleted<ClassWithPublicDtor>::value);
> }
>
>
> Output:
>
>
> std::is_destructible<ClassWithDeletedDtor>::value = 0
> std::is_destructible<ClassWithPrivateDtor>::value = 0
> std::is_destructible<ClassWithPublicDtor>::value = 1
>
> IsDestructorPrivateOrDeleted<ClassWithDeletedDtor>::value = 1
> IsDestructorPrivateOrDeleted<ClassWithPrivateDtor>::value = 1
> IsDestructorPrivateOrDeleted<ClassWithPublicDtor>::value = 0
>
>
> If you also want to require classes to be final, C++11 <type_traits> also
> has std::is_final for that.
>
> Cheers,
> Benoit
>
>
> 2014-05-28 16:24 GMT-04:00 Daniel Holbert <dholb...@mozilla.com>:
>
> Hi dev-platform,
>>
>> PSA: if you are adding a concrete class with AddRef/Release
>> implementations (e.g. via NS_INLINE_DECL_REFCOUNTING), please be aware
>> of the following best-practices:
>>
>> (a) Your class should have an explicitly-declared non-public
>> destructor. (should be 'private' or 'protected')
>>
>> (b) Your class should be labeled as MOZ_FINAL (or, see below).
>>
>>
>> WHY THIS IS A GOOD IDEA
>> =======================
>> We'd like to ensure that refcounted objects are *only* deleted via their
>> ::Release() methods. Otherwise, we're potentially susceptible to
>> double-free bugs.
>>
>> We can go a long way towards enforcing this rule at compile-time by
>> giving these classes non-public destructors. This prevents a whole
>> category of double-free bugs.
>>
>> In particular: if your class has a public destructor (the default), then
>> it's easy for you or someone else to accidentally declare an instance on
>> the stack or as a member-variable in another class, like so:
>> MyClass foo;
>> This is *extremely* dangerous. If any code wraps 'foo' in a nsRefPtr
>> (say, if some function that we pass 'foo' or '&foo' into declares a
>> nsRefPtr to it for some reason), then we'll get a double-free. The
>> object will be freed when the nsRefPtr goes out of scope, and then again
>> when the MyClass instance goes out of scope. But if we give MyClass a
>> non-public destructor, then it'll make it a compile error (in most code)
>> to declare a MyClass instance on the stack or as a member-variable. So
>> we'd catch this bug immediately, at compile-time.
>>
>> So, that explains why a non-public destructor is a good idea. But why
>> MOZ_FINAL? If your class isn't MOZ_FINAL, then that opens up another
>> route to trigger the same sort of bug -- someone can come along and add
>> a subclass, perhaps not realizing that they're subclassing a refcounted
>> class, and the subclass will (by default) have a public destructor,
>> which means then that anyone can declare
>> MySubclass foo;
>> and run into the exact same problem with the subclass. A MOZ_FINAL
>> annotation will prevent that by keeping people from naively adding
>> subclasses.
>>
>> BUT WHAT IF I NEED SUBCLASSES
>> =============================
>> First, if your class is abstract, then it shouldn't have AddRef/Release
>> implementations to begin with. Those belong on the concrete subclasses
>> -- not on your abstract base class.
>>
>> But if your class is concrete and refcounted and needs to have
>> subclasses, then:
>> - Your base class *and each of its subclasses* should have virtual,
>> protected destructors, to prevent the "MySubclass foo;" problem
>> mentioned above.
>> - Your subclasses themselves should also probably be declared as
>> "MOZ_FINAL", to keep someone from naively adding another subclass
>> without recognizing the above.
>> - Your subclasses should definitely *not* declare their own
>> AddRef/Release methods. (They should share the base class's methods &
>> refcount.)
>>
>> For more information, see
>> https://bugzilla.mozilla.org/show_bug.cgi?id=984786 , where I've fixed
>> this sort of thing in a bunch of existing classes. I definitely didn't
>> catch everything there, so please feel encouraged to continue this work
>> in other bugs. (And if you catch any cases that look like potential
>> double-frees, mark them as security-sensitive.)
>>
>> Thanks!
>> ~Daniel
>> _______________________________________________
>> dev-platform mailing list
>> dev-platform@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
>>
>
>
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
