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

PD: Fix crash when adding sketch to loft via tree view #13686

Merged
merged 1 commit into from May 6, 2024

Conversation

wwmayer
Copy link
Contributor

@wwmayer wwmayer commented Apr 27, 2024

The underlying problem is the method DocumentItem::updateItemSelection() where the selection is altered. This may cause the destruction and recreation of the DocumentObjectItems so that the passed pointer can become dangling.

The issue is fixed in two steps:

  1. Add the method 'DocumentObjectItem findItem(App::DocumentObject obj, const std::string& subname) const' to safely re-access the item.
  2. Add a boolean flag 'dirtyFlag' and the methods insertItem() and removeItem() to DocumentObjectData. This is needed to check when the iterator over the container becomes invalid.

The underlying problem is the method DocumentItem::updateItemSelection() where
the selection is altered. This may cause the destruction and recreation of the
DocumentObjectItems so that the passed pointer can become dangling.

The issue is fixed in two steps:
1. Add the method 'DocumentObjectItem *findItem(App::DocumentObject* obj, const std::string& subname) const'
   to safely re-access the item.
2. Add a boolean flag 'dirtyFlag' and the methods insertItem() and removeItem() to DocumentObjectData.
   This is needed to check when the iterator over the container becomes invalid.
@github-actions github-actions bot added the Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD label Apr 27, 2024
@wwmayer
Copy link
Contributor Author

wwmayer commented Apr 27, 2024

Fixes #13107

@wwmayer
Copy link
Contributor Author

wwmayer commented Apr 27, 2024

Comment to #13682:
That PR makes the crash disappear by not calling the function that triggers the problem in DocumentItem::updateItemSelection(). So to speak it cures the symptoms but doesn't fix the root of the problem. Nevertheless, there is nothing wrong with the PR and it's fine to merge it, too.

@chennes chennes mentioned this pull request Apr 28, 2024
@yorikvanhavre yorikvanhavre merged commit 4ea1ad5 into FreeCAD:main May 6, 2024
10 checks passed
@wwmayer wwmayer deleted the issue_13107 branch May 6, 2024 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants