Why is unique_ptr null?

Question

In the code snippet below, the assertion in foo always fires.

Can anyone explain why y is a nullptr? It looks like a lifetime issue, i.e. y is destroyed between the calls to put and get but I don't really understand why.

What am I missing?

TIA

class Y
{
public:
    Y(const std::string& name) : m_name(name)
    {
    }

    const std::string& getName() const
    {
        return m_name;
    }

private:
    Y(const Y&);

    const std::string m_name;
};

void foo(std::unique_ptr<Y> y)
{
    if (y == nullptr)
    {
        // ALWAYS FIRES
        assert(false && "nullptr\n");
    }

    std::string name = y->getName();
    std::cout << "name: " << name << "\n";
}

class X
{
public:
    X() {}

    void put(std::unique_ptr<Y> y)
    {
        m_queue.push([&] { foo(std::move(y)); });
    }

    void get()
    {
        std::function<void()> func = m_queue.front();
        func();
    }

private:
    std::queue<std::function<void()>> m_queue;
};


int main()
{
    std::unique_ptr<Y> y(new Y("fred"));
    X x;
    x.put(std::move(y));
    x.get();
    return 0;
}

Nawaz · Answer 1 · 2015-04-13T16:24:44.583

4

void put(std::unique_ptr<Y> y)
{
    m_queue.push([&] { foo(std::move(y)); });
}

In this function, y is a local variable which gets destroyed when it goes out of scope. The local variable is captured (by reference), by the lambda, doesn't exist by the time when it is executed — it is pointing to nothing/null/garbage/whatever, as the y has been already destroyed.

edited Apr 13 '15 at 16:24

answered Apr 13 '15 at 16:19

Nawaz

353,942
115
666
851

Your `push()` shouldn't compile - `function` must be copyable. – Barry Apr 13 '15 at 16:23

score 3 · Answer 2 · answered Apr 13 '15 at 17:40

Your problem is two fold. First, you are capturing by reference in a lambda whose lifetime (and the lifetime of its copies) exceeds the current local scope. Don't do that. Only use [&] if your lambda (and all copies) will not be copied out of the lifetime of the local scope.

The naive answer is to then do a [=] or [y], but you cannot copy a unique pointer.

In C++14, you can do [y=std::move(y)] which moves the y into the lambda capture. However, a lambda that has captured a unique_ptr by-value cannot be copied. And std::function can only store invokable, destroyable and copyable objects.

A solution to this would be to wait for a move-only function (which I think is coming down the pipe -- I've seen at least an informal proposal), or roll your own.

template<class Sig>
struct unique_function;
namespace details {
  template<class Sig>
  struct i_uf_impl;
  template<class R, class...Args>
  struct i_uf_impl<R(Args...)> {
    virtual ~i_uf_impl() {}
    virtual R invoke(Args&&...) = 0;
  };
  template<class Sig, class F>
  struct uf_impl;
  template<class R, class...Args>
  struct uf_impl<R(Args...):i_uf_impl<R(Args...)>{
    F f;
    virtual R invoke(Args&&...args) override final {
      return f(std::forward<Args>(args)...);
    }
  };
  template<class...Args>
  struct uf_impl<void(Args...):i_uf_impl<void(Args...)>{
    F f;
    virtual void invoke(Args&&...args) override final {
      f(std::forward<Args>(args)...);
    }
  };
}
template<class R, class...Args>
struct unique_function<R(Args...)> {
  std::unique_ptr<details::i_uf_impl<R(Args...)>> pimpl;
  unique_function(unique_function&&)=default;
  unique_function& operator=(unique_function&&)=default;
  unique_function()=default;
  template<class F, class=std::enable_if_t<
    !std::is_same<std::decay_t<F>, unique_function>
    && ( std::is_convertible<std::result_of_t< F(Args...) >, R >{}
      || std::is_same< R, void >{} )
  >>
  unique_function(F&& f):pimpl(
    new details::uf_impl<R(Args...), std::decay_t<F>>{std::forward<F>(f)}
  ) {}
  // override deduction helper:
  unique_function(R(*pfun)(Args...)):pimpl(
    pfun?
      new details::uf_impl<R(Args...), R(*)(Args...)>{pfun}
    : nullptr
  ) {}
  // null case
  unique_function(nullptr_t):unique_function(){}
  explicit bool operator() const { return static_cast<bool>(pimpl); }
  R operator()(Args...args)const{
    return pimpl->invoke( std::forward<Args>(args)... );
  }
};

which may or may not work, but should give you the gist.

Barry · Answer 3 · 2015-04-13T16:43:51.550

2

Because y gets destroyed:

void put(std::unique_ptr<Y> y)
{

    m_queue.push([&] {  // <== capturing y by-reference
        foo(std::move(y)); 
    });

    // <== y gets destroyed here
}

At the end of put(), y has already been cleaned up.

You'd want your functor to take ownership of y, which would ideally look something like:

[p = std::move(y)] {
    foo(std::move(p));
}

The lambda above has a member variable of type unique_ptr<Y>, so its copy constructor is implicitly deleted. But [func.wrap.func.con] stipulates that:

template<class F> function(F f);
template <class F, class A> function(allocator_arg_t, const A& a, F f);

Requires: F shall be CopyConstructible.

so this is won't compile either. Which leaves you somewhat stuck.

edited Apr 13 '15 at 16:43

answered Apr 13 '15 at 16:19

Barry

286,269
29
621
977

Can you explain the second part of your response please? i.e. the relevance of `std::function` needing to be copyable wrt to your example. – ksl Apr 13 '15 at 16:33
@ksl Added that explanation. – Barry Apr 13 '15 at 16:44

score 2 · Accepted Answer · answered Apr 13 '15 at 16:20

void put(std::unique_ptr<Y> y)
{
    m_queue.push([&] { foo(std::move(y)); });
}

Here, you push a lambda that contains a reference to the local variable y. The moment you leave put, the local variable is destroyed, and the lambda contains a dangling reference. Any further behavior is undefined.

You would need to capture the local variable by moving it into the lambda, but that is quite advanced, and also insufficient, because std::function cannot hold move-only function objects. The easiest way to solve this is to switch from unique_ptr to shared_ptr and capturing by value in the lambda.

Why is unique_ptr null?

4 Answers4