Home > Software engineering >  Probable bug in weak_ptr passed to a thread
Probable bug in weak_ptr passed to a thread

Time:11-08

In this code example, we pass a std::shared_ptr<int> to a thread, and expect it is weakened to a std::weak_ptr<int>. But in Visual Studio 2019, t takes a strong reference and the output of this program is "pw alive", in both debug and release builds. Same problem on linux with compiler g -lpthread.

This toy example is motivated by the more serious case where thread t does some long computations, and then sends results through shared resources t receives as arguments. Those resources may be deleted by other threads during t's computation, in which case t becomes useless and can be terminated early. But because of this (suspected) bug, t keeps the resources alive and wastes memory and calculation time.

Is this really a bug, or I am misunderstanding the relationship between weak_ptr and shared_ptr ? If this is a bug, do you know where to report it ? Probably at different places for Visual Studio and g .

#include <iostream>
#include <string>
#include <thread>
#include <chrono>

void TestWeakPtrAlive(std::weak_ptr<int> pw)
{
  // wait for main thread to delete pw
  std::this_thread::sleep_for(std::chrono::milliseconds(1000));
  std::shared_ptr<int> p = pw.lock();
  std::cout << (p ? "pw alive" : "pw dead")
            << std::endl;
}

int main()
{
  std::shared_ptr<int> p = std::make_shared<int>(18);
  std::thread t(TestWeakPtrAlive, p);
  p = nullptr;
  t.join();
  return 0;
}

CodePudding user response:

Those implementations copy the shared_ptr. i.e. They copy the argument and tie it to the lifetime of the thread. Copying is the default for argument binding with thread creation.

This wouldn't be considered a bug unless the standard said the implementation couldn't do that.

As far as getting the behavior you want, I think you'd have the same problem if you used a lambda. Instead you could use a functor, or you could use some global or static variables.

Here's your example converted to use a functor. The benefit is that you control the member variables. The downside is the boilerplate.

#include <iostream>
#include <string>
#include <thread>
#include <chrono>

class TestWeakPtrAlive
{
    std::weak_ptr<int> m_pw;

public:
    TestWeakPtrAlive(std::weak_ptr<int> pw)
        : m_pw(std::move(pw))
    {}
  
    void operator()()
    {
        // wait for main thread to delete pw
        std::this_thread::sleep_for(std::chrono::milliseconds(1000));
        std::shared_ptr<int> p = m_pw.lock();
        std::cout << (p ? "pw alive" : "pw dead")
                  << std::endl;
    }
};

int main()
{
    std::shared_ptr<int> p = std::make_shared<int>(18);
    auto functor = TestWeakPtrAlive(p);
    std::thread t(std::move(functor));
    p = nullptr;
    t.join();
    return 0;
}

Edit: I just thought of a fairly obvious solution which would be to just create a weak_ptr and pass that to the thread constructor.

#include <iostream>
#include <string>
#include <thread>
#include <chrono>

void TestWeakPtrAlive(std::weak_ptr<int> pw)
{
    // wait for main thread to delete pw
    std::this_thread::sleep_for(std::chrono::milliseconds(1000));
    std::shared_ptr<int> p = pw.lock();
    std::cout << (p ? "pw alive" : "pw dead")
        << std::endl;
}

int main()
{
    std::shared_ptr<int> p = std::make_shared<int>(18);
    std::weak_ptr<int> wp = p;
    std::thread t(TestWeakPtrAlive, std::move(wp));
    p = nullptr;
    t.join();
    return 0;
}
  • Related