Crash: cyclic connection

[fredakilla]

#111 expose and fix crash when you connect node output to self input (loop).

However it's still possible to make a loop through graph node and it will crash, like this :

Any idea to detect and forbid cyclic connection ?

[sindney]

Try this:

// 1.5) Forbid connecting the node to itself
  Node* node = _connection->getNode(oppositePort(requiredPort));

  if (node == _node)
    return false;

  if (node) {
      auto isConnected = [](Node* a, Node* b) {
          QQueue<Connection*> cons;
          QQueue<Node*> nodes;
          nodes.enqueue(a);
          while (!nodes.isEmpty()) {
              auto curNode = nodes.dequeue();
              auto& curNodeState = curNode->nodeState();
              auto curNodeCons = curNodeState.getEntries(PortType::Out);
              for (auto& nodeCons : curNodeCons) {
                  for (auto& nodeCon : nodeCons) {
                      cons.enqueue(nodeCon.second);
                  }
              }
              while (!cons.isEmpty()) {
                  auto curCon = cons.dequeue();
                  if (auto conNode = curCon->getNode(PortType::In)) {
                      if (conNode == b)
                          return true;
                      nodes.enqueue(conNode);
                  }
              }
          }
          return false;
      };
      if (requiredPort == PortType::Out) {
          if (isConnected(node, _node)) {
              return false;
          }
      } else if (isConnected(_node, node)) {
          return false;
      }
  }

I'll make a pr about this when @paceholder is free.

[fredakilla]

Excellent ! works fine. thanks.

[samiavasil]

Hi ... I found the same issue, but for me have a more simple way to fix crash. Just in Node.cpp on row 38, change : connect(_nodeDataModel.get(), &NodeDataModel::dataUpdated, this, &Node::onDataUpdated);

to:

connect(_nodeDataModel.get(), &NodeDataModel::dataUpdated, this, &Node::onDataUpdated, Qt::QueuedConnection);

In this way you don't a crash because doesn't break the stack. But have a high CPU usage, because this is a real data loop.

[Daguerreo]

@paceholder is there a reason to not integrate the @sindney 's fix? Is not ok? Please do answer

[rayment]

Any update on this being a thing?