Re: [igraph] confused about vector pointer memory management

[Gábor Csárdi]

Simply put, igraph_vector_ptr_t is not a proper container type, and it
is essentially just an array of pointers. It does not manage the
pointers at all. IIRC I even used it for pointers that do not point to
individually allocated memory, so even just calling free() on these
pointer will be a segfault.

At least this was the case before Tamas added the destructor, which
complicates things a bit.

I think ideally we should remove the destructor completely, and
implement a separate type that manages the memory of the items, like
igraph_list_t or something, if there is demand for it. Which I doubt,
to be honest.

Gabor

On Wed, May 10, 2017 at 5:15 PM, Szabolcs Horvát <address@hidden> wrote:
> Thank you for the reponse Tamas.
>
> To explain the practical issue behind the question:
>
> This came up while trying to create a C++ class to be used for RIAA
> memory-management of vector_ptr's that contain (pointers to) vectors.
>
> I originally asked about this here:
>
> https://lists.nongnu.org/archive/html/igraph-help/2015-12/msg00027.html
>
> But then I foolishly ignored Gabor's advice, and relied on the item
> destructor in my C++ class anyway.  As I remember, the reason I did
> this was that I was not sure that a vector_ptr that was handled by
> some arbitrary igraph function will not end up with an item destructor
> set.  Perhaps some igraph functions set item destructors.  I was
> worried that if I destroy the items manually, I may end up with
> double-destruction due to an existing item destructor (that I did not
> set myself).
>
> What actually happened was the opposite:
>
> https://github.com/igraph/igraph/issues/993
>
> An igraph function ended up double-destroying items because I passed
> it a vector_ptr on which I set an item destructor.
>
> Lesson: Never set an item destructor on a vector_ptr that is going to
> be passed to a (high level) igraph function!
>
> To fix this, I used this implementation of the class:
>
> https://github.com/szhorvat/IGraphM/blob/master/IGraphM/LibraryResources/Source/IGCommon.h#L168
>
> It is a bit ugly, but I believe it is safe.  It destroys the item
> manually, and it uses only those functions which do not themselves
> call item destructors.  This means that I could not use
> igraph_vector_ptr_clear() at all, and had to re-implement it.  That is
> not a big deal, of course, but it does involve messing with the
> pointers inside of the igraph_vector_t.
>
> On 10 May 2017 at 17:16, Tamas Nepusz <address@hidden> wrote:
>> Hi,
>>
>>> In particular, I do not understand why igraph_vector_ptr_destroy_all()
>>> will both destroy and free all items, but igraph_vector_ptr_clear()
>>> will destroy them without freeing.
>>
>> You are totally right -- igraph_vector_ptr_clear() should probably call the
>> element destructors AND free the items. The problem is that when we have
>> originally implemented pointer vectors, it did not have any item destructors
>> at all; they were added later as an "afterthought", and we did not get this
>> part right.
>>
>> Actually, for me it seems like igraph_vector_ptr_destroy_all() and
>> igraph_vector_ptr_free_all() should not have been necessary in the first
>> place if we had item destructors from the very beginning. In that case, we
>> could have simply said that igraph_vector_ptr_t vectors do not "own" their
>> pointers and will not ever free them on their own -- all they could do is to
>> call a "destructor" function on them when the items get out of the vector
>> somehow. This way the user could have attached igraph_free as a destructor
>> function for pointer vectors that claim ownership for any pointer added to
>> them, and if the user needed something more complicated than free() (i.e.
>> igraph_*_destroy() followed by igraph_free()), then this should have been
>> done in a specialized destructor function on a case-by-case basis.
>>
>> The best way to avoid confusion for the time being is probably not to use
>> item destructors at all, or at least ensure that whenever you pass an
>> igraph_vector_ptr_t to some part of the code that you don't control, then
>> ensure that the vector does not have an item destructor and that the vector
>> is empty. This way you won't run into any unexpected side effects.
>>
>> I am leaning towards trying to fix this by ensuring that the item
>> destructors get called when the items are removed from the vector using
>> igraph_vector_ptr_clear(), but I'm not sure how this would affect
>> third-party code. Personally, I don't use item destructors in the Python
>> interface and the C interface also uses them sparingly and only in places
>> where the pointer vectors equipped with destructors are not exposed to the
>> user, so this shouldn't cause too much problems for the end users, except
>> those who have been using the C interface directly and have used item
>> destructors before.
>>
>> T.
>>
>> _______________________________________________
>> igraph-help mailing list
>> address@hidden
>> https://lists.nongnu.org/mailman/listinfo/igraph-help
>>
>
> _______________________________________________
> igraph-help mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/igraph-help