[Gnucash-changes] r13152 - gnucash/trunk/lib/libqof/qof - Don't allow the event handler list to shrink while we're traversing it.

[warlord at MIT.EDU]

FYI, I've got a fix in place now that I'm about to commit.

-derek

Quoting Neil Williams <linux at codehelp.co.uk>:

> On Friday 10 February 2006 10:06 pm, Derek Atkins wrote:
>> It's not the best solution.  It /is/ simplistic.  But the problem is
>> that the code is re-entrant but it's not quite handling that properly.
>
> OK, that gives me a starting point.
>
>> Hopefully Chris will chime in here.  I honestly don't know offhand
>> what crash he's seeing, but it's certainly a theoretical problem due
>> to the re-entrancy.
>
> I agree.
>
>> > What about making this stage conditional? (It would at least limit the
>> > inexorable growth of the list.)
>>
>> I'm not sure how you make it conditional.  The real fix is to do
>> delayed destruction of the event handler, but then you also need a
>> nest calculator to make sure you clean up only at nestlevel 0.  This
>> requires at least changing the ABI if not the API
>
> What about doing that clean up internally?
> (at the end of the loop?)
>
>> (I don't recall if
>> the event-handler list is a public QOF object or not).
>
> It's not. Only registering and unregistering event handlers is supported in
> the API. The list is never exposed, there's no foreach or even a count
> available outside QOF.
>
>> Chris, Josh, David, and I had a dicussion about this on IRC before
>> Chris made this change (see what you miss by not hanging out on IRC?)
>
> (There was no way I'd have been around at the time that discussion was going
> on. Even allowing for timezones, I wasn't online due to other commitments.)
>
>> Yes, this is a short-term fix to keep the accessing freed memory.
>> It's not a long-term fix, which requires a little bit of
>> re-architecting the QOF Event code.
>
> OK.
>
>> > This isn't an API issue, it's a technical issue about how best to
>> > prevent this crash without causing another elsewhere by consuming
>> > resources on a much more limited platform.
>>
>> It /may/ be an API issue depending on whether the eventlist objects
>> are opaque or not.
>
> They are.
>
>> I don't recall and I haven't looked right now.
>> I'll go take a look this evening or this weekend and see if I can come
>> up with something.
>
> Thanks.
>
>
> --
>
> Neil Williams
> =============
> http://www.data-freedom.org/
> http://www.nosoftwarepatents.com/
> http://www.linux.codehelp.co.uk/
>
>