Avoid SIGTRAP when the constructor of a network edge throws

Question

Background

I have got a network like setting with nodes and edges. Both nodes and edges need to be classes, in this case Node or Arc, as in this question. In my real setup I am dealing with quite a number of subclasses of both Node and Arc. For memory management, I use this answer to the question above.

Problem

When the constructor throws an exception, Visual Studio and g++ with MinGW on Windows cannot catch it, but exit without error handling (g++/MinGW reporting a SIGTRAP signal) while g++ and clang++ on Linux handle the exception correctly. If the Arc is created without exception Arc(n1, n2, false), all compilers work fine. In all cases, there are no relevant compiler warnings (using /W4 resp. -Wall) Can someone explain me, why this is not working on Windows? Or even give a workaround?

Code

#include <iostream>
#include <stdexcept>
#include <vector>
#include <memory>

struct Node;
struct Arc {
    Node *left,*right;
private:
    // shared pointer to self, manages the lifetime.
    std::shared_ptr<Arc> skyhook{this};
public:
    // c'tor of Arc, registers Arc with its nodes (as weak pointers of skyhook)
    explicit Arc(Node* a_, Node* b_, bool throw_exc);
    // resets skyhook to kill it self
    void free() {
        std::cout << "  Arc::free();\n" << std::flush;
        skyhook.reset();
    }
    virtual ~Arc() {
        std::cout << "  Arc::~Arc();\n" << std::flush;
    }
};

struct Node {
    explicit Node() {
        std::cout << "  Node::Node()\n" << std::flush;
    }
    std::vector<std::weak_ptr<Arc> > arcs;
    ~Node() {
        std::cout << "  Node::~Node();\n" << std::flush;
        for(const auto &w : arcs) {
            if(const auto a=w.lock()) {
                a->free();
            }
        }
    }
};

Arc::Arc(Node *a_, Node *b_, bool throw_exc) : left(a_), right(b_) {
    std::cout << "  Arc::Arc()\n" << std::flush;
    if (throw_exc) {
        throw std::runtime_error("throw in Arc::Arc(...)");
    }
    a_->arcs.push_back(skyhook);
    b_->arcs.push_back(skyhook);

}

int main(int argc, char* argv[]) {
    std::cout << "n1=new Node()\n" << std::flush;
    Node *n1 = new Node();
    std::cout << "n2=new Node()\n" << std::flush;
    Node *n2 = new Node();
    std::cout << "try a=new Arc()\n" << std::flush;
    try {
        Arc *a = new Arc(n1, n2, true);
    } catch (const std::runtime_error &e) {
        std::cout << "Failed to build Arc: " << e.what() << "\n" << std::flush;
    }
    std::cout << "delete n1\n" << std::flush;
    delete n1;
    std::cout << "delete n2\n" << std::flush;
    delete n2;

}

Output

This is what I get both on Linux as well on Windows

n1=new Node()
  Node::Node()
n2=new Node()
  Node::Node()
try a=new Arc()
  Arc::Arc()

With g++ (7.4.0 and 8.3.0) or clang++ (6.0.0) on Linux ...

it works as expected:

  Arc::~Arc();
Failed to build Arc: throw in Arc::Arc(...)
delete n1
  Node::~Node();
delete n2
  Node::~Node();

With VC++ (2017) ...

it breaks

Arc::~Arc()

and the run terminates with exit code -1073740940 (0xC0000374)

with g++ (9.1.0) MinGW 7.0

it breaks, but reports the signal

Signal: SIGTRAP (Trace/breakpoint trap)
  Arc::~Arc();

And finishes with exit code 1

Answer 1

tl;dr: inherit from std::enable_shared_from_this and use weak_from_this().

---

Consider the following structure, which is similar to yours (https://godbolt.org/z/vHh3ME):

struct thing
{
  std::shared_ptr<thing> self{this};

  thing()
  {
    throw std::exception();
  }
};

What is the state of the objects *this and self at the moment the exception is thrown, and which destructors are going to be executed as part of stack unwinding? The object itself has not yet finished constructing, and therefore ~thing() will not (and must not) be executed. On the other hand, self is fully constructed (members are initialized before the constructor body is entered). Therefore, ~std::shared_ptr<thing>() will execute, which will call ~thing() on an object which is not fully constructed.

Inheriting from std::enable_shared_from_this does not exhibit this problem assuming no actual shared_ptrs are created before the constructor finishes executing and/or throws (weak_from_this() would be your friend here), since it only holds a std::weak_ptr (https://godbolt.org/z/TGiw2Z); neither does a variant where your shared_ptr is initialized at the end of the constructor (https://godbolt.org/z/0MkwUa), but that's not trivial to incorporate in your case since you're giving shared/weak pointers away in the constructor.

That being said, you still have an ownership problem. Nobody actually owns your Arc; the only outside references to it are weak_ptrs.

Answer 2

It looks like std::shared_ptr is used here to avoid thinking about lifetimes and ownership, which leads to poor code.

A better design is to have a class, say Network, that owns Nodes and Arcs and stores them in std::list. This way you don't need std::shared_ptr or std::week_ptr and the convoluted code that results from using those. Nodes and Arcs can just use plain pointers to each other.

Example:

#include <list>
#include <vector>
#include <cstdio>

struct Node;

struct Arc {
    Node *left, *right;
};

struct Node {
    std::vector<Arc*> arcs;
};

class Network {
    std::list<Node> nodes;
    std::list<Arc> arcs;

public:
    Node* createNode() {
        return &*nodes.emplace(nodes.end());
    }

    Arc* createArc(Node* left, Node* right) {
        Arc* arc = &*arcs.emplace(arcs.end(), Arc{left, right});
        left->arcs.push_back(arc);
        right->arcs.push_back(arc);
        return arc;
    }
};

int main() {
    Network network;
    Node* a = network.createNode();
    Node* b = network.createNode();
    Arc* ab = network.createArc(a, b);
    std::printf("%p %p %p\n", a, b, ab);
}

Answer 3

(It took me a few minutes to realize my own comments were the answer…)

The problem here is that the shared_ptr is (fully) constructed before the Arc is; if an exception interrupts the Arc construction, its destructor is not supposed to be called, but destroying skyhook calls it anyway. (It is legitimate to delete this, even indirectly, but not in this context!)

Since it’s impossible to release a shared_ptr without trickery, the simplest thing to do is to provide a factory function (which avoids certain other problems):

struct Arc {
  Node *left,*right;
private:
  std::shared_ptr<Arc> skyhook;  // will own *this
  Arc(Node *l,Node *r) : left(l),right(r) {}
public:
  static auto make(Node*,Node*);
  void free() {skyhook.reset();}
};
auto Arc::make(Node *l,Node *r) {
  const auto ret=std::make_shared<Arc>(l,r);
  ret->left->arcs.push_back(ret);
  ret->right->arcs.push_back(ret);
  ret->skyhook=ret;  // after securing Node references
  return ret;
}

Since constructing a shared_ptr has to allocate, this is already necessary if you’re concerned about bad_alloc at all.