[vtkusers] Understanding vtkSmartPointers

[Marcus D. Hanwell]

On Mon, May 28, 2018 at 11:45 AM, Fcs <ftpronk at engits.com> wrote:

> > I In my opinion you should aim to never call Delete, but to have a smart
> > pointer contain the things needed.
>
> Is there any danger in calling Delete()? The case I had in mind was to
> perhaps free the memory occupied by a big structured/unstructured grid
> object which would need to live until a sub-grid is extracted from it.
>
> You know your use case better than I do, once the reference count reaches
zero the object will be deleted. If you have a routine where you work on a
large grid, extract out the subgrids, and no longer need it then this is
absolutely an exception to the goal I mentioned. It is more a general goal
in modern C++ code to avoid manual memory management, unless absolutely
necessary. This may well the that case, in which case certainly do it. I
call it in places that warrant it, no additional danger in doing so.

>
> > I think all pipeline API will increase
> > the reference count, and so you only usually need to keep a reference to
> > the pipeline objects you are going to keep around/use. The debug leaks
> > code
> > will help you verify you got it right.
>
> How good is the guarantee that this will work? "I think" and "usually" seem
> to imply that my mileage might vary on this one.. :-)  Does it mean that
> this should work in general, but that perhaps some exotic or slightly buggy
> newer features might not behave correctly? The use case I'm trying to
> implement has a variable number of pre-definied filters which I'm chaining
> together. If I test all the possible combinations, and they work, do I have
> the guarantee they will always work?
>
> It means it works in my experience, and it would normally be a bug should
it not work. I hold on to a very small number of references in general,
deferring many to the pipelines, etc. Where possible and something else
owns it I avoid using smart pointers, and just grab the object as and when
I need it from the owning object.

>
> > I would add that we have found in bigger applications that a passing
> debug
> > leaks is not always enough, ensuring objects are deleted when they are
> > done with is more difficult but necessary, especially in graphical
> > applications that might run for some time.
>
> Does this not imply that every vtkObject created should be stored and
> deleted manually afterwards? Or are there other ways to detect living
> vtkObjects when you don't have a direct member variable pointing to it?
> (say, like the vtkPlane created in applyFilters() in my example code..)
>
> No, it means you need to get object lifetimes right, which can be achieved
using smart pointers. This is a general issue in software engineering, even
something that has no leaks according to valgrind, debug leaks etc
shouldn't accumulate objects.

In Qt I see it where people new a dialog, show it, parent it to the main
window, close it, and then new another dialog. It will be deleted as the
application closes down, they kind of did the right thing, but you could
have a thousand or more instances of that dialog parented to main window.
Valgrind etc will normally tell you there are no leaks in these cases, but
you got the object lifetime wrong.

In that case, having a member variable, and lazily initializing it, then
reusing, or setting the flag to delete the dialog on close would have
worked better. When we load data in pipelines we need to be careful to
ensure the pipelines get deleted as they are removed from the UI, along
with all of their data, rather than when the main window/application shuts
down. Valgrind/debug leaks would be happy, not specific to VTK as such.

>
> > It does the exact same thing with no repetition, and is clearer. The
> > examples require backwards compatibility with older VTK, and I don't
> think
> > Bill liked calling .GetPointer()/.Get() on the vtkNew object. You no
> > longer have to do that, but it is a more recent addition to the vtkNew
> API
> > - it will implicitly return the pointer to the contained object as
> > vtkSmartPointer does.
>
> Good to know! Starting from which version is the call to
> .GetPointer()/.Get() no longer needed? We are developing in vtk 6 at the
> moment, but we should update to a new version at some point in the near
> future.
>
> I think it was more recent than that, I live on or near master and it has
been this way for a while. The commit was from Jun 24, 2017, and it looks
like that was in VTK 8.1.0 (our most recent release).

Marcus
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://vtk.org/pipermail/vtkusers/attachments/20180528/cfd245d5/attachment.html>