[Geany-Devel] On document pointer recycling
nick.treleaven at xxxxx
Sat Oct 26 12:17:09 UTC 2013
On 26/10/2013 00:22, Lex Trotman wrote:
> On 26 October 2013 00:01, Nick Treleaven <nick.treleaven at btinternet.com>wrote:
>> On 24/10/2013 10:24, Lex Trotman wrote:
>>> Okay, but you still agree that doc->is_valid should be removed eventually?
>>>>> That's a step forward:)
>>> Of course I agree. So its not a terribly big step:)
>> I only skimmed the discussion, but how can we remove that?
> Given your experience it would be good if you could read the discussion and
> give your perspective on what the previous posters have discussed regarding
> handling doc pointers properly.
I read the discussion.
In summary, I think there are 3 issues:
* forgetting to check is_valid when iterating documents
* freeing memory
* code that uses is_valid to check if a document still exists, which is
I think freeing memory is a non-issue, I think you (Lex) agree with me
on that. It's not a memory leak, it may be reused later.
The first point needs to be handled before the last one (assuming each
are worth fixing). I think we can:
* remove GeanyDocument::index and fix any code that uses it
* review all code indexing documents_array and fix any code retaining an
index (I expect that is rarer than code using GeanyDocument::index).
* change documents_array to only contain valid documents, i.e. on close
we swap the last valid document pointer with the pointer being removed,
and decrement the length by 1.
* use a free list for invalid documents
The last point is important to ensure no memory corruption bugs are
introduced. I think it is harder to find the code that retains document
pointers (and there is some), so I recommend we don't free document
memory. At least, we would need a better reason to do so IMO than just
memory usage, which is insignificant.
Now, once we've done that it's no longer needed to check is_valid when
iterating documents. Possibly we could remove is_valid, but I think that
is less important to do. Note that if we do all the steps above, we
don't need to break the API, only the ABI. I can't guarantee that bugs
won't be introduced due to these changes, but it seems fairly reliable.
More information about the Devel