BlockMatrix and deleted matrices

[Joachim Haga]

The following code crashes, unless "del A" is commented out. I don't know the memory handling strategy in dolfin, (boost::shared_ptr is used in some places, but not consistently?) but attached is a patch which at least removes the crash by making private copies of the matrices (and vectors in BlockVectors).

from dolfin import *

mesh = UnitSquare(4, 4)
V = FunctionSpace(mesh, "CG", 1)
u,v = TrialFunction(V), TestFunction(V)

a = u*v*dx

AA = BlockMatrix(1,1)

A = assemble(a)
AA[0, 0] = A

del A

print AA[0,0]

[Garth Wells (garth-wells)]

The most robust solution would be to have BlockFoo use smart pointers.

[Johan Hake (johan-hake)]

It was a while we had the problem of object getting out of scope, especially
after we got the shared_ptr interface going. It think we are up for some heavy
coding if we are introducing shared_ptrs (or some other smart_pointer) for the
linear algebra objects.

Eventhough your patch fixes the problem in DOLFIN I am not sure we want the
extra complication od the view flag. I just added a place holder in the python
interface for the Foo object fixing the out of scope problem. A hack, but a
quite non intrucive one.

Johan

On Thursday January 13 2011 06:22:26 Joachim Haga wrote:
> Public bug reported:
>
> The following code crashes, unless "del A" is commented out. I don't
> know the memory handling strategy in dolfin, (boost::shared_ptr is used
> in some places, but not consistently?) but attached is a patch which at
> least removes the crash by making private copies of the matrices (and
> vectors in BlockVectors).
>
> from dolfin import *
>
> mesh = UnitSquare(4, 4)
> V = FunctionSpace(mesh, "CG", 1)
> u,v = TrialFunction(V), TestFunction(V)
>
> a = u*v*dx
>
> AA = BlockMatrix(1,1)
>
> A = assemble(a)
> AA[0, 0] = A
>
> del A
>
> print AA[0,0]
>
> ** Affects: dolfin
> Importance: Undecided
> Status: New

[Joachim Haga (jobh)]

Yes, but not only BlockFoo, it will still fail unless the other Matrix users (in particular, the python interface) also use shared_ptr. Does it already?

[Garth Wells (garth-wells)]

Most (or almost most) of the linear algebra backends use shared pointers, and they all should use shared pointers. It shouldn't be a big deal to fix this since the most complicated (PETSc and Epetra) already use shared pointers (which is particularly neat, since a customised destructor for shared pointers calls the necessary PETSc/Epetra functions to clean up).

I think that the Python interface respects the shared pointers. What I don't know is exactly what 'del' does to the object A.

I would like to see DOLFIN be free of plain pointers - it's just a source of trouble.

[Johan Hake (johan-hake)]

That is true. But we do not have too many objects that keep references to La objects. We probably need to list these and either add a similar hack or consider storing them using shared_ptrs, which they are not by today. And again it will introduce a quite large revamp of the interface to introduce that.

[Johan Hake (johan-hake)]

I guess you mean that all the actuall la backend objects use shared_ptr. The
thing is that these are mostly not exposed to Python. What I mean is that we
do not have a shared_ptr interface for the GeneriFoo objects.

Any method or object taking a GenericFoo expects a reference. We could
duplicate the interface to also include shared_ptr version of GenericFoo
methods, like we have for Function and FunctionSpace.

Johan

On Thursday January 13 2011 11:06:51 Garth Wells wrote:
> Most (or almost most) of the linear algebra backends use shared
> pointers, and they all should use shared pointers. It shouldn't be a big
> deal to fix this since the most complicated (PETSc and Epetra) already
> use shared pointers (which is particularly neat, since a customised
> destructor for shared pointers calls the necessary PETSc/Epetra
> functions to clean up).
>
> I think that the Python interface respects the shared pointers. What I
> don't know is exactly what 'del' does to the object A.
>
> I would like to see DOLFIN be free of plain pointers - it's just a
> source of trouble.

[Joachim Haga (jobh)]

I understand there is a larger issue here. In the meantime, Johan's fix (which just keeps the submatrices referenced on the python level) is sufficient to remove the crash. Or mine, but Johan's has the distinct advantage of being already committed :)

Be aware that there is a memory leak in Block* if "owner" is set on the constructor and "set(...)" is later called, but that's probably not very important.

[Johan Hake (johan-hake)]

On Thursday January 13 2011 23:41:26 Joachim Haga wrote:
> I understand there is a larger issue here. In the meantime, Johan's fix
> (which just keeps the submatrices referenced on the python level) is
> sufficient to remove the crash. Or mine, but Johan's has the distinct
> advantage of being already committed :)

I guess you are right! Sorry for that ;)

> Be aware that there is a memory leak in Block* if "owner" is set on the
> constructor and "set(...)" is later called, but that's probably not very
> important.

Ok, I think the best fix is the one that Garth started out on, using
shared_ptrs. But there are some issues as all objects inheriting from
GenericTensor then needs to be stored using shared_ptr in Python. This mostly
works out of the box, but we probably need to construct director typemaps or
add some shared_ptr<GenericFoo> arguments to director classes, as SWIG wont
handle these correctly.

On the TODO list!

Johan

[Garth Wells (garth-wells)]

The test program runs fine now. We're now using shared_ptrs in the Python interface and BlockFoo, which should resolve the issue.