[Uml-devel] crashes, recursion and a patch

[Carsten Pfeiffer]

-----BEGIN PGP SIGNED MESSAGE-----

Hiya,

I debugged the crash in current umbrello CVS when deleting a class in a 
classdiagram. While doing that, I found several problems in UMLView:

* It often uses QObject::queryList() to obtain a list of child objects to 
iterate over. For some strange reason, it's using a mix of iteration and 
recursion, like in slotObjectRemoved( UMLObject * o )

    QObjectList * l = queryList( "UMLWidget");
    QObjectListIt it( *l );
    UMLWidget *obj;

    while ((obj=(UMLWidget*)it.current()) != 0 ) {
        ++it;
        if(obj -> getID() == id)
        {
            removeWidget(obj);
            delete l;

            // keep calling self while a widget representing o exits
            // as there may be others
            slotObjectRemoved(o);
            return;
        }
    }

I don't see a reason for the recursion here, simply continuing with the 
traversal of the list should suffice, no?

Same goes for a few other methods.


* The next problem is QObject::deleteLater(), combined with queryList(). The 
QObject tree structure is built at QObject construction time through the 
parent pointer and destroyed at destruction time (i.e. ~QObject). Now if you 
deleteLater() an object, queryList() will still return it, as it is still 
alive.

So code like in deleteSelection() causes problems:

- - halfway cleanup() some objects and deleteLater() them in removeWidget()
- - access them again through queryList() in deletSelected() (cleanup() already 
destroyed its UMLWidgetData so dereferencing that causes a crash, see the 
debug output).

Two questions regarding that: 
- - Why call obj->cleanup() before delete obj; or obj->deleteLater() when obj's 
destructor calls cleanup() itself? Is the order important?
- - Why deleteLater() at all?


Additionally I changed a few occurrences of if ( foo ) delete foo; to delete 
foo; and fixed a double deletion (obj->deleteLater(); delete obj;).

Comments?

Cheers
Carsten Pfeiffer
-----BEGIN PGP SIGNATURE-----

iQEVAwUBPq8Rh6WgYMJuwmZtAQGtAwf/UmuA77+AvoXh2b0amUW4jWbX3Q+BNpTy
pFGmpTrwPe2Hvqf6SvgtFqOY/8wMqoVhBGn7/mRysJpEZdY1R9CuX+6xa5WjnH9u
mVwj8Fqlpi6MtI7YveB3U3aQAcGwp1ah2mCOht4JUD56EIsr5ndasX1H6d01uivz
YhAnZlgFWsqMN/oeMWH981kvei8D4441mTta1JW7wnH+rkSVY2pTFXqA00sOW65H
DQwscLebnN1KF+fdz1qh4mFVaRDFmihv39iAU+Wu9a78nppNaPle2pslCH8eYLMW
CBMlDFX9qUAibY5ZpsBvl9bzL8UWWXKgJZUUQXl/9xSbAOVzsBpNmg==
=U3SO
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch
Type: text/x-diff
Size: 3218 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/umbrello-devel/attachments/20030430/d58f368d/attachment.bin>