0

I worked with a class with unique_ptr pointing at object and method set it in runetime asynchroniously to nullptr and then another method might call make_unique and set mentioned pointer to this new object.

void Class0::f0()
{
    mPtr = std::make_unique<Class1>();
}

void Class0::f1(SomeType param)
{
    mPtr->doWork(param);
    mPtr      = nullptr; 
}

//f1 called after f0 asynchroniously, any number of times

Who and when deletes this previous that was not explicitly deleted? The unique_ptr is still alive since it's a class field so it's destructor is never called, but unique_ptr can be set to nullptr and make_unique can be called many times. I was almost sure it will cause a memory leak, and mPtr.reset() must be explicitly called first. But I've made small test in visual studio c++, that causes no leak.

void f()
{
    std::unique_ptr<std::vector<std::vector<int>>> ptr = std::make_unique<std::vector<std::vector<int>>>(100000);
    ptr = nullptr;
    f();
}

recursion only to check memory usage with and without ptr = nullptr; I've tried WSL g++ with -O0 with the very same result. Can someone explain it please?

Marcin Szeremeta
  • 5
  • 2
  • 3
    see here: https://en.cppreference.com/w/cpp/memory/unique_ptr/operator%3D its overload (3). – 463035818_is_not_an_ai Jul 28 '21 at 22:00
  • 2
    If a smart pointer couldn't handle assignements it wouldn't be very smart. – super Jul 28 '21 at 22:02
  • Thanks a lot: std::unique_ptr::operator= unique_ptr& operator=( std::nullptr_t ) noexcept; 3) Effectively the same as calling reset(). – Marcin Szeremeta Jul 28 '21 at 22:03
  • *"I was almost sure it will cause a memory leak"* - what led you to that conclusion, besides guessing? I suspect there is more going on with the *real* code than the ultra-simplicity of what is shown here, especially after you threw in the phrase "..set it in runetime asynchroniously to nullptr". That hints at potentially concurrent access to that member variable by one or more `f0` and `f1` calls running on independent threads, and without proper concurrency protection you're playing with fire. – WhozCraig Jul 28 '21 at 22:07
  • This is embedded in multi-processor environment, but access to those parts I refereed is safe, they are just part of (not so much) simple observer pattern. Well I suppose I didn't see ptr = nullptr; before and "Explicit always better than implicit" was something what turned the light on and said, "hmm it seems wrong", but it just seems as an example of code not so well explaining itself :) – Marcin Szeremeta Jul 28 '21 at 22:16

1 Answers1

3

Who and when deletes this previous that was not explicitly deleted?

Not explicitly deleting the object managed by the smart pointer is the reason to use a smart pointer in the first place. It takes more than deleting the object in the unique_ptrs destructor to properly manage the lifetime of the object (see What is The Rule of Three?). It is rather safe to assume that the smart pointer manages the lifetime correctly unless you do something weird.

Here ptr = nullptr; is overload (3) from cppreference/unique_ptr/operator=:

unique_ptr& operator=( std::nullptr_t ) noexcept;     (3)

Effectively the same as calling reset().

And reset():

void reset( pointer ptr = pointer() ) noexcept;

Replaces the managed object.

Given current_ptr, the pointer that was managed by *this, performs the following actions, in this order:

  • Saves a copy of the current pointer old_ptr = current_ptr
  • Overwrites the current pointer with the argument current_ptr = ptr
  • If the old pointer was non-empty, deletes the previously managed object
    if(old_ptr) get_deleter()(old_ptr).
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185
  • 1
    The same thing goes with all of the overloads of `unique_ptr::operator=`. When assigned *any* new value, the `unique_ptr` that is being assigned to will destroy its current object and then take ownership of the new object that is being assigned. – Remy Lebeau Jul 28 '21 at 22:21
  • @RemyLebeau The exception being reassigning the owned pointer again? Perhaps that causes UB. – Ted Lyngmo Jul 28 '21 at 22:30
  • 1
    @TedLyngmo assigning a `unique_ptr` back to itself is safe, actually. It is essentially a `reset(release())` operation, so the current pointer will be released first so there is nothing for `reset()` to destroy, and then it re-acquires the original pointer. – Remy Lebeau Jul 28 '21 at 22:35