17

I am creating a C++ server application using standalone Asio and C++11 and am getting an error, which is why I am asking for help.

The error

In the class worker_thread, during the call to shared_from_this(), a bad_weak_ptr exception is raised, which causes the program to crash.

The layout

  1. The class connection_manager creates and stores objects of type std::shared_ptr<worker_thread> inside a std::vector container
  2. The class worker_thread inherits from std::enable_shared_from_this<worker_thread>.
  3. The class worker_thread creates objects of type std::shared_ptr<connection>.
  4. The class connection requires a pointer (which is a shared pointer) to the class worker_thread, so that in can call the void handle_finish(std::shared_ptr<connection>)

Program flow

  1. The class worker_thread is created via its constructor, from the class connection_manager using std::make_shared<worker_thread> with two shared pointers as parameters.
  2. void init() is called from worker_thread by connection_manager
  3. Later in the program, connection_manager calls std::shared_ptr<connection> get_available_connection() from worker_thread
  4. During this method's execution, a new connection is created via std::make_shared<connection>, and one of the arguments is the shared pointer to the current worker_thread obtained via shared_from_this()
  5. During the shared_from_this() call, the program crashes with a bad_weak_ptr exception.

Research

From my research, the most common causes of this error are:

  1. When shared_from_this() is called within a constructor (or a function which is called by the constructor)
  2. When there is no existing std::shared_ptr pointing to the object.

In my program:

  1. The call to the constructor and the get_available_connection() are separate, and through outputing lines in the terminal, it seems that the worker_thread is constructed and initialised by the time the call to get_available_connection() occurs
  2. The connection_manager class holds a shared pointer to every worker_thread object.

Code

All something_ptr are std::shared_ptr<something>

Header files

connection_manager.hpp

typedef asio::executor_work_guard<asio::io_context::executor_type>
    io_context_work;
std::vector<worker_thread_ptr> workers;
std::vector<io_context_ptr> io_contexts;
std::vector<io_context_work> work;

worker_thread.hpp

class worker_thread : std::enable_shared_from_this<worker_thread> {
public:

/// Create a worker thread.
explicit worker_thread(io_context_ptr io, config_ptr vars_global);

void init();
void join();

connection_ptr get_available_connection();
//...

connection.hpp

explicit connection(std::shared_ptr<worker_thread> worker,
            std::shared_ptr<asio::io_context> io, 
            config_ptr vars_parent);

Source files

connection_manager.cpp

connection_manager::connection_manager(config_ptr vars) {
    std::size_t number_of_threads = vars->worker_threads;
    while(number_of_threads > 0) {
        io_context_ptr io_context(new asio::io_context);
        io_contexts.push_back(io_context);
        work.push_back(asio::make_work_guard(*io_context));

        worker_thread_ptr worker =
            std::make_shared<worker_thread>(io_context, vars);
        workers.push_back(worker);

        worker->init();

        --number_of_threads;
    }   
} 

connection_ptr connection_manager::get_available_connection() {
    std::size_t index_of_min_thread = 0;
    std::size_t worker_count = workers.size();
    for(std::size_t i = 1; i < worker_count; ++i) {
        if(workers[i]->active_connection_count() <
                workers[index_of_min_thread]->active_connection_count())
            index_of_min_thread = i;
    }
    return workers[index_of_min_thread]->get_available_connection();
}

worker_thread.cpp

worker_thread::worker_thread(io_context_ptr io, 
        config_ptr vars_global)
    :io_context(io), active_conn_count(0), vars(vars_global),
    worker(
        [this]() {
            if(io_context)
                io_context->run();
        }   
    ) {}

void worker_thread::init() {
    //Additional initialisation, this is called by connection_manager
    //after this thread's construction
}   

connection_ptr worker_thread::get_available_connection() {
    connection_ptr conn;
    if(!available_connections.empty()) {
        conn = available_connections.front();
        available_connections.pop();
        active_connections.insert(conn);
        return conn;
    } else {
        conn = std::make_shared<connection>(shared_from_this(), io_context, vars);
        active_connections.insert(conn);
        return conn;
    }
}

I am sorry if this question has been answered before, but I tried to resolve this, and after trying for some time, I decided it would be better to ask for help.

EDIT Here is a minimum test, which fails. It requires CMake, and you might have to change the minimum required version.

Google Drive link

sehe
  • 374,641
  • 47
  • 450
  • 633
Petar Nikić
  • 181
  • 1
  • 5
  • Please take some time to read [How to create a Minimal, Complete and Verifiable Example](https://stackoverflow.com/help/mcve). – gflegar May 13 '18 at 17:08
  • i do not see any of the commen mistakes. did you check that you do not break anything because of multi-threaded access? (use locks where necessary), sometimes this errors have their cause in some memory corruption or are follow-up of some "undefined behaviour" operation – skeller May 13 '18 at 17:19
  • While the program is multithreaded, the threads which are created are doing nothing until the `get_available_connection()` call arrives from the main thread, which is why I believe that this is not the problem. Nevertheless, I will check. – Petar Nikić May 13 '18 at 17:26
  • What's the stacktrace? Better yet, try using the debugger! – CygnusX1 May 13 '18 at 17:29
  • #9 0x000055555557b8e7 in std::shared_ptr::shared_ptr (this=0x7fffffffe560, __r=std::weak_ptr (empty) = {...})#10 0x000055555557b535 in std::enable_shared_from_this::shared_from_this (this=0x5555557ad790) #11 in musync::server::worker_thread::get_available_connection (this=0x5555557ad790) #12 in musync::server::connection_manager::get_available_connection #13 in musync::server::server::do_accept #14 in musync::server::server::init – Petar Nikić May 13 '18 at 17:35
  • You need to *publicly* inherit from [`enable_shared_from_this`](https://en.cppreference.com/w/cpp/memory/enable_shared_from_this) otherwise the `weak_ptr` that `enable_shared_from_this` holds is never initialized. – Sean Cline May 13 '18 at 19:28
  • @SeanCline isn't that what this line does `class worker_thread : std::enable_shared_from_this` – Petar Nikić May 13 '18 at 19:34
  • 2
    @PetarNikić `class`es inherit *privately* by default. See [this answer](https://stackoverflow.com/a/4797014/1020072). – Sean Cline May 13 '18 at 19:36
  • @SeanCline can you please write it a little bit more formally as an answer so that I can mark it as the answer? You've helped me! Thank you! – Petar Nikić May 13 '18 at 20:24
  • 2
    Glad that was it. Looks like I've been beaten to the punch on the answer. @NinaKaprez's answer covers it. – Sean Cline May 13 '18 at 21:47
  • 1
    Reworded the title to make this an excellent search hit for this recurring pitfall. (I saw it before https://stackoverflow.com/questions/47916281/shared-from-this-throws-bad-weak-ptr-with-boostasio/47929479#47929479 but it took too long for me to realize it here, again). This stuff is what [SO] is made for – sehe May 14 '18 at 00:16
  • Have you considered using a plain `weak_ptr` member that you initialize yourself instead of that automagic `enable_shared_from_this`? – curiousguy May 14 '18 at 04:25

1 Answers1

54

I think your problem might be that you use default private inheritance.

here is a simple example of a program that crashes:

class GoodUsage : public std::enable_shared_from_this<GoodUsage>
{
public:
    void DoSomething()
    {
        auto good = shared_from_this();
    }
};

class BadUsage : std::enable_shared_from_this<BadUsage> // private inheritance
{
public:
    void DoSomething()
    {
        auto bad = shared_from_this();
    }
};


int main()
{
    auto good = std::make_shared<GoodUsage>();
    auto bad = std::make_shared<BadUsage>();
    good->DoSomething(); // ok
    bad->DoSomething(); // throws std::bad_weak_ptr    
}
Elad Maimoni
  • 3,703
  • 3
  • 20
  • 37