| Summary: | Reproducible segfault when trying to create a synchronous message | ||
|---|---|---|---|
| Product: | [Applications] umbrello | Reporter: | J.H.M. Dassen (Ray) <bugs.kde.org> |
| Component: | general | Assignee: | Umbrello Development Group <umbrello-devel> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | Jan.van.Eldik, mbriza, pep.turro+bugs.kde, ralf.habacker |
| Priority: | NOR | ||
| Version: | SVN | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| Latest Commit: | http://commits.kde.org/umbrello/b8455e6a66f140903060e8f911ff0315c7aa98a7 | Version Fixed In: | 4.10.4 |
| Sentry Crash Report: | |||
| Attachments: |
Umbrello model file used in reproducer
gdb backtrace of crash simple test case for this kind of problem updated testcase Proposed fix fix recursive traversing |
||
|
Description
J.H.M. Dassen (Ray)
2012-09-14 10:26:54 UTC
Created attachment 73905 [details]
Umbrello model file used in reproducer
Created attachment 73906 [details]
gdb backtrace of crash
Git commit 72ee9088d4b8f3f9dd1bef2094c07772db9d0be7 by Ralf Habacker. Committed on 28/05/2013 at 02:22. Pushed by habacker into branch 'master'. Fixed "duplicate entry" error. M +2 -1 umbrello/widgets/messagewidget.cpp http://commits.kde.org/umbrello/72ee9088d4b8f3f9dd1bef2094c07772db9d0be7 Git commit ba9c3b19f89eac16cee8d8215bf81e5dd99c730c by Ralf Habacker. Committed on 28/05/2013 at 04:58. Pushed by habacker into branch '306785'. Fixed crash on adding sequence diagram messages by breaking recursive inheritance loops. Thanks to Martin Briza for pointing to this bug. M +9 -2 umbrello/classifier.cpp M +1 -1 umbrello/classifier.h http://commits.kde.org/umbrello/ba9c3b19f89eac16cee8d8215bf81e5dd99c730c Git commit 2961f865229f9aaacc61de40b6f6999373ff022d by Ralf Habacker. Committed on 28/05/2013 at 04:58. Pushed by habacker into branch 'master'. Fixed crash on adding sequence diagram messages by breaking recursive inheritance loops. Thanks to Martin Briza for pointing to this bug. M +9 -2 umbrello/classifier.cpp M +1 -1 umbrello/classifier.h http://commits.kde.org/umbrello/2961f865229f9aaacc61de40b6f6999373ff022d Created attachment 80129 [details]
simple test case for this kind of problem
Git commit e5f6b455e70f610171ba1abea790eae3f1cf7ea7 by Ralf Habacker. Committed on 28/05/2013 at 04:58. Pushed by habacker into branch 'KDE/4.10'. Fixed crash on adding sequence diagram messages by breaking recursive inheritance loops. Thanks to Martin Briza for pointing to this bug. M +9 -2 umbrello/classifier.cpp M +1 -1 umbrello/classifier.h http://commits.kde.org/umbrello/e5f6b455e70f610171ba1abea790eae3f1cf7ea7 Git commit 82e817e96f2eae23b68a6bc2c48958be3321a15c by Ralf Habacker. Committed on 28/05/2013 at 02:22. Pushed by habacker into branch 'KDE/4.10'. Fixed "duplicate entry" error. M +2 -1 umbrello/widgets/messagewidget.cpp http://commits.kde.org/umbrello/82e817e96f2eae23b68a6bc2c48958be3321a15c Thanks for the fix, Ralf. I was thinking about it and I'm quite sure this isn't a complete solution, though, as I think there's still a possibility there is a cycle in the graph somewhere deeper than returning to the root as both your and my patches expect. Completely solving this bug would involve creating a QSet of all previously processed nodes and checking if a particular node was already traversed through. Or a similar approach. Created attachment 80179 [details] updated testcase (In reply to comment #9) > Thanks for the fix, Ralf. > I was thinking about it and I'm quite sure this isn't a complete solution, > though, as I think there's still a possibility there is a cycle in the graph > somewhere deeper than returning to the root as both your and my patches > expect. I agree, see appended test case. Try to add a message from "neue_klasse" to "neue_klasse5" > Completely solving this bug would involve creating a QSet of all previously > processed nodes and checking if a particular node was already traversed > through. Or a similar approach. patches welcome :-) Git commit 01fa09b652cba8044b36ac1eeadcc716c6a5c272 by Ralf Habacker. Committed on 30/05/2013 at 16:02. Pushed by habacker into branch 'master'. Enable guard against resursive inheritance loops to self. M +1 -1 umbrello/dialogs/selectopdlg.cpp http://commits.kde.org/umbrello/01fa09b652cba8044b36ac1eeadcc716c6a5c272 Git commit 4b69830b26c81ae280ce3db030ba00315b3ce141 by Ralf Habacker. Committed on 30/05/2013 at 16:02. Pushed by habacker into branch 'KDE/4.10'. Enable guard against resursive inheritance loops to self. M +1 -1 umbrello/dialogs/selectopdlg.cpp http://commits.kde.org/umbrello/4b69830b26c81ae280ce3db030ba00315b3ce141 For now I disabled the display of inherited methods. Created attachment 80211 [details]
Proposed fix
Hi, I wrote the patch as I proposed in my previous comment. I'm still inclined to use a stack (in a QList) instead of recursion. Maybe it's not as elegant but it at least isn't limited by the stack frame... and it makes adding the set easier.
It reverts your yesterday's changes too, I suppose you'll want to revert them yourself in different commits.
Created attachment 80271 [details]
fix recursive traversing
I extended your latest patch with an experimental support for recursive traversing, with which the testcases do not crash anymore and the list of operations includes all inherited methods. [1]
If you still like to implement a linear search instead of recursive search feel free .
[1] Just got the impression that it may be a good to prefix inherited method with the related class name.
Ralf, it doesn't really matter to me, I'm just trying to use recursion as less as possible because of the risk of overflowing the stack. If you're satisfied with the patch, I agree with applying it. Thank you, Martin Git commit b939eb245482ff8a436003169baae8f240bccdbb by Ralf Habacker. Committed on 03/06/2013 at 07:13. Pushed by habacker into branch 'master'. Prevent recursive search loops. M +34 -12 umbrello/classifier.cpp M +2 -1 umbrello/classifier.h http://commits.kde.org/umbrello/b939eb245482ff8a436003169baae8f240bccdbb Git commit 44b93882e8a99c6ec9c25cdff198a7679a711c4f by Ralf Habacker. Committed on 03/06/2013 at 13:30. Pushed by habacker into branch 'master'. Reenable including of inherited methods on adding messages to sequence diagram. M +1 -1 umbrello/dialogs/selectopdlg.cpp http://commits.kde.org/umbrello/44b93882e8a99c6ec9c25cdff198a7679a711c4f Git commit b8455e6a66f140903060e8f911ff0315c7aa98a7 by Ralf Habacker. Committed on 03/06/2013 at 05:13. Pushed by habacker into branch 'KDE/4.10'. Prevent recursive search loops. M +34 -12 umbrello/classifier.cpp M +2 -1 umbrello/classifier.h http://commits.kde.org/umbrello/b8455e6a66f140903060e8f911ff0315c7aa98a7 Git commit d090de300ee303e94a028351feac6e7460a444d9 by Ralf Habacker. Committed on 03/06/2013 at 11:30. Pushed by habacker into branch 'KDE/4.10'. Reenable including of inherited methods on adding messages to sequence diagram. M +1 -1 umbrello/dialogs/selectopdlg.cpp http://commits.kde.org/umbrello/d090de300ee303e94a028351feac6e7460a444d9 |