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 <[email protected]> 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 <[email protected]> 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 >> [email protected] >> https://lists.nongnu.org/mailman/listinfo/igraph-help >> > > _______________________________________________ > igraph-help mailing list > [email protected] > https://lists.nongnu.org/mailman/listinfo/igraph-help _______________________________________________ igraph-help mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/igraph-help
