Does it make sense to check the nullptr value in the user-shared_ptr delete?

Question

I saw code that uses std::shared_ptr with user deletion that checks the argument for nullptr, like MyClass , which has a close() method and is built with some CreateMyClass :

 auto pMyClass = std::shared_ptr<MyClass>(CreateMyClass(), [](MyClass* ptr) { if(ptr) ptr->close(); }); 

Does it make sense to test ptr for uncertainty in deleter? Could this happen? as?

Answer 1

The constructor std::shared_ptr<T>::shared_ptr(Y*p) has a requirement that delete p be a valid operation. This is a valid operation when p is nullptr .

The constructor std::shared_ptr<T>::shared_ptr(Y*p, Del del) has the requirement that del(p) be a valid operation.

If your custom sender cannot handle p equal to nullptr , then passing null p in the shared_ptr constructor is not acceptable.

The constructor that you offer as an example can be better represented in this way:

 #include <memory> struct MyClass { void open() { // note - may throw }; void close() noexcept { // pre - is open } }; struct Closer { void operator()(MyClass* p) const noexcept { p->close(); delete p; // or return to pool, etc } }; auto CreateMyClass() -> std::unique_ptr<MyClass, Closer> { // first construct with normal deleter auto p1 = std::make_unique<MyClass>(); // in case this throws an exception. p1->open(); // now it open, we need a more comprehensive deleter auto p = std::unique_ptr<MyClass, Closer> { p1.release(), Closer() }; return p; } int main() { auto sp = std::shared_ptr<MyClass>(CreateMyClass()); } 

Note that now shared_ptr cannot have a null object.

Answer 2

Yes, that actually makes sense. Suppose CreateMyClass returns nullptr . The pMyClass ( use_count ) reference counter becomes 1 . When pMyClass is destroyed, the following will happen :

If *this belongs to an object, and it is the last shared_ptr that owns it, the object is destroyed through its department.

Therefore, if a user selector dereferences a pointer that is stored by shared_ptr ( ptr->close() in your code), then it should take care of checking for nullptr.

Note that empty shared_ptr does not match null shared_ptr .

Answer 3

 struct deleter { template<class T> void operator()(T*) const { std::cout << "deleter run\n"; } }; int main() { std::shared_ptr<int> bob((int*)0, deleter{}); } 

Living example .

"deleter run\n" . The divider is really running.

The concept of empty and the concept of nullptr membership are different concepts for shared_ptr .

bob is not empty, but bob.get()==nullptr . When non-empty, the destructor is called.

 int main() { int x; std::shared_ptr<int> alice( std::shared_ptr<int>{}, &x ); } 

alice empty, but alice.get() != nullptr . When alice goes out of scope, delete &x does not start (and in fact the destructor does not start).

This can be avoided if you never create your shared pointer with a null pointer and delete.

One way to approach this is to first create a unique pointer with user deletion.

 template<class Deleter, class T> std::unique_ptr<T, Deleter> change_deleter( std::unique_ptr<T> up, Deleter&& deleter={} ) { return {up.release(), std::forward<Deleter>(deleter)}; } struct close_and_delete_foo; // closes and deletes a foo std::unique_ptr<foo, close_and_delete_foo> make_foo() { auto foo = std::make_unique<foo>(); if (!foo->open()) return {}; return change_deleter<close_and_delete_foo>(std::move(foo)); } 

Unlike shared_ptr , unique_ptr cannot contain nullptr but be nonempty (the standard does not use the term empty for unique_ptr , instead it talks about .get()==nullptr ).

unique_ptr can be implicitly converted to shared_ptr . If it has nullptr , the resulting shared_ptr empty, and not just holding nullptr . The unique_ptr migrates to shared_ptr .

---

The disadvantage of all these methods is that the shared_ptr count memory block is a separate allocation of the object's memory block. Two distributions are worse than one.

But the make_shared constructor make_shared not allow you to execute a custom div.

If destroying your object is not possible, you can use the alias constructor to be extremely careful:

 // empty base optimization enabled: template<class T, class D> struct special_destroyed:D { std::optional<T> t; template<class...Ds> special_destroyed( Ds&&...ds ): D(std::forward<Ds>(ds)...) {} ~special_destroyed() { if (t) (*this)(std::addressof(*t)); } }; std::shared_ptr<MyClass> make_myclass() { auto r = std::make_shared< special_destroyed<MyClass, CloseMyClass> >(); r->t.emplace(); try { if (!r->t->open()) return {}; } catch(...) { r->t = std::nullopt; throw; } return {r, std::addressof(*rt)}; } 

Here we will be able to use one block for counting the destroyer and counting links, allowing, perhaps, working with the error open and automatically closing only when the data is actually there.

Note that the destroyer should only close MyClass and not delete it; deletion occurs using an external destroyer in make_shared wrapping special_destroyed .

This uses C ++ 17 for std::optional , but an alternative optional is available from boost and elsewhere.

---

The original C ++ 14 solution. We create a crude optional :

 template<class T, class D> struct special_delete:D { using storage = typename std::aligned_storage<sizeof(T), alignof(T)>::type; storage data; bool b_created = false; template<class...Ts> void emplace(Ts&&...ts) { ::new( (void*)&data ) T(std::forward<Ts>(ts)...); b_created=true; } template<std::size_t...Is, class Tuple> void emplace_from_tuple( std::index_sequence<Is...>, Tuple&&tup ) { return emplace( std::get<Is>(std::forward<Tuple>(tup))... ); } T* get() { if (b_created) return reinterpret_cast<T*>(&data); else return nullptr; } template<class...Ds> special_delete(Ds&&...ds):D(std::forward<Ds>(ds)...){} ~special_delete() { if (b_created) { (*this)( get() ); get()->~T(); } } }; struct do_nothing { template<class...Ts> void operator()(Ts&&...)const{} }; template<class T, class D, class F=do_nothing, class Tuple=std::tuple<>, class...Ds> std::shared_ptr<T> make_special_delete( F&& f={}, Tuple&& args=std::tuple<>(), Ds&&...ds ) { auto r = std::make_shared<special_delete<T,D>>(std::forward<Ds>(ds)...); r->emplace_from_tuple( std::make_index_sequence< std::tuple_size<std::remove_reference_t<Tuple>>::value >{}, std::move(args) ); try { f(*r->get()); } catch(...) { r->b_created = false; r->get()->~T(); throw; } return {r, r->get()}; } 

This is probably too far. Fortunately, our extremely limited optional can be written easier than the real optional , but I'm not sure everything worked out fine.

Living example .

For C ++ 11, a manual entry is required for make_index_sequence , etc.