constrained_domain argument to FunctionSpace should incref the Python object

[Patrick Farrell]

I think something funny is happening with the memory for the constrained_domain argument to FunctionSpace. Consider:

diff --git demo/documented/periodic/python/demo_periodic.py demo/documented/periodic/python/demo_periodic.py
index 143d25c..d397c2d 100644
--- demo/documented/periodic/python/demo_periodic.py
+++ demo/documented/periodic/python/demo_periodic.py
@@ -61,6 +61,7 @@ class PeriodicBoundary(SubDomain):
 # Create mesh and finite element
 mesh = UnitSquareMesh(32, 32)
 V = FunctionSpace(mesh, "CG", 1, constrained_domain=PeriodicBoundary())
+Z = MixedFunctionSpace([V, V])

 # Create Dirichlet boundary condition
 u0 = Constant(0.0)

This fails with

[pef@aislinn:~/src/dolfin/git/demo/documented/periodic/python] (master) $ python demo_periodic.py 
Traceback (most recent call last):
  File "demo_periodic.py", line 64, in <module>
    Z = MixedFunctionSpace([V, V])
  File "/home/pef/local/fenics-opt/src/dolfin/local.master/lib/python2.7/site-packages/dolfin/functions/functionspace.py", line 494, in __init__
    FunctionSpaceBase.__init__(self, spaces[0].mesh(), element, constrained_domain=spaces[0].dofmap().constrained_domain)
  File "/home/pef/local/fenics-opt/src/dolfin/local.master/lib/python2.7/site-packages/dolfin/functions/functionspace.py", line 90, in __init__
    dolfin_dofmap  = cpp.DofMap(ufc_dofmap, mesh, constrained_domain)
  File "/home/pef/local/fenics-opt/src/dolfin/local.master/lib/python2.7/site-packages/dolfin/cpp/fem.py", line 658, in __init__
    _fem.DofMap_swiginit(self,_fem.new_DofMap(*args))
AttributeError: 'MixedFunctionSpace' object has no attribute 'inside'

If I instead apply the following diff:

diff --git demo/documented/periodic/python/demo_periodic.py demo/documented/periodic/python/demo_periodic.py
index 143d25c..b0419ea 100644
--- demo/documented/periodic/python/demo_periodic.py
+++ demo/documented/periodic/python/demo_periodic.py
@@ -60,7 +60,9 @@ class PeriodicBoundary(SubDomain):

 # Create mesh and finite element
 mesh = UnitSquareMesh(32, 32)
-V = FunctionSpace(mesh, "CG", 1, constrained_domain=PeriodicBoundary())
+periodic = PeriodicBoundary()
+V = FunctionSpace(mesh, "CG", 1, constrained_domain=periodic)
+Z = MixedFunctionSpace([V, V])

 # Create Dirichlet boundary condition
 u0 = Constant(0.0)

then it works.

Since those two should be equivalent, I can only guess that Python is garbage-collecting the PeriodicBoundary() object because there are no Python references to it anymore?

[Johan Hake]

Your observations are correct. One would think that shared_ptr should fix this as it does for many of other similar cases. However, SWIG says this about directors and shared pointers:

Note: There is currently no support for %shared_ptr and the director feature.

We could make an explicit comment about it in the demo though as it is pretty subtle.

[Patrick Farrell]

Do you mean to add a comment that the user code must keep a reference to the boundary? That's pretty unsatisfactory, no? Is there no way we can retain a reference inside the Python wrapper for the FunctionSpace constructor?

[Johan Hake]

I think storing a reference in the Python layer is pretty unsatisfactory too as it only kicks in for objects of subclassed Python classes. Not so much that it might introduce memory leaks, but my objections goes to the maintenance issues it creates. Basically we need to keep track of all methods that takes a director enabled class and extend the Python layer to keep a reference.

[Prof Garth Wells]

Duplicate of #71.

[Prof Garth Wells]

Storing a reference in the Python layer is not ideal, but I don't see any other option given that SWIG does not intend to properly support this shared_ptr +directors.

Fixed made in e90d4842e096f4c1a0ef11b189bdc9466a8a5541

[Patrick Farrell]

Thanks, Garth.

[Prof Garth Wells]

Removing milestone: 1.4 (automated comment)