Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
li317546360 opened this issue Jun 20, 2024 · 0 comments

Comments

@li317546360
Copy link

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); });

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant