Issue 122120 - Crash when load a new arrow style in line dialog.
Summary: Crash when load a new arrow style in line dialog.
Alias: None
Product: General
Classification: Code
Component: ui (show other issues)
Version: 4.0.0-dev
Hardware: All All
: P3 Critical (vote)
Target Milestone: 4.0.0
Assignee: Armin Le Grand
QA Contact:
Depends on:
Blocks: [sidebar]
  Show dependency tree
Reported: 2013-04-22 06:51 UTC by Du Jing
Modified: 2022-10-28 12:54 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---


Note You need to log in before you can comment on or make changes to this issue.
Description Du Jing 2013-04-22 06:51:44 UTC
Trunk build: r1413470

1)Create a new spreadsheet/text document
2)Click View->Toolbars->Drawing to show drawing toolbar
3)insert a new line
4)Load a new arrow style via Line dialog->arrow styles->load arrow styles.

AOO crash when click "ok" button of line dialog.
Comment 1 Armin Le Grand 2013-04-24 17:36:29 UTC
ALG: This also happens with versions without Sidebar -> 

To reproduce:

- Open app (any, Draw/Impress simplest)
- Open LineStyle dialog
- In TabPage ArrowStyles, load some and press OK -> crash

Seems to have to do with a callback when the dialog already is deleted. Taking a look...
Comment 2 Armin Le Grand 2013-04-25 09:53:30 UTC
ALG: Reason is that someone changed with r1380436 the SdrModel::SetLineEndList to always create the destructor for the current list. Despite that there are (prob many) places in cui and other dialogs where something like

		delete pDrawModel->GetLineEndList();

is used before setting the new list -> The list is deleted twice, this crashes. This is done for all 6 lists in the SdrModel, thus I'm thinking about using shared_ptr here and building incompatible up to make this safe...
Comment 3 Armin Le Grand 2013-04-26 09:32:36 UTC
ALG: Basic changes in svx done, working up the build hierarchy...
Comment 4 Armin Le Grand 2013-04-26 13:57:45 UTC
ALG: Isolating the XPropertyList derivate constructors (to private), adding a factory to allow only instances of SharedPtrs of the XPropertyList-classes...
Comment 5 Armin Le Grand 2013-04-26 16:26:59 UTC
ALG: Pretty far in changes, but need a complete incompatible build to check. Preparing, also prepatring local update (at this occasion)...
Comment 6 Armin Le Grand 2013-04-28 11:53:06 UTC
ALG: Checked behaviour, all UI previews work as before, all XPropertyList instances are now handled as refcounted ones. It could be seen from the changed code that already many fixes in this area had happened to avoid memory leaks. This is over now.

- XPropertyList no longer freely incarnatable
- Only XPropertyXXXSharedPtr are allowed
- All methods/members/exchanges use this now, no more unclear ownerships
- Changed from List to stl::vector usage
- Unified all XPropertyList derived implementations to use a single SdrModel/VirtualDevice instance for preview generation

Preparing commit...
Comment 7 SVN Robot 2013-04-28 11:57:22 UTC
"alg" committed SVN revision 1476756 into trunk:
i122120 Cleanup of XPropertyList and derivates to RefCounted instances, more ...
Comment 8 Armin Le Grand 2013-04-29 08:17:22 UTC
ALG: OKay, comitted, done. The error after load does not happen anymore.
Comment 9 SVN Robot 2013-04-29 10:03:44 UTC
"alg" committed SVN revision 1476949 into trunk:
i122120 WaE corrections, missing definition in sd
Comment 10 SVN Robot 2013-04-29 10:58:02 UTC
"alg" committed SVN revision 1476958 into trunk:
i122120 corected error in XPropertyList::Insert when index is equal to -1
Comment 11 Armin Le Grand 2013-04-29 13:52:17 UTC
ALG: When using this i stumbled over a recursion problem when the temp SdrModel for preview creation itself creates XPropertyList(s), so the shared model will not be freed. Looking for a solution...
Comment 12 Armin Le Grand 2013-04-29 14:30:59 UTC
ALG: Okay, works with a flag in the SdrModel. Not nice, but good enough for now. The better solution would be to create the previews without needing SdrObjects/SdrModel at all (use primitives directly). It is even possible to use SdrObjects in this case without SdrModel, but this does not correlate with the paradigm of aw080 that all SdrObjects need a SdrModel at construction time.
Comment 13 SVN Robot 2013-04-29 14:33:43 UTC
"alg" committed SVN revision 1477102 into trunk:
i122120 Make sure helper model itself will not allocate XPropertyLists
Comment 14 Armin Le Grand 2013-04-30 12:41:11 UTC
ALG: Changed all UI preview creators to no longer need SdrModel/SdrObject at all, instead base on primitives. This avoids problems and will make the expensive ressource SdrModel obsolete for this purpose, thus being faster and less mem consuming.
Comment 15 SVN Robot 2013-04-30 12:43:41 UTC
"alg" committed SVN revision 1477598 into trunk:
i122120 Changed all UI preview creators to no longer need SdrModel/SdrObject
Comment 16 Armin Le Grand 2013-05-02 11:10:33 UTC
ALG: Checking all ListEntry creators - sometimes entries with no preview are generated. Also seen: Pattern editor for Bitmap fill somehow works wrong...
Comment 17 SVN Robot 2013-05-02 13:25:05 UTC
"alg" committed SVN revision 1478365 into trunk:
i122120 Ensured Append/Modify methods in LB implementations always add a UI g...
Comment 18 Andre 2013-05-03 13:02:29 UTC
Controls 'Style' and 'Arrow' in 'Line' panel show checkerboard backgrounds.  This is probably an artifact from the preview creation.  The fact that these previews are bitmaps and are created on the fly is an implementation detail that should not be exposed to the user.

Seen on Linux (Ubuntu 12.04 64bit).
Comment 19 SVN Robot 2013-05-07 09:37:02 UTC
"alg" committed SVN revision 1479829 into trunk:
i122120 corrected flag for drawing checkerboards, adapted previews, added to ...
Comment 20 Armin Le Grand 2013-05-07 15:06:34 UTC
ALG: My experiments with visualizing transparency in all previews were not complete and the default was wrong; have now completed, added a switch to the user settings and corrected the default (also for the gallery). Comitted, done.