On Mon, 2004-08-30 at 00:59, bulia byak wrote:
Mental, please check out the patch in this bug:
https://sourceforge.net/tracker/index.php?func=detail&aid=1012874&gr...
and apply if ok. It fixes a bad memory usage crash in deleteObject.
Unfortunately sourceforge is currently in "read-only" mode, so I cannot comment in the bug itself. The patch hides the problem without really fixing it, though (and sort of breaks the intended semantics I had for successors, albeit not in a way that matters too much).
The bug is not in deleteObject, but rather that deleteObject's caller is not retaining a reference to it (by increasing the reference count) while deleteObject() does its work.
The general rule is always, always, always make sure that you have claimed a refcounted object before calling a nontrivial function (especially one that modifies the object). This holds not just for deleteObject(), or SPObjects in Inkscape, but for any software with simple refcounted objects.
[ The garbage collector relaxes this requirement slightly, but only for classes which are managed by it, which SPObject is not yet. ]
The correct fix is to modify the portion of sp_marker_prev_new():
SPObject *oldmarker = sandbox->getObjectById("sample"); if (oldmarker) oldmarker->deleteObject(false);
thusly:
SPObject *oldmarker = sandbox->getObjectById("sample"); if (oldmarker) { sp_object_ref(oldmarker, NULL); oldmarker->deleteObject(false); sp_object_unref(oldmarker, NULL); }
Probably simliar mistakes are made in other places as well (and with other functions beyond just deleteObject() -- deleteObject() is simply more likely than most to remove the remaining external refcounts on the object).
-mental
On Mon, 2004-08-30 at 22:12, bulia byak wrote:
SPObject *oldmarker = sandbox->getObjectById("sample"); if (oldmarker) { sp_object_ref(oldmarker, NULL); oldmarker->deleteObject(false); sp_object_unref(oldmarker, NULL); }Can't we just add those ref/unref calls to the beginning and the end of the deleteObject?
I've been thinking about it.
I guess my main hesistation is that I don't want to make it look OK to do anything with SPObjects without owning references to them.
In the case of deleteObject(), you'll usually get slapped by a crash -- in other cases the bugs from playing loose with refcounts would manifest themselves less frequently and be harder to debug.
-mental
On Mon, 30 Aug 2004 22:23:14 -0400, MenTaLguY <mental@...3...> wrote:
On Mon, 2004-08-30 at 22:12, bulia byak wrote:
Can't we just add those ref/unref calls to the beginning and the end of the deleteObject?
I've been thinking about it.
I guess my main hesistation is that I don't want to make it look OK to do anything with SPObjects without owning references to them.
In the case of deleteObject(), you'll usually get slapped by a crash -- in other cases the bugs from playing loose with refcounts would manifest themselves less frequently and be harder to debug.
Obviously, my next suggestion would be to add the similar ref/unref to any other SPObject methods that need the object to stay live while they do something to it. I think it's natural for them to take care of it, if they need it. And I think it's highly unnatural to require the users' code to add artificial refs when all they want to do is to delete the stinking object :)
participants (2)
-
bulia byak -
MenTaLguY