[Geany-Devel] On document pointer recycling

[Matthew Brush]

On 13-10-26 06:22 AM, Lex Trotman wrote:
> On 26 October 2013 23:17, Nick Treleaven <nick.treleaven at btinternet.com>wrote:
>
>> [snip]
>> * 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.
>>
>
> Yes, the memory is not an issue, but the real problem (not handling close)
> needs to be fixed wherever it occurs, and changing the implementation won't
> help.  In fact, as I said in the PS that started this thread, the current
> implementation at least ensures that the pointer/index will always
> reference something that is a GeanyDocument object, if the document structs
> were de-allocated in the normal way the pointer could point to anything.
>
> Although the chances of the wrong document being referenced because of a
> retained pointer are slim, any fault that can happen, will happen (and at
> the worst moment according to Murphy).
>

If the GeanyDocument were a GObject (or we added hand-rolled reference 
counting), the document would never get freed from under the code that 
uses them's backs. It's a different way of thinking of it, but it's like 
std::shared_ptr or any GObjects, the last person that stops needing the 
thing and drops its last reference, causes the GeanyDocument destructor 
to run which would free the memory, and you'd be 100% sure everyone that 
anyone referencing the document was no longer doing so by the time the 
actual memory gets freed. This way it ensures any weird dangling 
pointers or pointing to the wrong document can't happen (unless someone 
is holding a document pointer, without taking a reference, and without 
watching for its close or destroy signals, which would mean it's buggy 
code anyway).

It takes a different way of thinking about it, but basically "close" of 
a document and "free" of a document are two distinct things, although an 
opportune time for various document holders to drop their reference is 
when the document emits its "close" signal, but I could imagine some 
cases (like caching closed documents in a free or MRU list or 
something), where the reference count would never drop to zero, even 
though the document emitted its close signal.

This makes it a lot easier in the future to have the same document open 
inside multiple notebooks or windows, or even some crazy plugin thing 
where it's showing the document in some other type of view where it 
doesn't make sense for its document pointer to become invalidated even 
if Geany core itself has dropped its references and doesn't want to 
manage/show this document anymore.

This is basically the same as the ScintillaObject; parts of core or 
plugins are all free to retain references to it to prevent it from 
vanishing when the main tab showing it gets closed. I feel quite 
confident that this is the most sane way to manage memory/sharing of 
objects in C and it's endemic in G*-style code already (including all 
over Geany's code for other objects/widgets).

My two cents

Cheers,
Matthew Brush