nodeeditor icon indicating copy to clipboard operation
nodeeditor copied to clipboard

Memory Out-of-Bounds Error When Updating Node Locked State After Scene Clearance

Open li317546360 opened this issue 8 months ago • 0 comments

Issue Description:

A potential memory out-of-bounds access error can occur when updating the locked state of nodes after clearing the scene. This issue arises in the following scenario:

A connection is established using connect to connect the AbstractGraphModel::nodeFlagsUpdated signal to a lambda function that updates the locked state of a NodeGraphicsObject:

connect(&_graphModel, &AbstractGraphModel::nodeFlagsUpdated,
                    [this](NodeId const nodeId) {
                      if (_nodeId == nodeId)
                        setLockedState();
                    });

The scene is cleared using the clearScene() function, which deletes all NodeGraphicsObject instances associated with the nodes. After clearing the scene, the locked state of a node is changed, e.g., by calling setNodesLocked(true/false) or similar methods. When the nodeFlagsUpdated signal is emitted, it attempts to invoke the connected lambda function, which tries to access the deleted NodeGraphicsObject instances, leading to a memory out-of-bounds access and potential crash.

Steps to Reproduce:

  • Create a graph with one or more nodes.
  • Call clearScene() to remove all nodes from the scene.
  • Change the locked state of a node by setting setNodesLocked(true/false) or similar methods.
  • The application will likely crash or exhibit undefined behavior due to the memory out-of-bounds access caused by the lambda function trying to access deleted NodeGraphicsObject instances

Expected Behavior:

The application should not crash or exhibit undefined behavior when updating the locked state of nodes after clearing the scene. Potential Solutions:

  1. Disconnect the nodeFlagsUpdated signal from the lambda function before clearing the scene, or ensure that the signal is not emitted after the scene is cleared.
  2. Implement a check in the lambda function to verify if the NodeGraphicsObject instances still exist before accessing them.
  3. Redesign the locking mechanism to avoid updating the visual representation of nodes after the scene has been cleared.
  4. Use the suggested solution, which disconnects the connection when the NodeGraphicsObject instance is destroyed:
auto conn = connect(&_graphModel, &AbstractGraphModel::nodeFlagsUpdated,
                    [this](NodeId const nodeId) {
                      if (_nodeId == nodeId)
                        setLockedState();
                    });
connect(this, &NodeGraphicsObject::destroyed, this,
        [conn] { disconnect(conn); });


li317546360 avatar Jun 20 '24 12:06 li317546360