Transaction/split question

Phil Longstaff plongstaff at rogers.com
Wed Dec 29 21:44:07 EST 2010


I have found a case where a Split is accessed after it has been freed
when its Transaction is being destroyed.

In Transaction.c, function do_destroy has:

    for (node = trans->splits; node; node = node->next)
    {
        Split *s = node->data;
        if (s->parent == trans)
        {
            xaccSplitDestroy(s);
            xaccSplitCommitEdit(s);
        }
    }
    g_list_free (trans->splits);

xaccSplitDestroy() marks the split as going to be destroyed and sends an
item removed event for the split.  xaccSplitCommitEdit() eventually
frees the memory used by the Split.  The problem is that the split item
removed event includes the split index (position on the split list).
This is found by traversing the list, calling xaccTransStillHasSplit()
on the split and looking for the specified one (and counting) to get the
index.  xaccTransStillHasSplit() checks that split->parent == trans.

So, for 1st split on the list, xaccSplitDestroy() is called, the item
removed event is sent with index == 0 and the split is freed, but left
on the split list.

For the 2nd split on the list, xaccSplitDestroy() is called.  The first
split (just freed above) is still on the list so is checked if its
split->parent == trans, which accesses its memory after the Split has
been freed.

Two possible ways to fix this:

1) split the loop so that xaccSplitDestroy() is called on all splits,
then xaccSplitCommitEdit() is called in a second loop.

2) keep as one loop, but after each split has been processed, remove it
from the split list.  This means that each item removed event will have
index == 0, but at that point, that split would be the first one on the
split list so the value would be correct.

I've tested the first solution which removes the 'invalid read' problems
when I run valgrind.  It also keeps behaviour like it is now, a series
of item removed events with different index values.  I haven't tested
the second solution.

Does anyone know the code which catches and uses these split item
removed events?  How does that code make use of the index?

Phil



More information about the gnucash-devel mailing list