11

We recently came across a crash when moving from a unique_ptr to a shared_ptr, using a custom deleter. The crash happened when the pointer used to create the smart pointer was null. Below is code that reproduces the problem, and shows two cases that work.

In the source below, One and Two run happily, while Three crashes in "ReleaseDestroy". The crash appears to be happening when the class being used in the smart pointer has a virtual "Release" and so the program is trying to look up the V-Table. unique_ptr looks like it checks for null pointers and doesn't run the destructor. Shared pointer seems to neglect this.

Does anyone know if this is by design, or is it a bug in the stl implementation? We are using Visual Studio 2015.

#include <iostream>
#include <memory>

template<class R>
void ReleaseDestroy(R* r)
{
    r->Release();
};

class FlatDestroy
{
public :
    void Release()
    {
        delete this;
    }
};

class VirtualDestroy
{
public:
    virtual void Release()
    {
        delete this;
    }
};

class SimpleOne
{
public :
};

void main()
{
    std::shared_ptr<SimpleOne> One(nullptr);
    std::shared_ptr<FlatDestroy> Two(nullptr, ReleaseDestroy<FlatDestroy>);
    std::shared_ptr<VirtualDestroy> Three(nullptr, ReleaseDestroy<VirtualDestroy>);

    One.reset();
    Two.reset();
    Three.reset();
}
Simon Parker
  • 1,656
  • 15
  • 37

1 Answers1

16

The destruction behaviours of unique_ptr and shared_ptr are different:

  • unique_ptr only calls the deleter if its held pointer is non-null.

  • shared_ptr always calls the deleter.

So your shared pointer deleter must be able to deal with null pointer values! For example, std::free is fine, but std::fclose is not, and neither is your deleter (since it dereferences r unconditionally).

Incidentally, this crops up in LWG 2415, which addresses constructing a shared pointer from a unique pointer, which was broken before that defect resolution for exactly that reason. (I hit this problem myself here; note how that code is careful to distinguish the null case for shared_ptr.)

Community
  • 1
  • 1
Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • *"since it dereferences r unconditionally"*: so the third case crashed because it tried to indirect through a "non-existing" vtable, while in the second case no such table was supposed to be there, is that right? – A.S.H Mar 14 '17 at 02:27
  • 1
    @A.S.H: Both cases are derefencing a null pointer, which results in undefined behaviour. – Kerrek SB Mar 14 '17 at 02:30
  • The experiment shows that are different (at least in Visual C++ 2015), but why? Is this part of the standard? Is there a reason for it? What is the rational for making these different? The real kicker is that if you transfer the pointer from the unique_ptr to the shared_ptr, and it is null, the shared_ptr now crashes, even when they use the same custom destructor. – Simon Parker Mar 14 '17 at 22:31
  • Having read through your reference, LWG 2415, it looks like the problem is a defect by design. – Simon Parker Mar 14 '17 at 22:34
  • @SimonParker: the construction of shared from unique was certainly defective until that was fixed. The different behaviour may be intentional; bear in mind that the shared pointer type-erases the deleter, so that calling it unconditionally affords a little extra flexibility. But I haven't looked at the original design rationale. – Kerrek SB Mar 14 '17 at 23:03