Could please someone explain to me how did I end up in a dead lock situation below:

This is a reprensatative example of what I was doing.

#include <unordered_map>
#include <memory>
#include <tbb/tbb.h>
class node_maker
{
public:
    using node = std::shared_ptr<tbb::flow::function_node<double, double, tbb::flow::rejecting>>;

    node_maker()
    {
        m_graph = std::make_shared<tbb::flow::graph>();
    }
    std::size_t make_node(const std::size_t id)
    {
        auto op = [](const double d)
        {
            return d   1;
        };

        node n = std::make_shared<tbb::flow::function_node<double, double, tbb::flow::rejecting>>(*m_graph, 1, op, tbb::flow::rejecting(), 1);
        m_cache[id] = n;
        return id;
    };

private:
    std::unordered_map<std::size_t, node> m_cache;
    std::shared_ptr<tbb::flow::graph> m_graph;
};

int main(int argc, char* argv[])
{
    node_maker maker;
    std::size_t n = maker.make_node(1);

    return 0;
};

Is this a bug in my code or TBB code please? The file where the dealock happen is: include\oneapi\tbb\flow_graph.h specifically when calling:

inline void graph::remove_node(graph_node *n) {
    {
        spin_mutex::scoped_lock lock(nodelist_mutex);
        __TBB_ASSERT(my_nodes && my_nodes_last, "graph::remove_node: Error: no registered nodes");
        if (n->prev) n->prev->next = n->next;
        if (n->next) n->next->prev = n->prev;
        if (my_nodes_last == n) my_nodes_last = n->prev;
        if (my_nodes == n) my_nodes = n->next;
    }
    n->prev = n->next = nullptr;
}

When running in debug mode, the constructor works fine and it goes here include\oneapi\tbb\flow_graph.h in the constructor below:

    //! Constructor
    // input_queue_type is allocated here, but destroyed in the function_input_base.
    // TODO: pass the graph_buffer_policy to the function_input_base so it can all
    // be done in one place.  This would be an interface-breaking change.
    template< typename Body >
        __TBB_requires(function_node_body<Body, Input, Output>)
     __TBB_NOINLINE_SYM function_node( graph &g, size_t concurrency,
                   Body body, Policy = Policy(), node_priority_t a_priority = no_priority )
        : graph_node(g), input_impl_type(g, concurrency, body, a_priority),
          fOutput_type(g) {
        fgt_node_with_body( CODEPTR(), FLOW_FUNCTION_NODE, &this->my_graph,
                static_cast<receiver<input_type> *>(this), static_cast<sender<output_type> *>(this), this->my_body );
    }

THIS IS ONLY HAPPENING IN DEBUG MODE. I AM USING C 20 64 VS

CodePudding user response：

Here, I think the caching is the issue, when the graph is destructed, the dead lock happen, when I take this line off: m_cache.insert({ id, n }); it works fine:

    std::size_t make_node(const std::size_t id)
    {
        auto op = [](const double d)
        {
            return d   1;
        };

        node n = std::make_shared<tbb::flow::function_node<double, double, tbb::flow::rejecting>>(*m_graph, 1, op, 1);
        m_cache.insert({ id, n });
        return id;
    };

This is not a response, but just to point out where the error is as the question already has more than 50 views. Please if you figure something out share :)

CodePudding user response：

This is solved, I am quoting the answer from Github:

" I believe it is the order of the destructor calls that lead to the deadlock. When maker is destroyed, its m_graph is destroyed first and then its m_cache. This results in the graph object being destroyed before the node that belongs to the graph. When finally the node is destroyed it tries to deregister itself from the graph that has already been destroy. I think if you just swap the order of the objects in your class definition, they will be destroyed in a safe order, nodes before graph. "

By vossmjp. Thanks too al who have tried offline.